dart-lang / pub

The pub command line tool
https://dart.dev/tools/pub/cmd
BSD 3-Clause "New" or "Revised" License
1.04k stars 229 forks source link

Make `pub get` call a Flutter post-get hook #2849

Open jonasfj opened 3 years ago

jonasfj commented 3 years ago

Instead of a scenario where flutter pub get has to wrap the call to pub get and do something else afterwards.

DanTup commented 3 years ago

@jonasfj would this avoid needing to call flutter pub? For the changes in the analysis server to call "pub outdated" I hit an issue with Flutter projects (it doesn't work). The analysis server doesn't (and probably shouldn't) know about having to call "flutter pub" so it would be ideal if Pub could just figure it out from the packages and do the right thing.

jonasfj commented 3 years ago

It is possible to work around this by setting FLUTTER_ROOT then dart pub outdated will work.

But yes, it would be great to get these tools aligned.

DanTup commented 3 years ago

I don't think it's trivial for me to know what to set FLUTTER_ROOT to (and it probably doesn't make sense to encode this knowledge into the analysis server, especially if it'll ultimately go away). Is this something that might happen in the near future?

Thanks!

devoncarew commented 3 years ago

@jonasfj, for context, we'd like to be able to call dart pub outdated from tooling, but don't want to have to set up the path correctly for the invocation to work, or, have the analysis server have to know to instead call flutter pub outdated.

I think an ideal solution is for the pub operations to work on flutter projects if the pub used is the one in /bin/cache/dart-sdk - for it to use it's own path in order to resolve the sdk vended packages like package:flutter.

I think this is essentially https://github.com/dart-lang/pub/issues/2307.

jonasfj commented 3 years ago

@DanTup if the dart command you're using in dart pub outdated originates from the Flutter SDK, then it should have FLUTTER_ROOT set (in its wrapper script), see https://github.com/dart-lang/pub/issues/2307#issuecomment-843606548.

So then that should be fine. This issue is regarding the fact that dart pub get doesn't yet trigger the same post-get hooks as flutter pub get. This mostly has to do with a bit of code-gen for hooking up plugins and such, it shouldn't affect the output of dart pub outdated.

Hmm, I guess it starts to depend on how analysis server is launched, when using the Dart SDK distributed with Flutter, does it have FLUTTER_ROOT? If you're invoking dart pub .. from $FLUTTER_ROOT/bin/cache/dart-sdk/bin/dart then it won't have FLUTTER_ROOT configured.

DanTup commented 3 years ago

if the dart command you're using in dart pub outdated originates from the Flutter SDK, then it should have FLUTTER_ROOT set (in its wrapper script), see #2307 (comment).

Unfortunately it does not - we don't use PATH for executing dart because there's no guarantee the one on PATH is the one the server is running from (for example the first dart on PATH on my machines is always the standalone Dart SDK, but Dart-Code will find/use the Dart SDK inside Flutter when opening a Flutter project - often Dart is stable and Flutter is master).

I guess it starts to depend on how analysis server is launched, when using the Dart SDK distributed with Flutter, does it have FLUTTER_ROOT? If you're invoking dart pub .. from $FLUTTER_ROOT/bin/cache/dart-sdk/bin/dart then it won't have FLUTTER_ROOT configured.

Correct, it's launched from there and does not have it set. I think the goal is to have the analysis server able to call pub for any project without needing to know about these details itself.

(Edit: there's some additional detail in @devoncarew's comments at https://github.com/dart-lang/pub/issues/2307#issuecomment-843612458)