make the logic to locate the Flutter SDK more robust

[devoncarew]

When running pub get on a Flutter project, I see:

Resolving dependencies... 
Because mp_chart depends on flutter_test any from sdk which doesn't exist (the Flutter SDK is not available), version solving failed.

This is when using the pub client that is embedded in the flutter sdk (flutter/bin/cache/dart-sdk/bin/pub). Currently the pub client only locates the Flutter sdk when the FLUTTER_SDK environment variable is set. We should expand this to also check if the specific instance of the pub tool itself is part of the Flutter sdk (something like using Platform.resolvedExecutable, and verifying it's location).

[jakemac53]

Note that build_runner also needed to know if you were running from the flutter sdk or the dart sdk - you can check the Platform.version for the flutter string but only in newer versions.

We have an outstanding issue to have a fallback for older sdks that does some file system spelunking to figure it out.

It would be good to establish a common way of determining if you are running from the flutter sdk (and its location).

dart-lang/build#2528 is the issue for the build repo

[sigurdm]

I'd like to get a bit more context - what is the use-case of calling pub get directly for a Flutter project? Could you just use flutter pub get (or set FLUTTER_SDK manually)?

It is not clear to me that pub in flutter/bin/cache/dart-sdk/pub is publicly exposed, but is there to be embedded by the flutter tool, and the right way to get dependencies of a Flutter project is flutter pub get that would set FLUTTER_SDK correctly.

Personally I'd like to put as little flutter-specific logic in the pub client as possible. But I think it would make sense to give a better error message explaining to use flutter get instead.

[jakemac53]

It is not clear to me that pub in flutter/bin/cache/dart-sdk/pub is publicly exposed, but is there to be embedded by the flutter tool, and the right way to get dependencies of a Flutter project is flutter pub get that would set FLUTTER_SDK correctly.

It is easier if you are a flutter developer to not bother installing the dart sdk twice, and instead add the one from flutter to your PATH. I don't have an idea of how common it actually is though.

[devoncarew]

It is not clear to me that pub in flutter/bin/cache/dart-sdk/pub is publicly exposed

People do use the dart sdk embedded in the Flutter SDK, and @mit-mit has a proposal to make this an officially supported use case.

Given that a very large portion of dart usage happens in the context of Flutter, having pub understand when it's in the context of a flutter sdk makes sense to me. It's odd to see a "can't find the flutter sdk" error message from pub when it's being run from the flutter sdk.

[devoncarew]

It would be good to establish a common way of determining if you are running from the flutter sdk (and its location).

Yup, +1 to establishing a simple, common mechanism for this.

[sigurdm]

It is easier if you are a flutter developer to not bother installing the dart sdk twice, and instead add the one from flutter to your PATH. I don't have an idea of how common it actually is though.

If they are adding the dart sdk from Flutter to their path it will work perfectly for dart projects, and the flutter command will work for flutter projects.

Yup, +1 to establishing a simple, common mechanism for this.

If we have an easy way of doing this I would be fine with pub using that in addition to FLUTTER_SDK.

We would need both a way of knowing if we are run from a flutter sdk, and also a way of locating it.

It would be awesome if it was a general mechanism for finding any sdk - I suspect we will have the same issue for Fuschia but that might be too far fetched for first iteration.

[devoncarew]

If they are adding the dart sdk from Flutter to their path it will work perfectly for dart projects, and the flutter command will work for flutter projects.

The issue I ran into was that I had ~17 projects to run pub get for, and didn't know in advance whether they were regular dart projects or flutter projects.

If we have an easy way of doing this I would be fine with pub using that in addition to FLUTTER_SDK.

I think this would likely look something like using Platform.resolvedExecutable, looking up the right number of directories, and recognizing that we were running from the context of a Flutter SDK. The relative location of the Dart sdk has been stable for quite a while, and we can likely graduate it to API with some discussion.

Here's an example of the lookup logic:

https://github.com/flutter/devtools/pull/1555/files#diff-c647d3c72b91c3144d5b38e18a17433dR77

[jakemac53]

I think this would likely look something like using Platform.resolvedExecutable, looking up the right number of directories, and recognizing that we were running from the context of a Flutter SDK. The relative location of the Dart sdk has been stable for quite a while, and we can likely graduate it to API with some discussion.

Note that in bazel the Platform.resolvedExecutable has to be replaced with Platform.executable. This is because it is just a symlink in the bazel-bin directory, so resolving it gets you back to the source tree where there isn't a fully built sdk. The Platform.executable in bazel is I believe already a full absolute path to the runfiles dir (it is not invoked with just dart).

[sigurdm]

Here's an example of the lookup logic:

It would be nice to have a package for this - then the logic can change moving forward.

[sigurdm]

The issue I ran into was that I had ~17 projects to run pub get for, and didn't know in advance whether they were regular dart projects or flutter projects.

You could just set FLUTTER_SDK before running pub get.

[devoncarew]

You could just set FLUTTER_SDK before running pub get.

Yes, but users are not going to know to do that. I really think it's odd that the pub instance vendored into the Flutter sdk doesn't know how to locate the Flutter SDK.

[jakemac53]

Fwiw I think this behavior is pretty consistent with all binaries shipped with the sdk except maybe the analyzer. Granted, pub is a more user facing tool so it might make more sense for it to hardcode knowledge.

[natebosch]

I really think it's odd that the pub instance vendored into the Flutter sdk doesn't know how to locate the Flutter SDK.

If the flutter shipped version of pub wants to hardcode the knowledge about flutter it could do that in the shell script.

I think it would be sensible to search up for a flutter SDK on the principle that if we can figure out what the user meant to do, we may as well do it.

[jakemac53]

If the flutter shipped version of pub wants to hardcode the knowledge about flutter it could do that in the shell script.

We want to keep the Dart sdk that is shipped with flutter as close to the normal Dart sdk as possible, so I would like to avoid this if we can.

[sigurdm]

The issue I ran into was that I had ~17 projects to run pub get for, and didn't know in advance whether they were regular dart projects or flutter projects.

users are not going to know to do that.

Actually you could run flutter pub get in all the projects. That seems to work and requires no special knowledge.

[devoncarew]

With flutter/flutter#53470 close to landing (exposing the dart command on the path for flutter users), people will hit this more often. I think it'll be very confusing to users to run pub and see the Flutter SDK is not available, when the command is running from a flutter sdk.

cc @mit-mit as he's been driving exposing the dart command for flutter devs

[sigurdm]

To me flutter/flutter#53470 reads like it exports FLUTTER_ROOT. That would make the pub work correctly without further work from us (when invoked via bin/dart).

[sigurdm]

@devoncarew and @jonahwilliams, do you agree that the dart script sets the FLUTTER_ROOT and that we can close this issue?
Or do you see more work needing to be done for the pub client?

[devoncarew]

If flutter's dart entry-point does run w/ the FLUTTER_ROOT env var set, then:

• flutter pub get will work
• dart pub get will work (using the dart on flutter's path)
• dart pub get will fail (using a dart that isn't from the flutter sdk); that may be difficult for the user to diagnose, but would be WAI
• pub get will fail; that's the situation today, but if we're moving away from recommending people run the bar pub tool, and instead use dart pub get or flutter pub get, then that doesn't seem to be an issue

[devoncarew]

do you agree that the dart script sets the FLUTTER_ROOT and that we can close this issue?

And yup, given the above, I don't have concerns closing the issue.

[jakemac53]

We should consider making pub get and dart pub get log something more useful than "the flutter sdk is not available". Most users will just respond to that with "yes it is! arrrghhhh!", because it is available on the machine.

We know exactly what is actually happening here (they are running pub or dart from the dart sdk and not the flutter sdk), so we should instead log a message saying exactly that.

[sigurdm]

The current message is:

$ pub get
Resolving dependencies... (1.6s)
Because flutter_blah depends on location >=2.5.0 which requires the Flutter SDK, version solving failed.

Flutter users should run `flutter pub get` instead of `pub get`.

Which is not all that bad IMO.

[jakemac53]

Ah ok, I was basing the comment off of the original log in this issue Because mp_chart depends on flutter_test any from sdk which doesn't exist (the Flutter SDK is not available), version solving failed., if we now see the above then that SGTM

[jonasfj]

@jonasfj, reconsider this on Monday..

[jonasfj]

@DanTup, @devoncarew reading up #2307, the current state of affairs is:

• Running dart pub ... from Dart SDK distributed with Flutter SDK will work for flutter projects, because the bash/bat wrapper for dart does setup FLUTTER_ROOT (See bin/dart which sources from shared.sh).
  • Running dart pub get|upgrade|downgrade won't run some of the Flutter-specific code-gen hooks for plugins, this would be fixed with Make pub get call a Flutter post-get hook #2849. To get Flutter specific code-gen users need to run flutter pub get.
• Running dart pub .... from Dart SDK that is not distributed with Flutter SDK won't work unless env var FLUTTER_ROOT is manually configured somewhere.

Let me know if you think we should reopen this again. I don't really mind introducing a new way to detect if we have Flutter available somewhere.

In practice, if you have both Flutter SDK and Dart SDKs installed, then if you run dart pub ... from the Dart SDK (and not the Dart SDK that comes with Flutter SDK), then we can't automatically detect the location of the Flutter SDK (nor which Flutter SDK to use). So I'm not sure we can make this work much better, maybe the error message could propose using dart pub from the Flutter SDK, setting FLUTTER_ROOT or using flutter pub ....

Please reopen, if you think we need to revisit this.

[devoncarew]

The failure mode we're seeing is:

• the analysis server wants to invoke some tooling from pub
• it uses Platform.resolvedExecutable to locate the dart binary
• it then calls <absolute-path-to-binary> dart pub outdated --machine
• that is the dart from <flutter-sdk>/bin/cache/dart-sdk/bin/dart; it's not the sh wrapper from <flutter-sdk>/bin, and does not have the flutter env var set up
• it fails, unable to locate the flutter sdk

In practice, if you have both Flutter SDK and Dart SDKs installed, then if you run dart pub ... from the Dart SDK (and not the Dart SDK that comes with Flutter SDK), then we can't automatically detect the location of the Flutter SDK (nor which

Just for some additional clarity on the above, we are seeing failures even with just one Dart SDK installed (the one vended into the flutter sdk); not all paths for invoking dart know to or are able to invoke it through the wrapper dart script in <flutter-sdk>/bin.

[jonasfj]

Hmm, how is analysis server started? Does it have a FLUTTER_ROOT env var we want to it to forward?

But yeah, maybe we should fallback to detecting FLUTTER_ROOT relative to Platform.resolvedExecutable when FLUTTER_ROOT isn't defined.

It just feels a bit messy to have all these strategies and fallbacks... It'll be hard to maintain and test properly.. It's likely to just break when / if the folder layout changes.

Maybe we should have a flutter_root file next to the version file in the Dart SDK... or something like that..

[devoncarew]

It just feels a bit messy to have all these strategies and fallbacks... It'll be hard to maintain and test properly.. It's likely to just break when / if the folder layout changes.

I don't disagree - this introduces another row in the matrix of ways things could go wrong.

What if we switch to only locating the Flutter SDK by relative path from the Dart SDK? The location has been the same more or less since the two SDKs started shipping together. If we elevate their relative locations to API, then a dependency like the one specified below:

dev_dependencies:
  flutter_test:
    sdk: flutter

would mean to look up from <containing sdk>/bin/cache/dart-sdk, expect a packages directory (as a sibling of bin), and look for a package directory for 'flutter_test' in packages/.

The nice thing about that is that Flutter is no longer special cased. Any other framework could vend the SDK, make sure their directory structure was set up as per convention, and have sdk packages - addressable from a pubspec file - in <sdk>/packages/.

[devoncarew]

@jonasfj - some of our tooling features are blocked on this bug - things like doing code completion for package versions in pubspec files. I believe you were going to reconsider this? Happy to VC to discuss, or, this might make a good item for Q3 planning.

[jonasfj]

@devoncarew yeah, we will look into this :)

I agree, that we probably should just add more way to detect FLUTTER_ROOT.

[devoncarew]

Thanks much!

[DanTup]

@jonasfj I tried this out now it's rolled into Flutter, and it seems to be working great, thanks! :-)