Proposal: Have `dart run <file.dart>` do resolution about sdk

[mit-mit]

Did I understand this correctly?

Current behavoir:

dart run <package>[:<executable>]    # resolves and then runs
dart run path/to/file.dart           # just runs
dart path/to/file.dart               # just runs

Suggested new behavoir:

dart run <package>[:<executable>]    # resolves and then runs
dart run path/to/file.dart           # resolves and then runs
dart path/to/file.dart               # just runs

[mit-mit]

OK, I think I'd do a breaking change for this. Given there is a work around (change dart run x to dart x), then should be relatively easy to get through.

[sigurdm]

cc @jonasfj

[jonasfj]

Is the procedure something like this:

• Start at the directory of the target (supposed you want to run path/to/file.dart, then start at path/to/)
• Walk up parent directory until we find either:
  • .dart_tool/package_config.json or,
  • pubspec.yaml.
• If we found .dart_tool/package_config.json and no pubspec.yaml, then dependency discovery is complete.
• If we have mtime(.dart_tool/package_config.json) > mtime(pubspec.lock) > mtime(pubspec.yaml), then
  • For each package in .dart_tool/package_config.json where package is not in PUB_CACHE:
    • Check that mtime(package:../pubspec.yaml) < mtime(.dart_tool/package_config.json), if not resolve dependencies with dart pub get.
  • Otherwise, discovery of dependencies is complete.
• If we found pubspec.yaml, but no .dart_tool/package_config.json or no pubspec.lock, then resolve dependencies with dart pub get.

[sigurdm]

Is the procedure something like this:

Yes I think that is close enough.

One thing I'm a bit unsure about is the priority og package config vs pubspec. Ie

• do we first search all the way to the file system root for a package config before looking for a pubspec,
• or do we search for the two kinds in parallel?

In the second case we risk having to parse yaml in order to decide if the pubspec we found is the right one. That's why I think we go with the first.

[sigurdm]

@bkonyi @mit-mit do you have opinions? Is this too breaking?
What kind of announcement would it require?

[sigurdm]

Yep!

[sigurdm]

With the detail that the resolution is not for the current working dir, but for the context ofpath/to/file (which doesn't have to be contained in the current working dir.)

[lrhn]

I'd look for both .dart_tool/package_config.json and pubspec.yaml at the same time.

If you find a pubspec.yaml and not package-config, it's a sign to run pub get to generate the package-config. Then use that.

If you find a package-config, but no pubspec.yaml, it's a sign of a hand-edited or other non-pub-managed package-config. Use that.

If you find both, it's the first case again, you may just not have to run pub get first.

So finding pubspec.yaml always signals a pub-managed package configuration. After that it's just about making sure it's up to date.
Finding a .dart_tool/package_config.json first, with no pubspec.yaml, is the self-managed case, which just happens to be what the Dart SDK repo uses.

(Will this cause pub get to be run in the SDK repo's pkg/whatever directories? Is there a way to opt out?)

[bkonyi]

Is the intention of this mostly to improve performance with snapshotting, to ensure packages are up to date, or both? For performance improvements, we're already in the process of exploring enabling the resident frontend server by default (cc @a-siva @derekxu16).

[sigurdm]

Is the intention of this mostly to improve performance with snapshotting, to ensure packages are up to date, or both?

My focus is on the package resolution. Snapshotting would just come as a bonus, but longer term I really want the snapshotting logic out of pub, as I think the VM has a lot more information available to do this efficiently.

Getting the resident frontend server on by default would be awesome!

[sigurdm]

I'd look for both .dart_tool/package_config.json and pubspec.yaml at the same time.

If you find a pubspec.yaml and not package-config, it's a sign to run pub get to generate the package-config. Then use that.

Unfortunately that doesn't work if that pubspec is part of a flutter.dev/go/pub-workspace, and you have to parse it to decide if that is the case.

[lrhn]

Then I guess you need to parse the pubspec.yaml before deciding whether to run pub get. Or at least grep for ^resolution:\s*workspace\s*$. Not awesome.
(Could we call it subspec.yaml instead, if it's part of a workspace?)

At least, if doing pub get in a workspace package fails, then we can react to that failure.

[sigurdm]

We have discussed a bit offline, and come to this solution:
pub get will leave a .dart_tool/pub/workspace_ref.json file next to each pubspec.yaml in a workspace.

It will be of the form:

{
  "workspaceRoot": "../.."
}

We then for each parent directory p of the file.dart:

• If p is the file system root: use an empty package config.
• look for $p/.dart_tool/package_config.json. If that exists check if it is up-to-date. (this allows a stand-alone package config).
• look for $p/pubspec.yaml:
  • If that exists: look for $p/.dart_tool/pub/workspace_ref.json.
    • If that exists: check if the .dart_tool/package_config.json relative to its "workspaceRoot" is up-to-date.
    • Otherwise: do full resolution
  • Otherwise continue to next parent directory.

[derekxu16]

Something that @a-siva had pointed out to me earlier is that dart run --reisdent path/to/file.dart currently behaves differently than dart run path/to/file.dart, because it does look for the nearest enclosing package_config.json:

sdk/pkg/dartdev/lib/src/generate_kernel.dart

Line 51 in 6e7aca7

final packageRoot = _packageRootFor(executable);

So I think it makes sense for your improved package resolution strategy to be implemented in dart run first, and then I will figure out how to implement the strategy in dart run --resident as well.

[sigurdm]

So I think it makes sense for your improved package resolution strategy to be implemented in dart run first, and then I will figure out how to implement the strategy in dart run --resident as well.

Ideally --resident should share the same resolution code as --no-resident? No?

[derekxu16]

Ideally --resident should share the same resolution code as --no-resident? No?

Yes, I will try to share as much code as possible between the two implementations.