.packages: Ability to pass packages config data into Isolate.spawnUri

[sethladd]

To support the new .packages file and issue #23369 , we would like to ask if we can pass the data from that file into Isolate.spawnUri. Currently, Isolate.spawnUri takes a named parameter for packageRoot.

[sethladd]

cc @nex3 to help with any use cases that might help with the API design.

cc @lrhn

[iposva-google]

Since this is changing the API of the Isolate.spawnUri and the plumbing of this into the VM loading code will be complicated, it is highly not likely to land in 1.12 for the VM implementation. As the VM is the only implementation where this feature really matters, I suggest to move the milestone to reflect reality.

[sethladd]

Thanks for the update.

@nex3 what would be the impact if this doesn't land for 1.12 ?

[nex3]

I'm fine if this doesn't make it into 1.12. I just wanted to make sure it was being tracked as part of #23372.

[nex3]

Oh, I thought of one case where this will be necessary: we will need this before we can do dart-lang/pub#1204, which is something I'd like to see done by 1.13.

[sethladd]

Let's push hard to get this in by 1.13.

[lrhn]

We should do this, and the parameter should be Map<String, Uri> packages. It will have the same disclaimers as the packageRoot parameter (may not work if the isolate code was pre-compiled).

[sethladd]

Thanks @lrhn . Looking forward to it.

[nex3]

We should do this, and the parameter should be Map<String, Uri> packages. It will have the same disclaimers as the packageRoot parameter (may not work if the isolate code was pre-compiled).

This may also be useful, but it's not sufficient. We need to be able to pass in URIs to the files generated by pub; it would be a problem if everyone had to read them into memory and parse them before using them to spawn an isolate.

[lrhn]

I'm not sure I understand the objection - is the problem that you don't want to read in the ".packages" files generated by pub? If so, I don't think that will be a problem - we have a package for doing that. We can even extend that package with ways to run Dart scripts directly, something like:

Future<Isolate> runScript(Uri script, [List<String> arguments, Object parameter]);

which will use the correct search protocol to look for .packages and packages/ around the script and run it with the appropriate parameters.

In other words, I think the problem can be handled by a package without extending the core Isolate.spawnUri API.

[nex3]

Why wouldn't you want to pass in a URI? That API is simpler, it's consistent with packageRoot and the --packages flag, it doesn't require the user to depend on an extra package, it uses the information users are likely to have in practice as opposed to derived information, and it's faster and more consistent to use the VM's package spec parser. A URI is certainly the best-fit API for pub; do you have any real-world examples of code that would find a map easier to use?

[lrhn]

I don't have any real-world examples.

I do think that using a pre-existing Dart application is a somewhat common usage, and we should support it. In that case, you shouldn't even pass a URI, you should just say "run as application/script" and it will find the ".packages" file itself. I'd make a third spawnScript method on Isolate for that - like spawnUri but without passing packageRoot or packages because that's handled automatically.

That leaves spawnUri for the case where you have found the packages computationally. In that case it's an overhead to create a file and let the system read it back in, so a map from package name to location Uri is the best internal representation.

So, I'm against passing a URI because it's either too much or too little for the usages that I actually predict.

[nex3]

I don't have any real-world examples.

I believe this is because, in practice, there isn't Dart code that's interested in spawning isolates using a package spec that's generated from whole cloth.

I'd make a third spawnScript method on Isolate for that - like spawnUri but without passing packageRoot or packages because that's handled automatically.

This would also be useful, but it doesn't address a lot of the code that's actually using isolates in practice. A common use case is to spawn a script that isn't located in the current package using the current package's dependencies. Both pub and test do this, and none of the APIs you're proposing handle it gracefully.

So, I'm against passing a URI because it's either too much or too little for the usages that I actually predict.

We don't have to predict or guess how people will use Isolates. We know how people use Isolates, because they're using them today. I use them all the time. And I'm saying it's important for actual practical use-cases to be able to spawn an isolate for code in an arbitrary location using an on-disk .packages file. It's not useful for actual practical use-cases to be able to generate an in-memory package spec.

[lrhn]

If we have only one parameter for specifying the package resolution, then it should be the most general one. That is the map parameter. With that, the other use-cases can be implemented by a package that we already have, either direclty in package_config, or possibly in the isolate package using package_config.

[nex3]

Both parameters are expressively equivalent. Just like you can parse a package spec on disk into a map in memory, you can serialize a map in memory into a spec on disk. Given this equivalence, we should choose the parameter that satisfies all the known practical use-cases easily and efficiently.

[iposva-google]

Natalie, your second statement is incorrect on a read-only file system. As Lasse mentioned, one can be built using the other, but not the other way around.

[nex3]

Presumably data: URIs could work though—and probably should, for consistency with the first argument Isolate.spawnUri. If they don't it seems like that's a way better solution to dynamically generating a package spec—which, I emphasize again, has no known real-world uses.

[lrhn]

Since this is new, we have exactly one real-world use. We should not generalize from one sample.

We should use the same guiding principles that we have used in existing API design. For spawnUri we have added parameters to match command-line switches, but we have used meaningful Dart representations of the data instead of mirroring the command line's flat string structure. That's why the arguments is a List, not a single string which would be split on spaces, and the package-root is a Uri, not a String. For the same reason, the package-spec should be a Dart mapping from package name to package base URI, not the string contents of a file or the path to a file - those are not first-class Dart values for the data.

And as Ivan points out, if you can write a file, then you can also read the file and parse it (and we already have code to do that), but if you have the data, you can't necessarily write a file. The structured-Dart-data parameter is the most general type.

[nex3]

Since this is new, we have exactly one real-world use. We should not generalize from one sample.

Isolates aren't new, and packages aren't new. It's been possible (with a little work) to start an isolate with a set of packages generated on the fly for more or less the entire public lifetime of the Dart project and no one has done so, despite a number of different applications using Isolates. Package specs don't bring any new expressive power to the table here.

We should use the same guiding principles that we have used in existing API design. For spawnUri we have added parameters to match command-line switches, but we have used meaningful Dart representations of the data instead of mirroring the command line's flat string structure. That's why the arguments is a List, not a single string which would be split on spaces, and the package-root is a Uri, not a String. For the same reason, the package-spec should be a Dart mapping from package name to package base URI, not the string contents of a file or the path to a file - those are not first-class Dart values for the data.

A string versus an argument list is a small difference in representation—and anyway, the executable itself receive the arguments as a list from the shell. A URI versus a manually-parsed file is a huge difference in terms of user effort, affecting their dependency tree and the speed of their program.

And as Ivan points out, if you can write a file, then you can also read the file and parse it (and we already have code to do that), but if you have the data, you can't necessarily write a file. The structured-Dart-data parameter is the most general type.

We consistently support data: URIs throughout the rest of our platform. Why not use them here?

[mit-mit]

Hello, who is looking at this for 1.13?

[iposva-google]

Out for review: https://codereview.chromium.org/1403693002/

[iposva-google]

Landed with 60eab65

[dgrove]

It seems that this was closed without really solving the use case. I'd like to see this reopened with more discussion with the stakeholders.

[lrhn]

There are plenty of suggestions on how to solve the use-case (in particular, anything that has a package mapping in a file can read that file using package:pacakge_config and get a map that is suitable as a parameter to spawnUri).
Unless there are new use-cases, I'm not sure what you want to discuss.

[peter-ahe-google]

@lrhn How do you find the package mapping?

Also, how do you pass the package mapping to dart2js or any other command-line tool?

[mit-mit]

@peter-ahe-google, have you seen the resource package? https://codereview.chromium.org/1387163002/

[lrhn]

@peter-ahe-google
Which package mapping? The one used by the current isolate (Isolate.packageMap) or the one passed on the command line to the current Dart VM process (Platform.packageMap, CL being written, name may change)?

To pass a mapping on the commend line to dart2js, you need to have a file. If you don't know of one that contains what you need, you can write one and use that. We have tools to help with that.

[peter-ahe-google]

@lrhn I'm looking for the name of the file that was passed to the Dart VM.

For example, let's say I have these two programs:

package_root.dart:

import "dart:io";

main() {
  ProcessResult result = Process.runSync(
      Platform.resolvedExecutable,
      <String>["-p", Platform.packageRoot, "my_test_program.dart"]);
  print("""
Exit code: ${result.exitCode}
=== stdout ===
${result.stdout}
=== stderr ===
${result.stderr}
""");
}

my_test_program.dart:

import "package:expect/expect.dart";

main() {
  Expect.equals(1, 1);
}

How do I convert this to use packages files?

[lrhn]

@peter-ahe-google That requires the yet-to-be-committed Platform.packageMap (or whatever its name ends up being).

import "dart:io";
main() {
   var resolveType, resolveUri;
   if (Platfrom.packageRoot != null) {
     resolveType = "-p";
     resolveUri = Platform.packageRoot;
  } else if (Platform.packageMap != null) {
     resolveType = "--packages";
     resolveUri = Platform.packageMap;
  } 
  ProcessResult result = Process.runSync(Platform.resolvedExecutable,
          <String>[resolveType, resolveUri, "my_test_program.dart"]);
  ....

But this is digressing - this issue was about providing a package mapping to spawnUri, which is now possible.

[peter-ahe-google]

This isn't digressing. You're claiming that you have solved all use cases by adding support for taking a Map but not provided support for passing a file name. In addition, you've chose to use the name "packageMap" to mean a Map<String,Uri> in Isolate.spawnUri, and apparently Platform.packageMap is going to be a String.

This brings us to the questions of how do you get the package map in an isolate that has been spawned with spawnUri and a package map. Perhaps you intend to use data URLs for that, and that may work. But in that case Platform.packageMap should return a URI.

I think there's a mismatch between how you specify packages to to Isolate.spawnUri and to a command-line tool, and there seems to be no compelling reason for this mismatch.

[lrhn]

I claim to have solved one use-case: Starting a new isolate with a different package resolution than the spawning isolate with a per-package mapping instead of a single package root (which we already had).

The naming is packageMap in Isolate, it likely will not be that in Platform, the naming is being actively discussed.

Getting the package map of an isolate that has been created using Isolate.spawnUri with a packageMap parameter is done using Isolate.packageMap. It returns the same type as the argument - a map. I do not plan to use data: URIs for anything, they are, at best, a hack someone can use on a command line instead of creating a file, but it's not something I'd recommend.

Platform.packageMap/packagesFlag/whatever returns a String. It is the value of the --packages flag on the command line, nothing more, nothing less.

There is a mismatch between how you provide structured data to a programming API and a command line because a command line is a horrible API for structured data. That's why we put the data in a file, so the first package mapping can be read from there. When you are writing Dart code, and staying inside Dart, you can use a Map, which is, in my opinion, a suitable level of abstraction. We should not lower ourselves to what fits on a command line when creating APIs.

Getting from a map to a .packages file is a matter of serializing the map and writing it to a file, then you can have a URI pointing to that file if you need that for starting a command line tool. We have a package that can help you doing that.

[peter-ahe-google]

I think that this API is unnecessarily complicated due to the addition of Maps, and I'm having a hard time seeing how this adds value to dart2js which won't be able to support it.

However, I can now see how my use cases are solved:

I use Isolate.packageMap if I want to call Isolate.spawnUri using the same packages config as the current VM.

I use Platform.packageMap (or whatever it is called) for external processes.

I'm simply out of luck if I modify the packageMap and pass it to Isolate.spawnUri and try to use Platform.packageMap in the new isolate. But as luck would have it, I don't plan to do so. I'll just keep my fingers crossed that nobody decides to make Platform.packageMap return null in isolates that have been spawned using spawnUri.

[lrhn]

We had to pass a mapping to spawnUri one way or another, either as an actual map, or as a string that can be parsed as a map, or as a URI to a file that can be read and parsed as a map. I don't think adding more indirections would make the the API less complicated. The current API is direct and easy to understand.

You can easily start a new Isolate with the same package resolution as the current Isolate (but then, you never needed a parameter for that, that is the default), and you can start a new process with the same command line arguments as this process.

If you want to take the resolution of the current isolate (basically a kind of reflection), modify it, and then provide that as a command line parameter, then, yes, you need to create a new file - just as you would if you wanted any other new mapping that there is no file for. That's not really surprising.

The only remaining case is to spawn a new isolate with a package mapping that currently exists in a file. You will have to convert that file to a map instead of just pointing to it. We have helper functions that can do that for you, so instead of perhaps writing Isolate.spawnUri(...., packagesFile: myUri) you have to write Isolate.spawnUri(..., packageMap: loadPackagesFile(myUri).asMap()).
I think that's better than having two parameters for the same thing.

[peter-ahe-google]

A simpler solution than having two parameters would be to only have on parameter: a file name which gives you the location of a .pacakges file. The indirection you talk about is an implementation detail, and not a convincing argument for requiring people to use another package. It also leaves the semantics of Platform.packageMap (or whatever it's called) somewhat problematic in a spawned isolate.

[nex3]

in particular, anything that has a package mapping in a file can read that file using package:pacakge_config and get a map that is suitable as a parameter to spawnUri

This is a serious amount of extra effort for very little added value. Not only is it vastly less common to want to do dynamically-generated package resolution, it's a lot easier and more reliable to create a data URI than it is to read a file from disk. The current API optimizes for the wrong use-case.

The current API also gets in the way of solving #24524. If the creator of an isolate knows the location of the package spec file, it should be easy for code in the isolate to re-use that file. If we pass in a map, we're just dropping that information on the floor.

Unless there are new use-cases, I'm not sure what you want to discuss.

As one of the primary customers and the person charged with ensuring that users can continue to run their applications successfully using pub run, I'm not satisfied with the API as it stands. It sounds like another customer, Peter, isn't either. If that's not enough reason to re-evaluate it, I don't know what is.

[peter-ahe-google]

I just realized another problem with using a Map. Currently, the type of packageMap in Isolate is:

Map<String, Uri> packageMap

This means that we'll be hard pressed if we decide to add more features to the package configuration in the future. This would be equivalent to defining an isolate like this:

Map<Uri, Map<String, Class>> libraries

And then later realize that we want top-level methods.

For this reason, the type of packageMap should be an identifier (a URI) or an opaque object (PackageConfiguration).

Here's a couple of things that would be hard to add while maintaining the map interface:

• Allowing multiple versions of the same package, for example, when package A says package:P it should resolve to p1 and when package B says package:P it should resolve to p2.
• Specifying that an isolate may not use certain features, for example, dart:io or dart:mirrors.

Regardless of your opinion of these various features, you need to provide a future-proof API.

[iposva-google]

Creating a future-proof API does not happen overnight. @mit-mit : We should drop this from 1.13.

[mit-mit]

I believe this has landed