Skip to content

Cannot use pub with --preview-dart-2 #32188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kevmoo opened this issue Feb 15, 2018 · 53 comments
Closed

Cannot use pub with --preview-dart-2 #32188

kevmoo opened this issue Feb 15, 2018 · 53 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-dart-2 P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@kevmoo
Copy link
Member

kevmoo commented Feb 15, 2018

> DART_VM_OPTIONS=--preview-dart-2 pub 
Invalid isolate state - Unable to make it runnable

On Dart VM version: 2.0.0-dev.26.0 (Thu Feb 15 03:06:13 2018 +0100) on "macos_x64"

@kevmoo
Copy link
Member Author

kevmoo commented Feb 15, 2018

CC @a-siva @nex3 @zanderso

@kevmoo kevmoo added customer-dart-2 area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Feb 15, 2018
@a-siva
Copy link
Contributor

a-siva commented Feb 20, 2018

in the dart sdk builds pub, analyzer, dart2js and ddc use an application snapshot to run, these snapshots are built in dart 1 mode. When preview-dart-2 is specified it is not compatible with these snapshots and hence the failure. The error message is not very helpful and we will fix that.

To get these command with --preview-dart-2 first the pub, dart2js, analyzer code needs to be made strong mode clean and these application snapshots need to be generated with that option turned on. I think we planned to do this in Q2.

The issue that the original bug wanted addressed was a way for folks to run tests in preview-dart-2 mode using pub. For this to work 'pub run' needs to implement the --preview-dart-2 option so that it starts the test process using this option, somewhat similar to the '-c' or '--checked' option that it currently supports.

sivalinuxmach[sdk]>out/ReleaseX64/dart-sdk/bin/pub run --help
Run an executable from a package.

Usage: pub run [args...]
-h, --help Print this usage information.
-c, --[no-]checked Enable runtime type checks and assertions.
--mode Mode to run transformers in.
(defaults to "release" for dependencies, "debug" for entrypoint)

@a-siva
Copy link
Contributor

a-siva commented Feb 20, 2018

Who needs to own this issue of implementing the option '--preview-dart-2' in 'pub run' ?
Not sure who owns pub these days.

@kevmoo
Copy link
Member Author

kevmoo commented Feb 20, 2018

@nex3 still does most of the work, although it's migrating slowly to AAR w/ @mit-mit driving

@mit-mit
Copy link
Member

mit-mit commented Feb 20, 2018

We have no plans, or bandwidth, to work on pub client in q1.

@dgrove
Copy link
Contributor

dgrove commented Apr 17, 2018

@nex3 @grouma @natebosch Can you all figure out what's needed here?

@dgrove
Copy link
Contributor

dgrove commented Apr 17, 2018

/cc @kmillikin - there are likely changes need to CFE to make it run on Dart 2.

@JekCharlsonYu JekCharlsonYu added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Apr 17, 2018
@jmesserly
Copy link

/cc @kmillikin - there are likely changes need to CFE to make it run on Dart 2.

FYI, I was able to locally run DDC and DDK with --preview-dart-2. All tests were passing. Also DDC/DDK are now building themselves as part of their snapshot process (selfhost for training data), and that compile succeeds with no errors. Based on that I believe Analyzer/CFE are pretty Dart 2 clean at this point. (There might be some runtime cast failures on code paths that DDC/DDK does not exercise).

To get these command with --preview-dart-2 first the pub, dart2js, analyzer code needs to be made strong mode clean and these application snapshots need to be generated with that option turned on. I think we planned to do this in Q2.

I haven't figured out how to build the DDC snapshots yet with --preview-dart-2. Would appreciate help with that. I've been chatting a bit with @a-siva

@nex3
Copy link
Member

nex3 commented Apr 17, 2018

Once dart-lang/pub#1871 lands, we won't need dart2js or DDC to be Dart 2-compatible anymore. We do still use the analyzer, though—is that Dart 2-clean?

@bwilkerson
Copy link
Member

There is at least one (external) method in dart:io that isn't, but all of the tests that don't rely on IO (which is the vast majority of them) pass. So, as far as I can tell based on our tests, yes, analyzer is Dart 2 clean. I'm sure we'll find a few bugs in code that isn't covered by tests.

@natebosch
Copy link
Member

I'll work on checking that pub run code paths work in Dart 2 runtime.

I'll likely need some help getting it set up in the pub wrapping script in the SDK.

@nex3
Copy link
Member

nex3 commented Apr 17, 2018

Is test Dart 2 clean? If so, we should be able to verify pub's cleanliness by running its tests.

If not, we can get part of the way by spawning the pub subprocesses the tests run with --preview-dart-2.

@natebosch
Copy link
Member

natebosch commented Apr 17, 2018

pub run seems to work when I fix the static errors.
dart-lang/pub#1874

However it's drastically slower.

Set up a package which has a single dependency:

dependencies:
  pub:
    path: ../pub

Invoke pub with no arguments - just prints usage and exits.

  • pub run pub - 1.36 s
  • dart ../pub/bin/pub.dart - 0.87s
  • dart ../pub/bin/pub.dart run pub - 1.76s
  • dart --preview-dart-2 ../pub/bin/pub.dart - 11.46s
  • dart --preview-dart-2 ../pub/bin/pub.dart run pub - 18.35s

It's possible that using a snapshot in Dart 2 semantics will be an improvement, and that this will carry over when using a hosted dependency rather than a path dependency...

@natebosch
Copy link
Member

I can't get timing data with snapshots - #32911

@natebosch
Copy link
Member

natebosch commented Apr 18, 2018

Here is the list of work that I think is blocking the end to end pub run --preview-dart-2 test scenario.

  • Sync pub into the SDK. Blocked on Move from pub build to dart2js for Observatory #32900
  • Set up the flags in the shell and bat script to get the --preview-dart-2 flag passed to the dart vm and not pub itself. (could skip this if we're ok with DART_VM_OPTIONS=--preview-dart-2 ux)
  • Ensure that package:test is Dart 2 runtime clean
  • Give pub an understanding of whether the run command is Dart 2 or not. Today it wants to run from snapshots when it can, and Dart1/Dart2 snapshots are incompatible (and today not possible for Dart2, blocked on Cannot run a snapshot with --preview-dart-2 #32911). The hacky thing to do is generate only 1 snapshot and run from source when using the "other" mode, the full solution would involve generating2 snapshots and picking the right one at runtime.

@nex3
Copy link
Member

nex3 commented Apr 18, 2018

Given that the running from source is so slow with the CFE right now, how do we verify that a given chunk of code is in fact Dart 2-clean?

@vsmenon
Copy link
Member

vsmenon commented Apr 18, 2018

@a-siva - note Nate's Dart 1 vs Dart 2 perf comment above.

@mit-mit
Copy link
Member

mit-mit commented Apr 20, 2018

This continues to cause friction when we work on making packages dart2 compatible. @a-siva is someone looking into the perf issues?

@kevmoo
Copy link
Member Author

kevmoo commented May 14, 2018

@vsmenon
Copy link
Member

vsmenon commented May 15, 2018

@a-siva - @natebosch 's cl is breaking on the bots with:

C:/b/s/w/ir/cache/vpython/1bda7b/Scripts/python.exe ../../build/gn_run_binary.py compiled_action dart.exe --deterministic --packages=C:/b/s/w/ir/cache/builder/sdk/.packages --snapshot=gen/pub2.dart.snapshot --snapshot-depfile=C:/b/s/w/ir/cache/builder/sdk/out/ReleaseIA32/gen/pub2.dart.snapshot.d --preview-dart-2 --snapshot-kind=script C:/b/s/w/ir/cache/builder/sdk/third_party/pkg/pub/bin/pub.dart
Command failed: ./dart.exe --deterministic --packages=C:/b/s/w/ir/cache/builder/sdk/.packages --snapshot=gen/pub2.dart.snapshot --snapshot-depfile=C:/b/s/w/ir/cache/builder/sdk/out/ReleaseIA32/gen/pub2.dart.snapshot.d --preview-dart-2 --snapshot-kind=script C:/b/s/w/ir/cache/builder/sdk/third_party/pkg/pub/bin/pub.dart
output: c:/b/s/w/ir/cache/builder/sdk/.packages: Error: Unable to read '.packages' file:
  FileSystemException(uri=c:/b/s/w/ir/cache/builder/sdk/.packages; message=File c:/b/s/w/ir/cache/builder/sdk/.packages does not exist.).

I'm seeing similar with my DDC CL:

Command failed: ./dart --deterministic --packages=/Users/vsm/dart/sdk/.packages --snapshot=gen/dartdevc.dart.snapshot --snapshot-depfile=/Users/vsm/dart/sdk/xcodebuild/ReleaseX64/gen/dartdevc.dart.snapshot.d --preview-dart-2 --snapshot-kind=app-jit /Users/vsm/dart/sdk/pkg/dev_compiler/bin/dartdevc.dart --dart-sdk /Users/vsm/dart/sdk/sdk --dart-sdk-summary /Users/vsm/dart/sdk/xcodebuild/ReleaseX64/gen/utils/dartdevc/ddc_sdk.sum --library-root /Users/vsm/dart/sdk/pkg/dev_compiler -o dartdevc.js /Users/vsm/dart/sdk/pkg/dev_compiler/bin/dartdevc.dart
output: Error while initializing Kernel isolate

Any ideas?

@keertip
Copy link
Contributor

keertip commented May 15, 2018

@vsmenon, are you running from xcodebuild/ReleaseX64/ directory?

@vsmenon
Copy link
Member

vsmenon commented May 15, 2018

Yes.

@vsmenon vsmenon assigned a-siva and unassigned natebosch May 16, 2018
@vsmenon
Copy link
Member

vsmenon commented May 16, 2018

@a-siva - I think we're blocked on this (and similar with DDC). It appears to be somewhat non-deterministic. Occasionally, DDC will build. Occasionally, it will fail with the "Kernel isolate" error. Occasionally (maybe just on bots), it will fail with the ".packages" error. Nate is seeing the same (we think) with pub.

@a-siva
Copy link
Contributor

a-siva commented May 17, 2018

This CL https://dart-review.googlesource.com/c/sdk/+/55589 fixes the .package not found error but now an assertion in the optimizing compiler is being triggered which I am investigating.

@alorenzen
Copy link
Contributor

Spoke to @JekCharlsonYu, this was moved to Stable by mistake.

@a-siva
Copy link
Contributor

a-siva commented May 17, 2018

https://dart-review.googlesource.com/c/sdk/+/55589 is being reviewed

@kevmoo
Copy link
Member Author

kevmoo commented May 18, 2018

Fixed in 02e47a9

@kevmoo kevmoo closed this as completed May 18, 2018
@kevmoo
Copy link
Member Author

kevmoo commented May 18, 2018

Spoke too soon. Waiting on https://dart-review.googlesource.com/c/sdk/+/54743 from @natebosch

@kevmoo kevmoo reopened this May 18, 2018
@kevmoo kevmoo assigned natebosch and unassigned a-siva May 18, 2018
@natebosch
Copy link
Member

@a-siva I'm still unable to build locally.

Error while initializing Kernel isolate

Were you able to reproduce this error, did your CL fix it for you?

@natebosch
Copy link
Member

the build error I'm seeing now might be flaky - it was for @vsmenon as well. I was able to get a successful build locally.

dart-bot pushed a commit that referenced this issue May 18, 2018
- Make a snapshot in Dart 2 mode
- Check for the VM argument `--preview-dart-2` and run the correct
  snapshot

Towards fixing #32188

Change-Id: I56d5e7f268ff40b80783fae571981705536280f2
Reviewed-on: https://dart-review.googlesource.com/54743
Commit-Queue: Nate Bosch <[email protected]>
Reviewed-by: Keerti Parthasarathy <[email protected]>
Reviewed-by: Kevin Moore <[email protected]>
@kevmoo
Copy link
Member Author

kevmoo commented May 18, 2018

Hopefully fixed in 2ff2af7

@kevmoo kevmoo closed this as completed May 18, 2018
@natebosch
Copy link
Member

We want dart-lang/pub#1683 to fix the performance issues, but we're unblocked to run things (from source) now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-dart-2 P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests