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 :see_no_evil:

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! :-)

[rrousselGit]

It appears that dart file.dart also implicitly runs pub get. But as opposed to other commands like dart run cmd, there is no mean to do dart --no-pub file.dart

Is that an oversight?

[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: 645511d7f427ea1456b3d9edfb7b8fbf039d6af4

[rrousselGit]

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

[sigurdm]

Ah - sorry. You said the flutter revision.

I believe https://github.com/flutter/flutter/commit/7725f5965b9ceb1bb3fd37aa12eb05ccda8da1c6 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 https://github.com/flutter/flutter/commit/7725f5965b9ceb1bb3fd37aa12eb05ccda8da1c6, 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).