dart cli commands should run pub get

[mit-mit]

The flutter command always runs a pub get when a sub-command that needs a resolution is executed. The dart command doesn't. I think the Flutter behavoir is most user friendly, so suggest we align with that.

Flutter

Analyze:

$ rm .dart_tool/package_config.json
$ flutter analyze
Running "flutter pub get" in f1...                                 699ms
Resolving dependencies...

Build:

$ rm .dart_tool/package_config.json
$ flutter build apk
Running "flutter pub get" in f1...                                 714ms
Resolving dependencies...

Run:

$ rm .dart_tool/package_config.json
$ flutter run -d chrome
Running "flutter pub get" in f1...                                 200ms

Dart

The dart command does this for test:

mit-macbookpro6:app1 mit$ rm pubspec.lock
mit-macbookpro6:app1 mit$ dart test
Resolving dependencies in /Users/mit/tmp/app1... (1.1s)

But not for analyze:

$ rm .dart_tool/package_config.json
$ dart analyze
Analyzing app1...                      0.7s

  error • bin/app1.dart:1:8 • Target of URI doesn't exist: 'package:app1/app1.dart'. Try creating the file referenced by
          the URI, or try using a URI for a file that does exist. • uri_does_not_exist

Or for compile:

$ rm .dart_tool/package_config.json
$ dart compile kernel bin/app1.dart
Compiling bin/app1.dart to kernel file bin/app1.dill.
Info: Compiling with sound null safety.
Error: Couldn't resolve the package 'app1' in 'package:app1/app1.dart'.

Or for migrate:

mit-macbookpro6:app1 mit$ dart migrate
Migrating /Users/mit/tmp/app1

See https://dart.dev/go/null-safety-migration for a migration guide.

Analyzing project...
[------------------------------------------------------------------------------------------------------------------------------|]
8 analysis issues found:
  error • Target of URI doesn't exist: 'package:app1/app1.dart' at bin/app1.dart:1:8 • (uri_does_not_exist)

Or for run:

mit-macbookpro6:app1 mit$ dart run bin/app1.dart
../../.pub-cache/hosted/pub.dev/file-6.1.2/lib/src/interface/file.dart:15:16: Error: The method 'File.create' has fewer named arguments than those of overridden method 'File.create'.
  Future<File> create({bool recursive = false});

[mit-mit]

cc @bkonyi @devoncarew @jonasfj

[lrhn]

Is there some way we can make pub get be faster? Say a --quick-check flag which makes it no do anything if the .dart_tool/package_config.json exists and was created later than the latest modification of pubspec.yaml (or however it can quickly get a hint that it shouldn't do anything).

Running dart pub get in a very minimal project, which just depends on test, takes ~three second for me.
Doing that every time I try to do dart run would be very annoying.
(I assume it wouldn't print the output of dart pub get, because then it would be extra annoying.)

[mit-mit]

Yes, for run maybe we just check that:

• both pubspec.lock and .dart_tool/package_config.json exist on disk
• both have time stamps newer than pubspec.yaml
• the generatorVersion tag in matches the version of the current SDK

?

[jonasfj]

We have an assertUpToDate which has some rather subtle logic for testing if package_config.json is reflection of pubspec.lock, and whether pubspec.lock is a resolution to pubspec.yaml.

https://github.com/dart-lang/pub/blob/a73598b5f36cf32c2d498391c6b9e7b015c02a4f/lib/src/entrypoint.dart#L564

Doing that every time I try to do dart run would be very annoying.

dart run already has this behavior, when the input is dart run <package>[:<command>], for some reason it doesn't happen when doing dart run file.dart.

$ dart create myfoo
$ cd myfoo
$ rm -rf .dart_tool/
$ dart run myfoo
Resolving dependencies in /tmp/myfoo... 
Got dependencies in /tmp/myfoo!
Building package executable... 
Built myfoo:myfoo.
Hello world: 42!

Running dart test also has this behavior.

---

The downside of doing this everywhere is that: using dart for things that don't have a pubspec.yaml will become increasingly hard.

Today, we have reports that dart test won't work in the pkg/<package>/ folder of the Dart SDK. Are we confident we can further break these work-flows 🙈

What is the semantics if there is no pubspec.yaml? should we look in parent directories? I don't think we do this today.
Is cd test/; dart foo_test.dart allowed (there is no pubspec.yaml in current directory)? Today, I don't think dart run <package>[:<command>] works if not running next to the pubspec.yaml.
Should we have this behavior for dart file.dart too?

[mit-mit]

We should probably special cases commands where a --packages flag is passed to not call get in that case (passing the file implies that you already have one).

[devoncarew]

The flutter command always runs a pub get when a sub-command that needs a resolution is executed. The dart command doesn't.

I'd like to tweak that to say that the change in behavior we want is to:

• run dart get automatically for more of the commands, (as above) and
• run dart get automatically after a SDK major version upgrade

It's that 2nd part that would help with https://github.com/google/file.dart/issues/204 - getting a new version of package:file after an SDK upgrade. Does that sounds right?

Here's the list of the current dart commands:

Command	Description	Needs pub get if out-of-date	Needs pub get on SDK upgrade
analyze	Analyze Dart code in a directory.	?	?
compile	Compile Dart to various formats.	yes	yes
compiler-server-shutdown	Shut down the Resident Frontend Compiler.	no	no
create	Create a new Dart project.	no	no
devtools	Open DevTools (optionally connecting to an existing application).	no	no
doc	Generate API documentation for Dart projects.	yes	yes
fix	Apply automated fixes to Dart source code.	yes	yes
format	Idiomatically format Dart source code.	no	no
migrate	Perform null safety migration on a project.	yes	yes
pub	Work with packages.	no	no
run	Run a Dart program.	yes	yes
test	Run tests for a project.	yes	yes

Some other thoughts:

When running pub get in the above scenarios, we should:

• always emit the full pub get text (so people can use it to diagnose issues)
• emit to stderr, so we don't harm the ability for people to continue to parse stdout output of the above commands
• likely [no-]auto-pub should be a top-level flag, available to all commands, but that only really applies to the ~6 above

[mit-mit]

I was thinking that those are two orthogonal dimensions, but that we'd want the same behavoir in both cases. And it does look like you have the same values in all our rows.

[jonasfj]

There is a possible complication around flutter. If you use dart format with flutter, or dart analyze with flutter, then the dart pub get we run won't necessarily do all of the plugin registration setup steps -- ie. it's not the same as flutter pub get.

[DanTup]

Here's the list of the current dart commands:

There's language_server and debug_adapter too (which I think should not attempt to run anything here).

Today, we have reports that dart test won't work in the pkg// folder of the Dart SDK

Yes, I've seen issues with this. In VS Code we have to use dart test (or for reasons I don't remember, dart run test:test) to run individual tests, but running it in the SDK repo seems to mess things up (I think it produces .dart_tool/packages_config.json inside the packages folder, and the errors are really confusing and don't make it easy to figure out what happened and how you fix it).

So in VS Code we currently detect the SDK and disable the functionality of running individual tests (which makes me sad when working on some packages 🙃). While this works, it's not ideal, and if anybody else has projects with a similar setup to the Dart SDK they won't be handled specially and may have issues. It'd be good if there was some more general way this could be detected/handled (by dart or pub and not the editor).

[mit-mit]

If you use dart format with flutter, or dart analyze with flutter, then the dart pub get we run won't necessarily do all of the plugin registration setup steps -- ie. it's not the same as flutter pub get.

Hmm. Maybe the root issue here is having the behavoir depend on whether you run pub get or flutter get rather than what is declared in the pubspec.yaml. I think the right resolution here is to run the Flutter steps whenever resolving a pubspec.yaml that is a Flutter pubspec, i.e. has flutter: sdk: flutter in it.

[mit-mit]

The present issue seems even more critical in Dart 3. Consider a pubspec that doesn't support 3, e.g. sdk: '>=2.9.0 <3.0.0'. This will not resolve in Dart 3, but if you just run a Dart command like dart analyze or dart compile exe, then those commands will continue to run, but now with Dart 3 as the language version. They really should run pub get and produce an error instead.

[itsjustkevin]

@mit-mit are we still targeting this for Dart 3? It seems like a nice to have.

[mit-mit]

We'd like it in 3 as it's a breaking change in tools behavoir

[rrousselGit]

Is there any way to disable this with maybe some flags?
Flutter has a flutter run --no-pub

I have some tests which try to use Process.run('dart', ['run', 'my_cli']) on a temporary project created with a package_config.json already setup in a very specific manner.
The command trying to run pub get is fundamentally the opposite of what I want in my case.

It also makes my test perform network requests when it could work fully offline.

[mit-mit]

run --no-pub SGTM -- wdyt @sigurdm ?

[sigurdm]

Is there any way to disable this with maybe some flags? Flutter has a flutter run --no-pub

Yes, we will have a flag --no-pub!

I have some tests which try to use Process.run('dart', ['run', 'my_cli'])

You probably want to do Process.run('dart', ['bin/my_cli.dart']) to avoid going into dartdev entirely...

[DanTup]

If this will be merged for Dart 3, are commands like dart language_server and dart debug_adapter going to be excluded? I don't think it will ever make sense to start running Pub things for these, they are to start servers that provide functionality to editors, and the directory they are run from is not necessarily indicative of the project(s) they will be used for analyzing/debugging.

[sigurdm]

If this will be merged for Dart 3, are commands like dart language_server and dart debug_adapter going to be excluded?

Yes, see: https://dart-review.googlesource.com/c/sdk/+/291500

[DanTup]

@sigurdm ah, I see it's opt-in per-command and not run for everything. Thanks! :-)

[sigurdm]

Reopening, since this was reverted.

[sigurdm]

It appears that dart file.dart also implicitly runs pub get.

That sounds like a bug. Can you reproduce this?

[rrousselGit]

It appears that dart file.dart also implicitly runs pub get.

That sounds like a bug. Can you reproduce this?

I switched back to stable in the meantime.

What triggered it is, I followed your suggestion of using dart bin/file.dart instead of dart run cmd.
But to my surprise, dart bin/file.dart also overwrote the package_config.json, which breaks my tests (I was at a commit before these changes were reverted but after the introduction of --no-pub)

I switched back to the stable channel, and my tests are now passing.
The tests were also passing when using dart run --no-pub cmd on master.

Since this was reverted, I'll check if things work again on master.

[sigurdm]

Did you run with any other flags? Otherwise dartdev should be skipped, and .dart_tool/package_config.json should stay intact...

[rrousselGit]

No other flag, no.
But it's not impossible that my assumption that this was an implicit pub get is wrong.

In any case with the current state of master, I've just tested and dart file.dart works fine. I guess I'd have to try again after this is reimplemented

[rrousselGit]

If that would be helpful, I also don't mind trying my test suite on a specific commit ID for the Dart SDK to try and investigate.

Not sure which Flutter commit exactly had this enabled. If you know, I can easily check again

[sigurdm]

The change landed in dart sdk commit: 645511d

[rrousselGit]

I'm running it through Flutter – my test suite involves Flutter too.
Do you know which Flutter commit has 645511d?

[sigurdm]

Ah - sorry. You said the flutter revision.

I believe flutter/flutter@7725f59 should have the change.

[rrousselGit]

I can't seem to reproduce the issue with dart file.dart at that commit.

Are you sure that this is the right revision?
While playing around with
flutter/flutter@7725f59, I tried dart run --no-pub a bit. And the flag doesn't seem to quite work.

The command doesn't complain about an unrecognized flag. But the command still seems to perform a pub check.

I tried running dart run --no-pub my_command on a project in two situations:

1. Before running the command, I edited the pubspec to include a package which is not published on pub (so pub get would fail).

The command then fails with:

% dart run --no-pub custom_lint
Resolving dependencies in /Users/remirousselet/dev/dart_custom_lint/packages/custom_lint/example... 
Because custom_lint_example_app depends on package_not_on_pub any which doesn't exist (could not find package package_not_on_pub at https://pub.dev), version solving
  failed.

Since --no-pub is passed, I don't see why the command should "resolve dependencies" :)

2. To check if the issue with 1) was only a validation check or if a complete pub get was performed, I reverted the pubspec change and edited the package_config.json before running the command: I replaced the "packages" list with an empty list.

What's unexpected is, the package_config.json was overwritten as if dependencies were resolved again.

But somehow the command fails anyway:

 % dart run --no-pub custom_lint
Resolving dependencies in /Users/remirousselet/dev/dart_custom_lint/packages/custom_lint/example... 
Got dependencies in /Users/remirousselet/dev/dart_custom_lint/packages/custom_lint/example.
Could not find package `custom_lint` or file `custom_lint`

Maybe the package_config.json was parsed by the command before the file was regenerated – hence the error?

Because if I run the command twice, the second attempt works fine (since the first run restores the package_config.json somehow).

[sigurdm]

If the command recognizes --no-pub it is the right revision. That was introduced in the same commit.

But what you are pointing out is indeed a bug. The resolution logic for dart run is broken here. The --no-pub only applies to dart run path/to/file.dart not to dart run command. Thanks for spotting this. It should probably be fixed if we decide to reland this.

[DanTup]

@sigurdm what was the reason for reverting this, and how likely is it to re-land? I'm wondering whether it may be related to discussions at #52067 (pub being run in sdk/pkg and breaking things). If it's likely to re-land, I'll do some testing with the change included and see if I can find what might be triggering it.

[sigurdm]

I think the chances for relanding this for dart 3.0 are pretty slim.

In my understanding it was reverted mainly because how it affected the sdk workflow, but also because there was some doubts about failure modes - what happens eg. in case of no network.

(and also some discussions if it would be a ui improvement at all).

[rrousselGit]

If we're talking about UX, I personally dislike Flutter doing this.

Most IDEs already offer running pub get if out of date or on pubspec change anyway.
So the only thing it really does is cause the analyzer to restart an analysis whenever a CLI is run.

[mit-mit]

From asking around I've heard these arguments against:

1. General concern that we were too late in the 3.0 release cycle. This is probably better to do early in a cycle

2. Doesn't fail graciously when the network connection fails -- tracked in https://github.com/dart-lang/sdk/issues/52066

3. Some issues when developing in the Dart SDK itself. I believe those have been resolved, or are resolvable

4. Some suggestions for us to make sure this is aligned with the general expectation of the user coming from other languages

[mit-mit]

So the only thing it really does is cause the analyzer to restart an analysis whenever a CLI is run.

@rrousselGit can you elaborate on this? -- if you have an up-to-date resolve, the implicit pub get is expected to be a no-op, so I'd not expect the analyzer to restart.

[DanTup]

@mit-mit

3. Some issues when developing in the Dart SDK itself. I believe those have been resolved, or are resolvable

Do you know how this was/will be resolved? Dart-Code has some detection of the SDK try and avoid running pub get, but it would be nice if this was handled by pub (or at least, there was an official way to opt-out of pub) so it's harder to accidentally break things (as seems to have been happening in #52067, although I'm not sure of the exact trigger yet).

[rrousselGit]

@mit-mit

Here's a video of me running pub get n a project with up-to-date dependencies.
As we can see, the lints disappear for a second and reappear

Screen.Recording.2023-04-18.at.12.25.19.mov

Note that I'm also using analyer plugins in this workspace. That could be the cause (maybe the analyzer restarts to restart plugins. Wild guess). Also the project where I'm running pub get doesn't have enabled plugins (but another one in a different folder does).

[DanTup]

I can reproduce what @rrousselGit sees. In the analyzer log I can see that both pubspec.lock and .dart_tool/package_config.json are modified, triggering the analysis context to be re-created:

1681814088892:Watch:<unknown>:/Users/danny/Desktop/dart_sample/pubspec.lock:modify
1681814088893:Watch:<unknown>:/Users/danny/Desktop/dart_sample/.dart_tool/package_config.json:modify

I can't see any obvious changes in pubspec.lock, but package_config.json has a timestamp that is updated:

"generated": "2023-04-18T10:34:48.792316Z",

[rrousselGit]

Also we'd had issues with this in the past with Melos.

It's a custom command line that used to edit the package_config.json for the sake of bootstrapping mono-repositories with the local code of packages instead of the remote pub.dev implementation – without having to edit the pubspec.yaml (since having to do a back-and-forth between path dependencies and version dependencies is a pain).

Commands running pub get automatically meant that they would override the package_config, reverting any change made by melos.

Melos later switched to a newer pub feature (pubspec_overrides.yaml files), so this is no longer an issue for that package. But the problem is worth mentioning.

[lrhn]

Running dart pub get directly is expected to try to do something.

It would be neat if dart pub get avoided touching the pubspec.lock and .dart_tool/package_config.json files if it resolved to the same package versions again - but it's also valuable to be able to overwrite those files in case manual edits have clobbered them, so always writing new files is the safest approach.

Automatically running of pub get for other commands, the feature which is being discussed here, should only happen if there is no (or not up-to-date) .dart_tool/package_config.json file, because that file is needed for running or analyzing Dart code. Doing dart test without a .dart_tool/package_config.json file just won't run (or, worse, might find a different package_config.json in a parent directory).
Doing it with an out-of-date package_config.json file is ignoring changes made to pubspec.yaml, which can again mean you're not testing what you think you're testing.

If your IDE has already run pub get, there should be no need to do an extra run, and nothing should happen.

I'm not sure which criteria would be used to see if the package_config.dart file is up-to-date (but a modified-time later than pubspec.yaml modified time would be the simplest one.)

[jonasfj]

We have decent logic for detecting if we need to run a full resolution.

1. Fast path:
   • Check modification time of pubspec.yaml, pubspec.lock and .dart_tool/package_config.json.
   • Grep pubspec.lock for path: indicating a path-dependency (this is necessary because path dependencies require extra checks)
2. Check the current resolution:
   • Check if pubspec.lock satisfies pubspec.yaml
   • Check if .dart_tool/package_config.json provides packages from pubspec.lock
3. Do an actual resolution with dart pub get semantics.

[sigurdm]

We have decent logic for detecting if we need to run a full resolution. ...

Implemented here: https://github.com/dart-lang/pub/blob/master/lib/src/entrypoint.dart#L624

[rrousselGit]

The implicit pub get also has the added side-effect of stopping any pending build_runner watch commands.

build_runner stops if a pub get is detected. So running a Dart command in a world where the command automatically does pub get causes build_runner to stop, which is a major inconvenience IMO.

[sigurdm]

The implicit pub get also has the added side-effect of stopping any pending build_runner watch commands.

So, if the resolution is up-to-date (and the time-stamps are in the right order) that should not happen.

But it is a good point, that might be an undesirable side-effect.

[rrousselGit]

Would it maybe make sense to offer a way to globally disable the pub get?

Like maybe:

dart config RUN_PUB_GET=false

which sets the default for --pub to false for all Dart commands?

[sigurdm]

Would it maybe make sense to offer a way to globally disable the pub get?

I don't think this is the level of configuration we want. That would create a cognitive overhead, and you could not look at a command and know what it is doing out of context. Also this would most likely be something that you want on a project basis (eg. you might not want it in the sdk, but always for a pub project).