Add the ability to check if "pub get" needs running

[DanTup]

When a user opens a Dart project in Dart Code I'd like to inform them if they need to run pub get (with an easy button to do so) to avoid lots of analysis errors opening freshy-cloned/updated projects.

Currently there doesn't seem to be a way to detect if local packages are out of date in order to prompt the user (we could try checking timestamps on .packages and pubspec.yaml but it's not very reliable) so it'd be nice if something could be added.

(I'm requesting this in pub, but I don't know if it might make sense elsewhere; feel free to request I re-raise this in sdk or somewhere if it would be more appropriate there).

[zoechi]

den fetch should do what you want https://pub.dartlang.org/packages/den
But the existence of newer versions doesn't mean they are compatible with your dependencies. You would need to run pub upgrade to get the resolved dependencies.

[DanTup]

den fetch should do what you want

I'm not sure this would work - we can't globally install pub packages on the end-users machine and even if you can write a dart app using this library that reads another projects pubspec, our extension is written in TypeScript and it'd be a bit clunky shipping a dart app inside our TypeScript app (long-term we'd like to be written in Dart, but that's way off and might not be feasible).

But the existence of newer versions doesn't mean they are compatible with your dependencies.

We're not planning on upgrading anything - this is a request for detecting when pub get is required, eg. you've cloned a repo that has never had "pub run" called or the user has modified the pubspec file (eg. changing the version of a package) and the local packages are incorrect. In this case, a project will report many errors/warnings where the fix may be just running pub run but it's not obvious to the user from the errors/warnings.

An IDE should be able to detect when the local .packages file (or packages symlinks) are stale so they can suggest the correct fix rather than confuse the user. In .NET if you open a project missing NuGets you're either prompted to download them or it silently happens before build; it's really painless. We'd like to achieve something similar.

[sigurdm]

Could this be implemented as

$dart pub get --dry-run

With an exit-code of 0 meaning everything is up-to-date?

[DanTup]

I think so. For now I'm just checking timestamps on .packages/pubspec.yaml/etc.

Some things to consider:

• The folder opened in the editor might have multiple projects so we'd like to detect this as quickly as possible for them all - would it be safe to call this for them all concurrently? (I know there have been issues with concurrent pub calls in the past, though I suspect that was related to writing to disk that --dry-run might not need to do?)
• If the project is not being managed by pub (for example dart-lang/sdk) would what the result be? Does the editor still need to detect that separately, or could this indicate that too?

[sigurdm]

The folder opened in the editor might have multiple projects so we'd like to detect this as quickly as possible for them all - would it be safe to call this for them all concurrently? (I know there have been issues with concurrent pub calls in the past, though I suspect that was related to writing to disk that --dry-run might not need to do?)

Yeah I think this should be thread safe as it is not writing anything.

Btw. 5bda798 should fix the issues with running pub gets concurrently.

If the project is not being managed by pub (for example dart-lang/sdk) would what the result be? Does the editor still need to detect that separately, or could this indicate that too?

Good question. I would think pub get --dry-run would return non-0 if there is no pubspec.yaml in the current directory (how could it know how to handle non-pub directories in general?) probably the exit-code would be 66 as it already is for dart pub get.

[DanTup]

probably the exit-code would be 66 as it already is for dart pub get.

So we could run --dry-run and assume:

• 0 exit code - nothing to do
• 66 exit code - not a pub project (so also nothing to do)
• any other exit code - package update required?

What about if there are other types of failures (eg. Pub can't be contacted)? Would it be better to have a single exit code that means there definitely are updates and you should run without --dry-run and then we can assume other error codes are other types of failures that shouldn't trigger the prompt?

[sigurdm]

Would it be better to have a single exit code that means there definitely are updates and you should run without --dry-run and then we can assume other error codes are other types of failures that shouldn't trigger the prompt?

Yeah I think having a specific error code for update-needed would be good.

Looking in exit_codes.dart doesn't yield much prior art here. Perhaps exiting with 1 could mean not-up-to-date...

[sigurdm]

Grep returns 1 if it found no matches - that seems somehow analogous.

[DanTup]

Isn't that the opposite? Grep returns 1 for nothing found (eg. it failed to do what you want) but here we're talking about signalling there is the thing you were checking for (available updates)?

That said, the number isn't so important to me (as a consumer), as long as it's documented and uniquely means I should prompt the user to fetch, so anything you think suitable gets my vote :-)

[sigurdm]

@jonasfj probably has an opinion here.

[jonasfj]

Doesn't this have to be very fast? Is the overhead of using dart pub get --dry-run not a little high?

Also dart pub get --dry-run will do network I/O, which isn't necessary if we're just checking if dart pub get needs to run.

IMO, we should rely on:

• mod_time pubspec.yaml < mod_time pubspec.lock < mod_time .dart_tool/package_config.json

This is a very good approximation. Indeed this is the first thing dart pub get will do internally to decide if we should simply skip running dart pub get.

In cases where this invariant doesn't hold, I suggest running dart pub get -- it seems rare to me that this over-approximation won't work. It'll recommend running dart pub get a little bit more often than is necessary (it's an over-approximation). But this is normally a corner-case that will only happen if you've edited comments in pubspec.yaml.

---

In general, I'm not sure we shouldn't just run dart pub get without asking the user whenever the modification-timestamp invariant above suggest it. Because:

• It's cheap to run dart pub get,
• It's very rarely destructive in nature,
• The only sensible thing to do when package_config.json is out-of-date is to run dart pub get -- so instead of telling people to do, maybe just do it!

For the record, dart run and dart test will implicitly run dart pub get in the current project if necessary (so this wouldn't be new territory).

---

The folder opened in the editor might have multiple projects so we'd like to detect this as quickly as possible for them all - would it be safe to call this for them all concurrently?

dart pub get --dry-run won't be fast, because it'll check online for versions and actually do version resolution.

Checking modification timestamps, and then just running dart pub get might be fast :D

If the project is not being managed by pub (for example dart-lang/sdk) would what the result be? Does the editor still need to detect that separately, or could this indicate that too?

If pubspec.yaml doesn't exist, then don't run dart pub get :D
I guess it would still cause problems in pkg/ sub-folders, maybe an option for SDK developers to disable this in editor configuration would be fine.

[DanTup]

Doesn't this have to be very fast? Is the overhead of using dart pub get --dry-run not a little high?

Yes, I think you're probably right. Really we want something fast that doesn't even touch the network.

mod_time pubspec.yaml < mod_time pubspec.lock < mod_time .dart_tool/package_config.json

This is what I'm doing today, although it's possible pubspec.lock doesn't exist and we shouldn't assume it's safe to run pub get (for ex. https://github.com/dart-lang/sdk). I guess the goal would be to offload that logic into pub so editors don't need to be aware of the specifics (which if it changes - as it has with .packages going away, requires tool updates, and them handling multiple different SDK versions, both of which would not be if pub did it).

Given the network access in pub get --dry-run, I think if it was worth doing, it would probably be better as a specific command/flag whose job is only to do that check (and take into account rules like the SDK that are not pub-managed - and if we care about performance, perhaps doing it for multiple folders in a single command would be even better). Whether it's worth doing though, I'm less certain about.

In general, I'm not sure we shouldn't just run dart pub get without asking the user whenever the modification-timestamp invariant above suggest it

We do this if the file is modified while the editor is open, but doing it automatically when you open a folder feels a bit aggressive (especially if you consider the user may be offline, and Flutter can easily get into loops like #2242 that never end). I want my editor to open as fast as possible, and it's already starting a bunch of expensive work (like analyzing the project). Although, perhaps there should be an option (currently our option is just to prompt, but we could make it doNothing/prompt/automaticallyFetch).

For the record, dart run and dart test will implicitly run dart pub get in the current project if necessary (so this wouldn't be new territory).

As an aside - this behaviour has actually tripped me up a few times.. I didn't realise this happened and invoked one of them inside an sdk/pkg folder and had weird issues that took me a while to figure out 🙃 It would be good if pub knew when it was in a folder where it's not in control (although I don't know how defined that is - I think there were some changes from Kevin(?) to try and improve that in some places).

If pubspec.yaml doesn't exist, then don't run dart pub get :D
I guess it would still cause problems in pkg/ sub-folders, maybe an option for SDK developers to disable this in editor configuration would be fine.

I'd prefer to detect this better because of the issue above... If you don't know about the config, you open the SDK, it calls pub get, you end up with nested .dart_tool folders, and even once you fix the config you need to delete those files. However, VS Code is already detecting the Dart SDK folder and disabling pub anyway, so it's not a significant issue (it's already handled), although I do feel like this logic would be better inside a central tool than each editor needing to understand it.

With LSP/DAP, it should be much simpler for additional editors to support Dart, but there are still a bunch of little bits around the edge like that that aren't nicely abstracted. Whether it's worth doing them, I'm not sure - but it's food for thought. Requirements changing across SDK versions is certainly one place where things get tricky, as the tools may need to support multiple SDK versions, whereas tools inside the SDK are always their own SDK version :-)

[jonasfj]

I guess the goal would be to offload that logic into pub so editors don't need to be aware of the specifics (which if it changes - as it has with .packages going away, requires tool updates, and them handling multiple different SDK versions, both of which would not be if pub did it).

I think it's just as likely that the tools will change. If we embedded pub functionality in the analysis_server maybe as an LSP extension or something.. But that seems far more complicated.

and if we care about performance, perhaps doing it for multiple folders in a single command would be even better

Yeah, many subprocesses aren't cheap, the other reason I would suggest checking timestamps.

We do this if the file is modified while the editor is open, but doing it automatically when you open a folder feels a bit aggressive

Fair enough.

it's already starting a bunch of expensive work (like analyzing the project)

So true.

As an aside - this behaviour has actually tripped me up a few times.. I didn't realise this happened and invoked one of them inside an sdk/pkg folder and had weird issues that took me a while to figure out

hehe, when there is a pubspec.yaml then dart pub owns fetching dependencies. The only corner case I know where this isn't the case is:

• Dart SDK
• google3 (but, afaik people don't use dart command-line here)

In google3 they could probably choose not to import the pubspec.yaml (or rename it when importing). The only corner case I know of is the Dart SDK.

I'd prefer to detect this better because of the issue above...

It's hard to detect this. It seems a bit weird to support an ignore_this_pubspec: true property in pubspec.yaml 🤣
I'm open to ideas, but I suspect that in many cases when people have nested .dart_tool folders, it's because they have nested packages -- and maybe that's okay. I think we should be careful to optimize for Dart SDK developer ergonomics, maybe just have a configuration file for vscode in the Dart SDK that disables some behaviors is enough.

[DanTup]

The only corner case I know where this isn't the case is:

Dart SDK
google3 (but, afaik people don't use dart command-line here)

I don't know of others either, although I assumed that these aren't particularly special - eg. there's nothing to say some other companies are also not using Pub and doing something similar. Although, without a clear convention, I'm not sure there's an expectation that tools like VS Code would know what to do.

Based on the above, I'm not certain if it's worth doing anything here. It does feel inconvenient that each editor will need to understand these rules (and handle .packages -> .dart_tool/package_config.json move/multiple SDK versions), but it's not the sort of change that happens frequently and the rules are relatively simple.

I haven't had any complaints about VS Code doing the wrong thing in a long time (I presume this only came up recently because of the .packages removal.. this issue is quite old) and I've already shipped an update to not rely on .packages for the pub get check.

[sigurdm]

Do we need to do more here?

Can we run with just checking timestamps?

[DanTup]

VS Code currently does check timestamps. It works ok, I don't get many complaints (some Googlers have said they see it more often than they expect, although I don't have anything concrete).

It's not ideal because it relies on some assumptions about the files pub writes (these assumptions might also be affected by #4022?).

It would certainly be nicer if there was some simple API to allow asking Pub to run whatever the correct logic is and give a response. Whether it's worth doing, I'm less sure.

[DanTup]

It occurs to me that the monorepo work being discussed might impact this. Right now the IDE locates all of the projects, checks timestamps of files in each one, and then offers to run pub get in all those projects.

It seems like in the monorepo world, we might no longer want to run pub get in every project (because even if it works, it might be much slower and do a lot of duplicated work). It seems like having some kind of better "API" for this IDE<->Pub integration (specifically, determining whether pub get needs running, and where it may need to be run in a multi-project/multi-folder workspace) might be better.