Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Get Packages" commands should not include example folder implicitly #4709

Closed
pattobrien opened this issue Aug 29, 2023 · 12 comments
Closed
Labels
in commands Relates to commands (usually invoked from the command Palette) is bug
Milestone

Comments

@pattobrien
Copy link

Description

With a Dart package at root, and a nested Flutter example package, running Dart Code's pub get command (via pubspec save) fails with error 69. See https://github.com/pattobrien/nested_package_bug to reproduce.

Steps to Reproduce

  1. Create a new Dart package, and in the root directory create a Flutter package named example.
  2. Save Dart package's pubspec.yaml (i.e at root) to trigger pub get via Dart Code (VSCode extension).

Observed Result

[nested_package_bug] dart pub get
Resolving dependencies...
Got dependencies!
Resolving dependencies in ./example...
Because example depends on flutter_test from sdk which doesn't exist (the Flutter SDK is not available), version solving failed.

Flutter users should run `flutter pub get` instead of `dart pub get`.
exit code 69

Additional Info

Dart Code extension v3.70.0

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.13.0, on macOS 13.5 22G74 darwin-arm64, locale en-US)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0-rc1)
[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
[✓] Android Studio (version 2022.3)
[✓] VS Code (version 1.81.1)
[✓] VS Code (version 1.78.2)
[✓] Connected device (2 available)(code -10)
[✓] Network resources

• No issues found!
@DanTup
Copy link
Member

DanTup commented Aug 29, 2023

I think this is because pub is automatically recursing into example into by default now (see dart-lang/pub#3856) but because the example requires a Flutter SDK, it can't function if not invoked via flutter.

@sigurdm does this seem like a Pub bug? Should pub do this when the example is Flutter if it's running without Flutter?

As a fix for VS Code, I think I should probably pass --no-example here (because VS Code will handle running multiple projects if required), but not sure if Pub should be tweaked for CLI users?

@DanTup DanTup added this to the v3.72.0 milestone Aug 29, 2023
@DanTup DanTup added the in commands Relates to commands (usually invoked from the command Palette) label Aug 29, 2023
@DanTup DanTup changed the title Flutter SDK cant be found in nested "example" package "Get Packages" commands should not include example folder implicitly Aug 29, 2023
@DanTup
Copy link
Member

DanTup commented Aug 29, 2023

Although, for reasons I don't understand - this works for me 🤔

image

In the Pub log I see FLUTTER_ROOT: C:\Dev\Google\flutter although I don't know where it's getting it from. If I echo $env:FLUTTER_ROOT it's empty.

@DanTup
Copy link
Member

DanTup commented Aug 29, 2023

Oh, I think I see - I think dart for me is using the dart from the Flutter SDK so it can find it that way.

@pattobrien can you confirm if this only happened because you created the example project while VS Code was open, and if you restart VS Code with the example project already created, this doesn't repro?

If so, the fix is probably to ensure we detect when a Flutter project appears in a Dart-only workspace and re-initialize (which will have the effect of selecting the Flutter SDK instead of a pure Dart SDK).

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label Aug 29, 2023
@pattobrien
Copy link
Author

@DanTup the issue is reproducible both when creating the projects via VSCode terminal and in a different terminal (Kitty). The problem also persists even after restarting VSCode.

However, running dart pub get via CLI on the root project works fine:

❯ dart pub get
Resolving dependencies... 
Got dependencies!
Resolving dependencies in ./example... 
Got dependencies in ./example.

@pattobrien
Copy link
Author

I don't think this is related, but FWIW I've noticed that two different processes run in the workspace:

dart pub get runs on both packages:

Screenshot 2023-08-29 at 5 46 36 PM

flutter pub get runs on both packages:

Screenshot 2023-08-29 at 5 44 45 PM

So pub get is still run on example, but I get the exit 69 pop up on the console output 100% of the time I save either pubspec (which is obviously annoying).

I assume that either A) dart pub get should run with --no-example flag, and allow the flutter pub get process to handle the nested example package, or B) only the dart pub get process should be run on both (though the bug would still need to be fixed).

@DanTup
Copy link
Member

DanTup commented Aug 30, 2023

I think we should definitely add --no-example and that will solve the issue. Although I'm curious why running dart pub get in the parent folder from VS Code is failing for you. I can't reproduce this - for me it works just like it does from the CLI for you. dart pub get seems to run for the nested example folder (without complaining that it can't find Flutter).

@DanTup
Copy link
Member

DanTup commented Aug 30, 2023

dart pub get runs on both packages:
flutter pub get runs on both packages:

Are you sure this is happening, and it's not that:

  • dart pub get runs on the root folder, and tries (itself) to run in example (because there's no --no-example)
  • flutter pub get runs on the example folder

That's the behaviour I see (minus the error from the first step) and that I expect.

(I've pushed a fix to use --no-example anyway, because we handle nested folders directly ourselves and don't want to duplicate work)

@DanTup DanTup closed this as completed in ed354f9 Aug 30, 2023
@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label Aug 30, 2023
@pattobrien
Copy link
Author

I think we should definitely add --no-example and that will solve the issue. Although I'm curious why running dart pub get in the parent folder from VS Code is failing for you. I can't reproduce this - for me it works just like it does from the CLI for you. dart pub get seems to run for the nested example folder (without complaining that it can't find Flutter).

Interesting.. I do have some third-party tools installed - namely fvm for managing flutter versions - so perhaps this is a configuration issue (though I did check my settings before submitting the issue). I'll still keep an eye out for possible causes, in case its of value.

Thanks for pushing such a quick fix!

@DanTup
Copy link
Member

DanTup commented Aug 30, 2023

I do have some third-party tools installed - namely fvm for managing flutter versions

If you're able to confirm whether this is what's doing it, I'd be interested to know. If the dart you're running from the terminal is the same dart that we're finding (at bin/cache/dart-sdk/bin/dart from the detected Flutter SDK) I would've expected it to still behave the same.

It does feel like there's a bug here somewhere, but I don't know whose it is 🙃

@pattobrien
Copy link
Author

Okay, I was able to track down and fix the issue and find a way to reproduce it somewhat, by changing the FLUTTER_ROOT env variable:

# "Cant find Flutter SDK" appears when using tilde:
export FLUTTER_ROOT="~/.fvm/default"

# Fixed by using $HOME variable:
export FLUTTER_ROOT="$HOME/.fvm/default"

From a brief lookup, the tilde is allowed syntax for zsh, but seems like Dart Code fails to parse that path properly.

Here's all of the Flutter and Dart-related exports in my zsh config:

# Flutter and Dart setup
export PATH="$PATH:${HOME}/.fvm/default/bin"
export PATH="$PATH:${HOME}/.fvm/default/bin/cache/dart-sdk/bin"

export PATH="$PATH":"$HOME/.pub-cache/bin"

export PATH="${HOME}/Library/Android/sdk/tools:${HOME}/Library/Android/sdk/platform-tools:${PATH}"
export PATH="$JAVA_HOME/bin:$PATH"

export PUB_CACHE="$HOME/.pub-cache"
export FVM_HOME="$HOME/.fvm"
export FLUTTER_ROOT="$HOME/.fvm/default" # line which fixed the issue

And my Dart Code-related VSCode settings:

{
  "dart.addSdkToTerminalPath": true,
  "dart.projectSearchDepth": 5,
  "dart.flutterSdkPath": "~/.fvm/default",
  "dart.flutterSdkPaths": ["~/.fvm/versions/"],
  "dart.checkForSdkUpdates": false,
  "dart.flutterGenerateLocalizationsOnSave": "allIfDirty",
  "dart.runPubGetOnNestedProjects": "both",
  "dart.renameFilesWithClasses": "prompt",
  "dart.previewFlutterUiGuides": true,
  "dart.showInspectorNotificationsForWidgetErrors": false,
  "dart.debugExternalPackageLibraries": true,
  "dart.debugSdkLibraries": true,
  "dart.warnWhenEditingFilesInPubCache": false,
  ...
}

@sigurdm
Copy link

sigurdm commented Aug 31, 2023

@sigurdm does this seem like a Pub bug? Should pub do this when the example is Flutter if it's running without Flutter?

Not sure it is a bug really.

pub get now by default also fetches dependencies in example/ and if that requires Flutter you should run it with FLUTTER_ROOT set up.

I believe FLUTTER_ROOT is set by flutter/bin/flutter and flutter/bin/dart.

If you want to develop on the package with a non-flutter sdk (and thus be unable to test the example), you'll have to resort to --no-example, but I think that is really a minority case.

For Dart Code I think it makes some sense to always use --no-example, though it will probably be slower (because when several resolutions happens in the same call to pub get we can reuse package listings).

@DanTup
Copy link
Member

DanTup commented Aug 31, 2023

@pattobrien thanks for digging!

From a brief lookup, the tilde is allowed syntax for zsh, but seems like Dart Code fails to parse that path properly.

I don't think Dart-Code is doing anything with this variable, I think it's pub that will be using it. However I'm also not sure it should need to handle it - ~ is normally expanded by the shell, but I think your quotes are stopping it:

export A=~/foo
echo $A            # /Users/danny/foo
export A="~/foo"
echo $A            # ~/foo

So I think you should either remove the quotes or use $HOME. That will solve the issue today, although example will be resolved twice (once as the nested project and then once as Dart-Code does it explicitly). When the fix I made above ships, we'll be passing --no-example.

@sigurdm thanks - I also think there's probably no bug here now.

For Dart Code I think it makes some sense to always use --no-example, though it will probably be slower (because when several resolutions happens in the same call to pub get we can reuse package listings).

Yep, understood. It can always be slower because we might call this 10 times if the workspace has 10 projects in. If pub gets a batch mode that would let us provide a bunch of folders (which may or may not be nested) that would definitely speed things up but I don't know how worthwhile that is. The only time I really notice this taking a while is if I open the Flutter repo and it needs running for every folder 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in commands Relates to commands (usually invoked from the command Palette) is bug
Projects
None yet
Development

No branches or pull requests

3 participants