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

Always use tool/flutter-sdk for devtools_tool + remote FLUTTER_ROOT #6776

Merged
merged 7 commits into from
Nov 21, 2023

Conversation

DanTup
Copy link
Contributor

@DanTup DanTup commented Nov 20, 2023

This removes some code I added to support using FLUTTER_ROOT for the SDK and instead just updates the devtools_tool shell scripts to always run from tools/flutter-sdk and just locate the Flutter SDK from the running VM.

This makes the assumption that we always want to use the DevTools version of Flutter and avoids any issues where you might have something different on PATH.

It also renames the --use-flutter-from-path flag to --update-flutter/--no-update-flutter (and retains the default of updating).

This removes some code I added to support using FLUTTER_ROOT for the SDK and instead just updates the devtools_tool shell scripts to always run from tools/flutter-sdk and just locate the Flutter SDK from the running VM.

This makes the assumption that we _always_ want to use the DevTools version of Flutter and avoids any issues where you might have something different on PATH.

It also renames the --use-flutter-from-path flag to --update-flutter/--no-update-flutter (and retains the default of updating).
@DanTup DanTup requested a review from a team as a code owner November 20, 2023 11:18
@DanTup DanTup changed the title Always use tool/flutters-sdk for devtools_tool + remote FLUTTER_ROOT Always use tool/flutter-sdk for devtools_tool + remote FLUTTER_ROOT Nov 20, 2023
BuildCommandArgs.updateFlutter.flagName,
negatable: true,
defaultsTo: true,
help: 'Whether to update the Flutter SDK before building DevTools '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove "before building DevTools" because this flag can be used for more than just the build command.

After removing, it should say 'Whether to update the Flutter SDK contained in the "tool/flutter-sdk" directory'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

  • another minor fix I forgot to push earlier, which ensures if you pass --no-update-flutter we remove it before passing on to the local server.

Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment but lgtm

@DanTup DanTup added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 20, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 20, 2023
Copy link

auto-submit bot commented Nov 20, 2023

auto label is removed for flutter/devtools/6776, due to - The status or check suite benchmarks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@DanTup DanTup added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2023
Copy link

auto-submit bot commented Nov 21, 2023

auto label is removed for flutter/devtools/6776, due to - The status or check suite benchmarks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@DanTup DanTup added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2023
@auto-submit auto-submit bot merged commit 20f3dc6 into flutter:master Nov 21, 2023
23 checks passed
zanderso added a commit that referenced this pull request Nov 22, 2023
jacob314 pushed a commit that referenced this pull request Nov 22, 2023
@jacob314
Copy link
Contributor

Reverted this temporarily to unblock Flutter.

DanTup added a commit to DanTup/devtools that referenced this pull request Nov 23, 2023
DanTup added a commit that referenced this pull request Nov 29, 2023
* Revert "Revert "Always use tool/flutter-sdk for devtools_tool + remote FLUTTER_ROOT (#6776)" (#6820)"

This reverts commit 79cde66.

* Add an env var to force devtools_tool to use SDKs from PATH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App release-notes-not-required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants