-
Notifications
You must be signed in to change notification settings - Fork 231
Pub run should support passing in flags to vm #1018
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
Comments
<img src="https://avatars.githubusercontent.com/u/46275?v=3" align="left" width="48" height="48"hspace="10"> Comment by munificent Relatively soon, I want pub run to change from spawning a process to spawning an isolate for the entrypoint being run. (See: https://code.google.com/p/dart/issues/detail?id=19438). If that happens, we wouldn't be able to have separate VM flags for the spawned app. Would it make sense to spawn the pub process itself with these settings? That would mean the user is also implicitly debugging pub. This could be good if they want to see what transformers are doing, but may be confusing otherwise. What do you think? cc @nex3. |
<img src="https://avatars.githubusercontent.com/u/2156198?v=3" align="left" width="48" height="48"hspace="10"> Comment by kasperl Added this to the 1.6 milestone. |
<img src="https://avatars.githubusercontent.com/u/46275?v=3" align="left" width="48" height="48"hspace="10"> Comment by munificent Removed this from the 1.6 milestone. |
<img src="https://avatars.githubusercontent.com/u/3766663?v=3" align="left" width="48" height="48"hspace="10"> Comment by nicolasgarnier I would also need this for the test runner to be able to gather code coverage data from tests. I guess that would still be OK with isolates though just a bit more handling/filtering to do. |
<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd Does the test runner run via |
<img src="https://avatars.githubusercontent.com/u/3766663?v=3" align="left" width="48" height="48"hspace="10"> Comment by nicolasgarnier The test runner runs Standalone VM test (aka non Browser tests) using "pub run" and browser tests using "pub serve" that allows us to make sure eventual transformers are applied without having to run "pub build" first. |
<img src="https://avatars.githubusercontent.com/u/3766663?v=3" align="left" width="48" height="48"hspace="10"> Comment by nicolasgarnier By the way it would be great if we could also have "pub run" run the VM in un-checked mode (currently it always runs in checked mode). I'm asking about that here because it seem related (you could just make that a flag you optionally pass to the underlying VM) |
<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3 When we add support for this, we'll definitely allow explicit control of checked mode as well. |
<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd |
<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd Possibly blocked by https://code.google.com/p/dart/issues/detail?id=21791 ? |
Without ability to pass |
@alexander-doroshko FYI: there is a tracking bug on the test package for this: dart-lang/test#50 |
Turning on Observatory and configuring it is blocked on dart-lang/sdk#23320. |
But issue dart-lang/sdk#23320 is closed and labeled as not planned on Apr 26. So does that mean we are not going to see debugging support of the |
@kasperpeulen We have work to align our VM work with test package – but we will have great debug support in pkg/test. 😄 |
Guys we are a year on... Debugging VM test is still not sorted.... Is there an update is there a plan? |
Yes,
This request is tracked as #1204. 2nd approach looks a bit better to me, but I'm not from the Dart team and do not make decisions. |
@rayk: The issue is marked as blocked. Nothing is going to move forward here until it's unblocked.
@alexander-doroshko This is specifically tracking the addition of the functionality provided by VM options, which is an additional feature (or collection of features) that will need to be implemented on top of #1204. |
Any progress on this issue ? |
Have you tried |
@kevmoo thanks I didn't try this env variable and it seems to work. |
We don't have plans of implementing an option other than |
@natebosch I used |
That does sound like a valid use case. Re-opening but note that it doesn't necessarily mean we'll find the time to do this, just that I think it's reasonable for the issue to stick around. :) |
Do you have an idea of how it could work? Is it contributable? I also can't guarantee time; but it bugs me that the test runner in VS Code is only going to be enabled for Flutter by default (behind a flag for Dart) because of a few issues like this, so I'm motivated to try and fix that :-) |
I'm unsure of If you have a use case where you can't avoid |
Sorry, maybe I described it badly. The test is calling Although there may be workarounds for my specific tests; I'm not confident in shipping things on-by-default for end uses with lots of edge cases and caveats (especially when the failure can be hard to debug - it took me a while to figure out my test timeouts where caused by being inside my test runner!). |
Ah yeah, a |
Yeah; which is reasonable enough for me to do in these specific tests because they're mine and it's in test code, but doesn't make a great caveat for end-users (if you have tests that call pub/flutter, or even call production code that calls pub/flutter, you need to mess with the env vars) :( |
@munificent @natebosch Any thoughts on this?
|
I don't know of a way to make this work. Today we invoke the executable in an Isolate and can't pass different VM args to an isolate. We'd need to do something like spawn an entirely separate process in the case where there are args. There will be some runtime and memory overhead. |
There are also semantic differences between running in an isolate and running in a subprocess, with respect to how the OS treats the executable and especially how its signals are handled. |
Does inheritStdio resolve most of those differences? I think I remember seeing signals getting handled by the subprocess. |
I don't believe it affects signal handling, but I haven't tested extensively. |
I think my request is just to let me pass that as a flag to pub instead (or alternatively, wipe out that env variable when spawning the VM so it's unset for the VM and child processes)? |
Hmm, though now I'm wondering why I can't just run the VM and pass the pub snapshot... I did try that previously, but it was when trying to bypass the runner so possibly the reason I gave up on it doesn't apply here. I can't see anything complicated in the |
Well, Unless anyone has a good reason not to, I'm going to ship like this (and remove the preview flag on using the test runner for VM tests) in the next version of Dart Code. |
The link to the change is dead - can you update it? |
Rebase strikes! It's here (until it gets rebased again; it's not merged yet because of CI badness): Dart-Code/Dart-Code@b4d80e0#diff-2c4d134c0921e50e0f44bfbf9be83300L37 It's fairly simple though; just swaps |
That seems like a pretty reasonable approach. It's sensible that when you're invoking pub in this way you want to have full control and invoke |
I thought some more about this and I think we're not going to be able to solve this in a way that makes everyone happy. We could clear out We could add support for arbitrary flags but we'd need to fall back to a subprocess VM instead of an isolate, but as pointed out above that has consequences around signal handling etc. I think in the end We might not get that implemented right away but we also have some existing workarounds mentioned in this thread so I don't think there is any action for us to take on this issue. |
<img src="https://avatars.githubusercontent.com/u/2192312?v=3" align="left" width="96" height="96"hspace="10"> Issue by keertip
Originally opened as dart-lang/sdk#19569
A typical invocation of the vm to run command line apps from DartEditor would look like
dart --enable-checked-mode --debug:50677 --enable-vm-service:50678 --trace_service_pause_events --pause-isolates-on-start myapp.dart
and any other that the user may specify
DartEditor should be able to specify arguments to pass on to the vm when starting pub run from the editor.
The text was updated successfully, but these errors were encountered: