Add bool checkedMode optional, named arg to spawn/spawnUri isolate functions

[nex3]

We'd like "pub run" to run its Dart files in an isolate rather than as a subprocess, both to avoid extra Dart VM startup time and to provide it with an accurate view of the stdio it's talking to via [stdioType]. We'd also like to run executables in checked mode when pub itself is not, and to allow users to provide various other VM flags for e.g. debugging. To support all of this, we need some sort of VM-specific means of spawning an isolate with different flags than the parent isolate.

[lrhn]

Added Area-Library label.

[nex3]

Marked this as blocking #21821.

[nex3]

This is also important for unittest. The unittest executable isn't run in checked mode, but the isolates it loads to run tests should be.

[kevmoo]

Removed Priority-Unassigned label.
Added Priority-High label.
Changed the title to: "Add bool checkeMode optional, named arg to spawn/spawnUri isolate functions".

[kevmoo]

Blocking dart-lang/test#50

---

Added this to the 1.11 milestone.

[kevmoo]

Ivan: please reassign. We'd love to have this for 1.11.

Please bring me into API discussions. We'd love other flags as well.

---

Set owner to @iposva-google.

[iposva-google]

Removed the owner.

[kevmoo]

Ivan mentioned supporting more that checked, but having checked at a minimum for 1.11 would be great for our test story.

---

Set owner to @sgjesse.

[sgjesse]

One thing to keep in mind is that for dart2js this will not work, so it will be specific to the actual Dart implementation. Therefore adding it as a named argument in the spawn/spawnUri might be too prominent.

Another option could be to add an optional "options" Map<String, String> argument where any arguments (e.g. '--checked' with value whatever) can be passed, and then it is implementation specific if it has any effect.

---

cc @lrhn.

[nex3]

I wouldn't mind an options-based approach.

[kevmoo]

I'd argue we do want a named argument.

An options map throws away a lot of tooling help.

setting 'checked' via a named arg or a map will be equally broken on dart2js. Anything another doesn't help.

---

Changed the title to: "Add bool checkedMode optional, named arg to spawn/spawnUri isolate functions".

[lrhn]

A Map<String,String> is not a good design. A better alternative would be a parameter object.
In this case, I don't have a problem with a parameter that doesn't always work, as long as it's documented that it is a request to spawn an isolate with that flag set if possible.

[iposva-google]

And what does this Parameter object contain? A set of flags like a command line?

[lrhn]

A Parameter Object is just an object with properties that can be passed to a function instead of passing the properties individually.

It's more useful in languages that don't have optional named parameters or default values because you can use the full power of objects instead of the, possibly limited, features available for function calls. You can have helper functions creating parameter objects pre-filled with certain combinations of arguments.

Another advantage of parameter objects is that you can extend them later without changing the signature the function, and you can pass extended parameter objects to old functions without them caring about the parameters they don't recognize.
That means that VM specific isolate flags can be added to a "VMSpawnParameters" class to a VM specific library, extending a default "SpawnParameters", and then passed to Isolate.spawnUri, without changing the signature - it's a hidden back-channel to send more parameters that might be recognized by the receiving function. Which is actually also kind-of icky :)

On the other hand, a parameter object should just be an object with a method that does what you want, instead of being a dumb object that is passed to a tightly coupled function. So the real solution would be "(new IsolateSpawner()..paused=true..packageRoot=something).spawnUri(uri)". Then you can extend IsolateSpawner if you want further functionality.

I don't think we want, or need, a parameter object here, but it beats a Map<String,String> any day.

[kwalrath]

cc @Sfshaza.

[sgjesse]

Set owner to @lrhn.

[sethladd]

Would love to be able to run my tests in checked mode, so greatly looking forward to this new API. Thanks for working on it!

[lrhn]

Added Started label.

[kevmoo]

@lrhn is this resolved as of 5a843eb ?

[lrhn]

I think so. @iposva-google landed a fix for the 5a843eb patch not really working, as f5e3f94.

[nex3]