package:vm/kernel_front_end.dart does not accept package_config.json file

[jonahwilliams]

To ensure null safety rolls are not blocked, I am updating all of the dart rules in flutter/engine to use the package_config.json file. When updating the kernel_compiler rule for fuchsia, I found that the kernel_front_end.dart file is doing its own parsing of .packages files. The immediate error is an exception when I provide a package_config.json, but this may cause other problems in the future when .packages are removed.

Error:

FAILED: clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill 
python ../../third_party/dart/build/gn_run_binary.py compiled_action /b/s/w/ir/cache/builder/src/third_party/dart/tools/sdks/dart-sdk/bin/dart --depfile=/b/s/w/ir/cache/builder/src/out/fuchsia_debug_x64/clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill.d --depfile_output_filename=clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill --dfe=/b/s/w/ir/cache/builder/src/third_party/dart/tools/sdks/dart-sdk/bin/snapshots/kernel-service.dart.snapshot /b/s/w/ir/cache/builder/src/third_party/dart/pkg/vm/bin/gen_kernel.dart --packages=/b/s/w/ir/cache/builder/src/third_party/dart/.dart_tool/package_config.json --platform=/b/s/w/ir/cache/builder/src/out/fuchsia_debug_x64/clang_x64/vm_platform_strong.dill --no-aot --no-embed-sources --no-link-platform --output=/b/s/w/ir/cache/builder/src/out/fuchsia_debug_x64/clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill /b/s/w/ir/cache/builder/src/flutter/shell/platform/fuchsia/dart/compiler.dart
Command failed: /b/s/w/ir/cache/builder/src/third_party/dart/tools/sdks/dart-sdk/bin/dart --depfile=/b/s/w/ir/cache/builder/src/out/fuchsia_debug_x64/clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill.d --depfile_output_filename=clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill --dfe=/b/s/w/ir/cache/builder/src/third_party/dart/tools/sdks/dart-sdk/bin/snapshots/kernel-service.dart.snapshot /b/s/w/ir/cache/builder/src/third_party/dart/pkg/vm/bin/gen_kernel.dart --packages=/b/s/w/ir/cache/builder/src/third_party/dart/.dart_tool/package_config.json --platform=/b/s/w/ir/cache/builder/src/out/fuchsia_debug_x64/clang_x64/vm_platform_strong.dill --no-aot --no-embed-sources --no-link-platform --output=/b/s/w/ir/cache/builder/src/out/fuchsia_debug_x64/clang_x64/gen/flutter/shell/platform/fuchsia/dart/kernel_compiler.dart.dill /b/s/w/ir/cache/builder/src/flutter/shell/platform/fuchsia/dart/compiler.dart
output: Unhandled exception:
FormatException: Scheme not starting with alphabetic character (at character 1)
 "2020-06-29T11:21:54.984521",
^

#0      _Uri._fail (dart:core/uri.dart:1628:5)
#1      _Uri._makeScheme (dart:core/uri.dart:2167:7)
#2      new _Uri.notSimple (dart:core/uri.dart:1459:18)
#3      Uri.parse (dart:core/uri.dart:1022:17)
#4      _Uri.resolve (dart:core/uri.dart:2592:27)
#5      convertToPackageUri (package:vm/kernel_front_end.dart:697:39)
<asynchronous suspension>
#6      runCompiler (package:vm/kernel_front_end.dart:240:21)
#7      compile (file:///b/s/w/ir/cache/builder/src/third_party/dart/pkg/vm/bin/gen_kernel.dart:39:10)
#8      main (file:///b/s/w/ir/cache/builder/src/third_party/dart/pkg/vm/bin/gen_kernel.dart:34:25)
#9      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:299:32)
#10     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

[7204/7281] ACTION //third_party/dart/utils/kernel-service:frontend_server_dill(//build/toolchain/fuchsia:fuchsia)
[7205/7281] ACTION //flutter/flutter_frontend_server:frontend_server_dill(//build/toolchain/linux:clang_x64)
ninja: build stopped: subcommand failed.

This is blocking flutter/flutter#60999

Example PR: flutter/engine#19582

[zanderso]

@alexmarkov @aam

[alexmarkov]

.packages file was used to convert main script from file: URI to a correct package: URI automatically if main belongs to a package. If we can drop this auto-correction then we don't need to parse package_config.json.

[aam]

I think we still need to canonicalize script uri provided on a command line to package: scheme if possible. I think that means supporting package_config.json in addition to .packages in vm front_end wrapper.

[alexmarkov]

@aam Why do we need to keep this auto-correction? This auto-correction is only done in gen_kernel but not in frontend_server, and as far as I know only used in Fuchsia. Maybe we can fix Fuchsia build rules instead and remove the auto-correction.

[aam]

@aam Why do we need to keep this auto-correction?

Main reason is not to break things.

but not in frontend_server,

Yeah, flutter has logic to convert uri to package: already: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/compile.dart#L262

[jonahwilliams]

I do not think migrating all kernel_compiler targets is a reasonable short-term solution. @aam or @alexmarkov is there anything blocking kernel frontend from using package_config?

[alexmarkov]

@jonahwilliams package:package_config sound like the right way to handle this for now.

However, this auto-correction was causing problems multiple times in the past, and we discussed removing it as it doesn't provide much value but only causing problems. This auto-correction was basically a short-term workaround to the breaking change in CFE when CFE stopped accepting incorrect URIs, but unfortunately this workaround is still there and comes back to bite us again. Maybe it's time to stop it from happening again?

@chaselatta @iskakaushik Is it feasible to fix Fuchsia build rules or we should keep this main script URI auto-correction WDYT?

[jonahwilliams]

I don't have an opinion on whether removing the auto conversion is a problem, but every build rule in the engine that is using .packages is potentially going to block all null safety work. Adding the package config library and then separately deciding whether to deprecate the functionality seems like a much more straightforward path.

[chaselatta]

We do have a backlog item to migrate from .package to package_config. I can bump that up on the priority list since it is going to be needed eventually anyway.

I also would like to migrate the fuchsia rules to pass a package that contains main to the kernel compiler instead of passing fuchsia-source:///path/to/main.dart. I'm not sure how much work this will be but I can work with @alexmarkov and @jonahwilliams to figure out the best path forward.