sdk icon indicating copy to clipboard operation
sdk copied to clipboard

dart2js skips compilation if the same dart library is used by dart vm project

Open annagrin opened this issue 3 years ago • 3 comments

Dart debug extension build skips generating background.js file when the library package:dwds/src/utilities/batched_stream.dart (https://github.com/dart-lang/webdev/blob/b6439072d118b740fbc2d0e5af4e10952642d864/dwds/lib/src/utilities/batched_stream.dart#L9) is used by the dart extension and unrelated dwds code.

Note: the build was working fine until we started using the package:dwds/src/utilities/batched_stream.dart in dwds in https://github.com/dart-lang/webdev/blob/b6439072d118b740fbc2d0e5af4e10952642d864/dwds/lib/src/services/batched_expression_evaluator.dart#L15 (added in https://github.com/dart-lang/webdev/pull/1746)

To work around the issue, we created an identical copy of the library only used from code compiled by dart2js: package:dwds/src/web_utilities/batched_stream.dart (https://github.com/dart-lang/webdev/blob/b6439072d118b740fbc2d0e5af4e10952642d864/dwds/lib/src/web_utilities/batched_stream.dart#L9)

Is this a bug in dart2js or should we modify our code somehow to avoid duplicating the code?

Repro

(on linux or macos)

  • clone webdev repo https://github.com/dart-lang/webdev
  • comment out the line https://github.com/dart-lang/webdev/blob/b6439072d118b740fbc2d0e5af4e10952642d864/dwds/debug_extension/web/background.dart#L26 and uncomment the line above it
  • run tool/build_extension.sh prod in webdev/dwds/debug_extension directory

Expected

Build succeeds and generates webdev/dwds/debug_extension/prod_build/background.js

Actual

➜  debug_extension ✗ tool/build_extension.sh prod
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Building dart2js-compiled extension to /prod_build directory.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[INFO] Generating build script completed, took 165ms
[INFO] Reading cached asset graph completed, took 118ms
[INFO] Checking for updates since last build completed, took 338ms
[WARNING] build_web_compilers:entrypoint on web/background.dart:
Skipping compiling extension|web/background.dart with dart2js because some of its
transitive libraries have sdk dependencies that are not supported on this platform:

dwds|lib/src/utilities/shared.dart
dwds|lib/src/handlers/injector.dart
dwds|lib/src/servers/devtools.dart
dwds|lib/src/handlers/dev_handler.dart
dwds|lib/src/utilities/sdk_configuration.dart
dwds|lib/src/servers/extension_backend.dart
dwds|lib/src/services/debug_service.dart
dwds|lib/src/services/chrome_proxy_service.dart
webkit_inspection_protocol|lib/webkit_inspection_protocol.dart
package_config|lib/src/util_io.dart
package_config|lib/src/package_config_io.dart
package_config|lib/src/discovery.dart
package_config|lib/package_config.dart
file|lib/src/io.dart
http_multi_server|lib/src/multi_headers.dart
http_multi_server|lib/src/utils.dart
http_multi_server|lib/http_multi_server.dart
dds|lib/dds.dart
dds|lib/src/devtools/handler.dart
dds|lib/src/dds_impl.dart
devtools_shared|lib/src/server/file_system.dart
devtools_shared|lib/src/server/usage.dart
devtools_shared|lib/src/server/server_api.dart
usage|lib/src/usage_impl_io.dart
shelf_static|lib/src/static_handler.dart
shelf_static|lib/src/directory_listing.dart
shelf|lib/shelf_io.dart
shelf|lib/src/io_server.dart

https://github.com/dart-lang/build/blob/master/docs/faq.md#how-can-i-resolve-skipped-compiling-warnings

[INFO] Running build completed, took 358ms
[INFO] Caching finalized dependency graph completed, took 116ms
[INFO] Reading manifest at build/.build.manifest completed, took 0ms
[INFO] Deleting previous outputs in `build` completed, took 25ms
[INFO] Creating merged output dir `build` completed, took 18ms
[INFO] Writing asset manifest completed, took 0ms
[INFO] Succeeded after 538ms with 87 outputs (163 actions)

annagrin avatar Sep 15 '22 17:09 annagrin

/cc @fishythefish @rakudrama appreciate your advice!

annagrin avatar Sep 15 '22 17:09 annagrin

I don't think this is something dart2js does. The error message points to dart-lang/build, so that's probably what's producing the error, and you might find someone more knowledgeable on that issue tracker. I'm not sure which SDK dependency it thinks is unsupported on web, but does using generate_for (see the FAQ link) fix the problem?

fishythefish avatar Sep 15 '22 18:09 fishythefish

does using generate_for (see the FAQ link) fix the problem?

Looks like the link explains how to not generate the files, but we actually want to generate them (and were able before) so the failure seems erroneous.

/cc @jakemac53 could you please help redirect the issue? Should it be moved to build repo?

annagrin avatar Sep 19 '22 21:09 annagrin

The relevant message in the logs is this:

[WARNING] build_web_compilers:entrypoint on web/background.dart:
Skipping compiling extension|web/background.dart with dart2js because some of its
transitive libraries have sdk dependencies that are not supported on this platform:

There is a transitive import to dart:io (well, actually several). We don't allow that in build_web_compilers (even though dart2js does, it was really only a hack for flutter web, and we decided not to implement the same).

You can use import_path if you are having trouble understanding where the imports of non-web libraries are coming from.

Once you identify the problematic imports, they need to be converted to conditional imports (might just import a throwing stub on the web, the apis already couldn't possibly work).

jakemac53 avatar Sep 26 '22 15:09 jakemac53

@jakemac53 thanks, will do that. I am a bit confused here though - about how is the dart:io included when we share the batched_stream.dart with dwds and not included when we use the copy of the same file. Btw the error message mentions several files that are not reachable from the batched_stream.dart or from background.dart:

dwds|lib/src/utilities/shared.dart
dwds|lib/src/handlers/injector.dart
dwds|lib/src/servers/devtools.dart
dwds|lib/src/handlers/dev_handler.dart
dwds|lib/src/utilities/sdk_configuration.dart
dwds|lib/src/servers/extension_backend.dart
dwds|lib/src/services/debug_service.dart
dwds|lib/src/services/chrome_proxy_service.dart
webkit_inspection_protocol|lib/webkit_inspection_protocol.dart
(and more)

I wonder how we end up compiling them with the background.dart? Is it because the same file is used in dwds and compiled together with the that list of files in that case? I wonder why the same does not happen for other shared files like package:dwds/data/devtools_request.dart etc.

annagrin avatar Nov 30 '22 18:11 annagrin

@annagrin do you have src imports? Generally, if you use src imports (outside of your lib dir), then the module structure we create can end up containing libraries you didn't actually import. We use the public imports of a package to determine how to structure the modules.

jakemac53 avatar Nov 30 '22 18:11 jakemac53

yes we do! will see if I can eliminate them, thanks!

annagrin avatar Nov 30 '22 19:11 annagrin

I am not sure if it still exists but we also used to support a fine grained module strategy which wouldn't have this problem (although it would compile much slower). Basically we would only group libraries into a module based on strongly connected components and not try to infer anything based on public imports.

jakemac53 avatar Nov 30 '22 21:11 jakemac53

I think you could configure that strategy for your package with something like this (untested):

targets:
  $default:
    builders:
      build_web_compilers:ddc_modules:
        options:
          strategy: fine   

jakemac53 avatar Nov 30 '22 21:11 jakemac53

Not using src imports fixes the issue, thanks @jakemac53!

annagrin avatar Feb 07 '23 18:02 annagrin