Don't rely on .packages to detect if dependencies needs to be fetched

[jonasfj]

Starting Dart 2.17 (as of current planning), dart pub get won't generate .packages anymore. I noticed a few places in Dart-Code that looks for .packages files, for example to determine if dart pub get needs to be run.

Maybe there are other places where .packages is used in Dart-Code?

See: dart-lang/sdk#48272

Side note: The best was to determine if dart pub get needs to run is probably to check that the modification time of pubspec.yaml < pubspec.lock < .dart_tool/package_config.json.

[DanTup]

Thanks for the heads up! We do already handle package_config.json in many places, but it does look like there are places that will do the wrong thing if .packages doesn't exist:

Dart-Code/src/extension/pub/pub.ts

Lines 22 to 24 in 03c6662

	// If we don't have .packages, we probably need running.
	if (!fs.existsSync(packagesPath))
	return true;

I'll go through everywhere and fix them up.

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

I'll update to do this too. Currently we're only doing pubspec.yaml < .packages. Thanks!

[jacob314]

Can we instead surface an api in dart dev that answers whether dependencies need to be fetched rather than having each tool implement the logic? That would make it easier if we need to implement the logic again and would provide a natural hook to suppress the logic in google3 where this logic doesn't make sense.

[DanTup]

That could be nice - although we may do this check for many folders are once (eg. you open a monorepo) so if we did it by calling pub/dart it would be good to be able to ask for them all in one go to avoid spawning many processes.

Slightly related, there are some other small things we have some logic in the editor for - like "run pub get on pubspec save" that presumably need the same google3/google SDK exclusions. We also have to debounce it in case it's saved multiple times. IIRC it's done with file watchers, so perhaps there's value in some mini-daemon for Pub that can handle requests both for checking if the dependencies need fetching, and handling watching/fetching (reporting progress to the editor)? (although, perhaps that wouldn't play nice with Flutter projects since they currently want flutter pub get).

[DanTup]

I've updated to use package_config for now. Even if we do the above, we'll need to support the SDKs between now and then.

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

I left this as it was (pubspec.yaml < .dart_tool/package_config.json) because I noticed pubspec.lock doesn't exist in some repos (like the SDK) and it seemed simpler to not have to worry about that (if I understand correctly, I think it's unikely it would change the answer).

[jonasfj]

@DanTup, if you have pubspec.lock checked into git, and you pull down new commits that only change the lockfile, then it would be wise to run dart pub get.

So I would suggest checking the file modification time, if it exists..

But yes, in many cases it won't matter too much..

As for making a daemon that would be cool, and fast as we could cache a lot of state in memory (rather than loading from PUB_CACHE). But it's probably also a lot of work to build it, and make it useful. For now I think we should just keep it simple :)

[jonasfj]

@DanTup, if you have pubspec.lock checked into git, and you pull down new commits that only change the lockfile, then it would be wise to run dart pub get.

So I would suggest checking the file modification time, if it exists..

But yes, in many cases it won't matter too much..

As for making a daemon that would be cool, and fast as we could cache a lot of state in memory (rather than loading from PUB_CACHE). But it's probably also a lot of work to build it, and make it useful. For now I think we should just keep it simple :)

[DanTup]

@DanTup, if you have pubspec.lock checked into git, and you pull down new commits that only change the lockfile, then it would be wise to run dart pub get.

Good point. Though I think I'll have to assume if that file is missing that's not an indicator that it needs to be run (given the SDK example).