Lookup the package config location for Dart and Flutter test targets

[kenzieschmoll]

This depends on dart-lang/test#2246 being resolved and all test bootstrap generators being updated with the new changes to add a packageConfigLocation constant.

[kenzieschmoll]

@jakemac53 @natebosch @bkonyi is this backwards compatibility required? DevTools is pinned to the Dart SDK version. Is package:test also? Or is it possible for a user to be on a new Dart SDK with these DevTools changes, but an old version of package:test that does not have the generated constant we are trying to eval?

[natebosch]

Yes, it's possible for a new SDK to be used with an old package:test. We should not rely on these always being present. We also should be resilient if the strings have the content 'null' in case these code paths are used in places where Isolate.packageConfig returns null.

[kenzieschmoll]

Sounds good. The current code covers that case, so we should be good to go. In the case you described where Isolate.packageConfig returns 'null`, would we still want to fallback to the case where we are checking the test import prefix? From the DevTools perspective, I don't mind more fallbacks that if it allows us to provide a more robustness experience to the user.

[natebosch]

would we still want to fallback to the case where we are checking the test import prefix? From the DevTools perspective

I don't know of any concrete reasons for getting a null, so it's hard to say whether the normal fallback behavior would help or not. In any case I don't think it hurts to try a fallback approach for this situation. I mentioned it because the API is nullable and we are explicitly not doing anything special with it there. If it would be useful to omit the variable entirely over writing the string 'null' we could do that too.

[kenzieschmoll]

Let's leave the variable even if it writes null. If the variable is missing, the evaluation would throw, so I think it is better to just rely on our string comparison to cover this case.

[jakemac53]

Note that the name/location of this file is not guaranteed to be consistent. This is just the default path, and it could be anything in practice.

[jakemac53]

Consider also the pub workspaces feature, I am not sure if this will work well with that or not.

But, in the future it will be common externally to have a .dart_tool/package_config.json file which lives in a directory above the actual package root (not sure if that will matter for this or not).

[kenzieschmoll]

What is an example where the package config will not be named package_config.json? It is my understanding that much of our tooling system, pub, etc. rely on this assumption.

Even with a pub workspace, shouldn't the URI to the package config still end with .dart_tool/package_config.json? @sigurdm

Generally, I filed #7944 to track ensuring DevTools extensions work well with pub workspaces.

[jakemac53]

Any user or tool for any reason can pass any path they want for the --packages argument. Yes, most of the time this doesn't happen, but it is supported.

Specifically, build systems might create a file by any name, and pass the path to it that way.

[sigurdm]

Even with a pub workspace, shouldn't the URI to the package config still end with .dart_tool/package_config.json? @sigurdm

Yes with workspaces it will - but also what @jakemac53 said!

[kenzieschmoll]

I have removed these shared constants and filed #7944 to track moving away from the assumptions around .dart_tool/package_config.json

[jakemac53]

It is also worth noting that in the workspace case, the package config will contain all packages depended on by any package in the workspace, not just the current package.

I don't know exactly all the details here, but if DevTools is looking in that file for certain dependencies, assuming they are dependencies of the current package, then that assumption won't hold up (it will contain dependencies which are not transitive deps of the current package).

I don't know enough to suggest a fix for that though.

[kenzieschmoll]

It is also worth noting that in the workspace case, the package config will contain all packages depended on by any package in the workspace, not just the current package.

Noted. This will take some more thought and design as pub workspaces roll out.

[kenzieschmoll]

I will wait to land this until the bootstrapping changes have landed

• Write the package config location to the test bootstrap. flutter#150440
• Add package config URI into VM test entrypoints dart-lang/test#2245 (@natebosch will there be additional bootstrapping changes outside of this PR that I should wait for?)

[sigurdm]

It is also worth noting that in the workspace case, the package config will contain all packages depended on by any package in the workspace, not just the current package.

Noted. This will take some more thought and design as pub workspaces roll out.

I don't know how and why you are consuming the package config currently

But for deciding exactly which packages are dependencies of your package pubspec.yaml is the way to go.

If you want the transitive closure of the set of dependencies you will have to look up each dependency in turn in the package_config and load that pubspec.yaml and find the dependencies etc.

[kenzieschmoll]

It is also worth noting that in the workspace case, the package config will contain all packages depended on by any package in the workspace, not just the current package.

Noted. This will take some more thought and design as pub workspaces roll out.

I don't know how and why you are consuming the package config currently

But for deciding exactly which packages are dependencies of your package pubspec.yaml is the way to go.

If you want the transitive closure of the set of dependencies you will have to look up each dependency in turn in the package_config and load that pubspec.yaml and find the dependencies etc.

We use the package config file to pass it to the extension_discovery package, which finds extensions for every package dependency in the package config (both direct and transitive).

[jakemac53]

It is also worth noting that in the workspace case, the package config will contain all packages depended on by any package in the workspace, not just the current package.

Noted. This will take some more thought and design as pub workspaces roll out.

I don't know how and why you are consuming the package config currently
But for deciding exactly which packages are dependencies of your package pubspec.yaml is the way to go.
If you want the transitive closure of the set of dependencies you will have to look up each dependency in turn in the package_config and load that pubspec.yaml and find the dependencies etc.

We use the package config file to pass it to the extension_discovery package, which finds extensions for every package dependency in the package config (both direct and transitive).

As long as you are OK with finding some extra extensions for some of the workspace packages, you might not even need to do anything. I don't know what the cost of loading extra extensions is.

[natebosch]

• will there be additional bootstrapping changes outside of this PR that I should wait for?

We don't have any other specific changes planned at the moment. We expect there could be some use cases which are missing this variable, but we can respond to those places as they surface.