Skip to content

Commit

Permalink
Revert "Revert "Always use tool/flutter-sdk for devtools_tool + remot…
Browse files Browse the repository at this point in the history
…e FLUTTER_ROOT (#6776)" (#6820)"

This reverts commit 79cde66.
  • Loading branch information
DanTup committed Nov 23, 2023
1 parent d0a7090 commit f21229b
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 93 deletions.
18 changes: 9 additions & 9 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}

- name: tool/ci/bots.sh
Expand All @@ -55,7 +55,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: Run tool/ci/bots.sh
run: ./tool/ci/bots.sh
Expand All @@ -65,7 +65,7 @@ jobs:
wget -qO- https://dcm.dev/pgp-key.public | sudo gpg --dearmor -o /usr/share/keyrings/dcm.gpg
echo 'deb [signed-by=/usr/share/keyrings/dcm.gpg arch=amd64] https://dcm.dev/debian stable main' | sudo tee /etc/apt/sources.list.d/dart_stable.list
sudo apt-get update
sudo apt-get install dcm=1.11.0-1
sudo apt-get install dcm=1.11.0-1
sudo chmod +x /usr/bin/dcm
echo "$(dcm --version)"
- name: Setup Dart SDK
Expand All @@ -92,7 +92,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: tool/ci/package_tests.sh
env:
Expand All @@ -118,7 +118,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: tool/ci/bots.sh
env:
Expand All @@ -145,7 +145,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: tool/ci/bots.sh
env:
Expand Down Expand Up @@ -215,7 +215,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: tool/ci/bots.sh
env:
Expand Down Expand Up @@ -248,7 +248,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: tool/ci/bots.sh
env:
Expand All @@ -269,7 +269,7 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ needs.flutter-prep.outputs.latest_flutter_candidate }}
- name: tool/ci/benchmark_tests.sh
run: ./tool/ci/benchmark_tests.sh
Expand Down
14 changes: 7 additions & 7 deletions .github/workflows/flutter-prep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,21 @@ jobs:
uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
with:
path: |
./flutter-sdk
./tool/flutter-sdk
key: flutter-sdk-${{ runner.os }}-${{ steps.flutter-candidate.outputs.FLUTTER_CANDIDATE }}

- if: ${{ steps.cache-flutter.outputs.cache-hit != 'true' }}
name: Clone Flutter SDK if none cached
run: |
git clone https://github.com/flutter/flutter.git ./flutter-sdk
cd flutter-sdk
git clone https://github.com/flutter/flutter.git ./tool/flutter-sdk
cd tool/flutter-sdk
git checkout $LATEST_FLUTTER_CANDIDATE
env:
LATEST_FLUTTER_CANDIDATE: ${{ steps.flutter-candidate.outputs.FLUTTER_CANDIDATE }}

- name: Assert that the Latest Flutter Candidate is checked out
run: |
cd flutter-sdk
cd tool/flutter-sdk
HEAD_SHA=$(git rev-parse HEAD)
LATEST_FLUTTER_CANDIDATE_SHA=$(git rev-list -1 "$LATEST_FLUTTER_CANDIDATE")
if [ "$HEAD_SHA" != "$LATEST_FLUTTER_CANDIDATE_SHA" ]; then
Expand All @@ -68,6 +68,6 @@ jobs:

- name: Setup Flutter SDK
run: |
./flutter-sdk/bin/flutter config --no-analytics
./flutter-sdk/bin/flutter doctor
./flutter-sdk/bin/cache/dart-sdk/bin/dart --disable-analytics
./tool/flutter-sdk/bin/flutter config --no-analytics
./tool/flutter-sdk/bin/flutter doctor
./tool/flutter-sdk/bin/cache/dart-sdk/bin/dart --disable-analytics
2 changes: 1 addition & 1 deletion tool/bin/devtools_tool
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/bin/bash -e
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
dart run "$SCRIPT_DIR/devtools_tool.dart" "$@"
"$SCRIPT_DIR/../flutter-sdk/bin/dart" run "$SCRIPT_DIR/devtools_tool.dart" "$@"
11 changes: 1 addition & 10 deletions tool/bin/devtools_tool.bat
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
@echo off

pushd "%~dp0"
dart run ./devtools_tool.dart %*
%~dp0/../flutter-sdk/bin/dart run %~dp0/devtools_tool.dart %*

IF %errorlevel% NEQ 0 GOTO :error

popd
EXIT /B 0

:error
popd
EXIT /B %errorlevel%

6 changes: 3 additions & 3 deletions tool/ci/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ if [[ $RUNNER_OS == "Windows" ]]; then
fi

# Make sure Flutter sdk has been provided
if [ ! -d "./flutter-sdk" ]; then
echo "Expected ./flutter-sdk to exist"
if [ ! -d "./tool/flutter-sdk" ]; then
echo "Expected ./tool/flutter-sdk to exist"
exit 1;
fi

# Look in the dart bin dir first, then the flutter one, then the one for the
# devtools repo. We don't use the dart script from flutter/bin as that script
# can and does print 'Waiting for another flutter command...' at inopportune
# times.
export PATH=`pwd`/flutter-sdk/bin/cache/dart-sdk/bin:`pwd`/flutter-sdk/bin:`pwd`/bin:$PATH
export PATH=`pwd`/tool/flutter-sdk/bin/cache/dart-sdk/bin:`pwd`/tool/flutter-sdk/bin:`pwd`/bin:$PATH

# Look up the latest flutter candidate (this is the latest flutter version in g3)
# TODO(https://github.com/flutter/devtools/issues/4591): re-write this script as a
Expand Down
5 changes: 0 additions & 5 deletions tool/lib/commands/analyze.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ class AnalyzeCommand extends Command {
@override
Future run() async {
final sdk = FlutterSdk.current;
if (sdk == null) {
print('Unable to locate a Flutter sdk.');
return 1;
}

final log = Logger.standard();
final repo = DevToolsRepo.getInstance();
final processManager = ProcessManager();
Expand Down
7 changes: 3 additions & 4 deletions tool/lib/commands/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import '../utils.dart';
class BuildCommand extends Command {
BuildCommand() {
argParser
..addUseFlutterFromPathFlag()
..addUpdateFlutterFlag()
..addUpdatePerfettoFlag()
..addPubGetFlag()
..addBulidModeOption();
Expand All @@ -54,8 +54,7 @@ class BuildCommand extends Command {
final repo = DevToolsRepo.getInstance();
final processManager = ProcessManager();

final useLocalFlutter =
argResults![BuildCommandArgs.useFlutterFromPath.flagName];
final updateFlutter = argResults![BuildCommandArgs.updateFlutter.flagName];
final updatePerfetto =
argResults![BuildCommandArgs.updatePerfetto.flagName];
final runPubGet = argResults![BuildCommandArgs.pubGet.flagName];
Expand All @@ -64,7 +63,7 @@ class BuildCommand extends Command {
final webBuildDir =
Directory(path.join(repo.devtoolsAppDirectoryPath, 'build', 'web'));

if (!useLocalFlutter) {
if (updateFlutter) {
logStatus('updating tool/flutter-sdk to the latest flutter candidate');
await processManager.runProcess(CliCommand.tool('update-flutter-sdk'));
}
Expand Down
6 changes: 0 additions & 6 deletions tool/lib/commands/pub_get.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ class PubGetCommand extends Command {

@override
Future run() async {
final sdk = FlutterSdk.current;
if (sdk == null) {
print('Unable to locate a Flutter sdk.');
return 1;
}

final log = Logger.standard();
final repo = DevToolsRepo.getInstance();
final processManager = ProcessManager();
Expand Down
11 changes: 6 additions & 5 deletions tool/lib/commands/serve.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ServeCommand extends Command {
'Whether to build the DevTools web app before starting the DevTools'
' server.',
)
..addUseFlutterFromPathFlag()
..addUpdateFlutterFlag()
..addUpdatePerfettoFlag()
..addPubGetFlag()
..addBulidModeOption()
Expand Down Expand Up @@ -87,16 +87,16 @@ class ServeCommand extends Command {
final processManager = ProcessManager();

final buildApp = argResults![_buildAppFlag];
final useFlutterFromPath =
argResults![BuildCommandArgs.useFlutterFromPath.flagName];
final updateFlutter = argResults![BuildCommandArgs.updateFlutter.flagName];
final updatePerfetto =
argResults![BuildCommandArgs.updatePerfetto.flagName];
final runPubGet = argResults![BuildCommandArgs.pubGet.flagName];
final devToolsAppBuildMode =
argResults![BuildCommandArgs.buildMode.flagName];

final remainingArguments = List.of(argResults!.arguments)
..remove(BuildCommandArgs.useFlutterFromPath.asArg())
..remove(BuildCommandArgs.updateFlutter.asArg())
..remove(BuildCommandArgs.updateFlutter.asArg(negated: true))
..remove(BuildCommandArgs.updatePerfetto.asArg())
..remove(valueAsArg(_buildAppFlag))
..remove(valueAsArg(_buildAppFlag, negated: true))
Expand Down Expand Up @@ -125,11 +125,12 @@ class ServeCommand extends Command {

final devToolsBuildLocation =
path.join(repo.devtoolsAppDirectoryPath, 'build', 'web');

if (buildApp) {
final process = await processManager.runProcess(
CliCommand.tool(
'build'
'${useFlutterFromPath ? ' ${BuildCommandArgs.useFlutterFromPath.asArg()}' : ''}'
' ${BuildCommandArgs.updateFlutter.asArg(negated: !updateFlutter)}'
'${updatePerfetto ? ' ${BuildCommandArgs.updatePerfetto.asArg()}' : ''}'
' ${BuildCommandArgs.buildMode.asArg()}=$devToolsAppBuildMode'
' ${BuildCommandArgs.pubGet.asArg(negated: !runPubGet)}',
Expand Down
14 changes: 7 additions & 7 deletions tool/lib/commands/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,21 @@ extension BuildCommandArgsExtension on ArgParser {
);
}

void addUseFlutterFromPathFlag() {
void addUpdateFlutterFlag() {
addFlag(
BuildCommandArgs.useFlutterFromPath.flagName,
negatable: false,
defaultsTo: false,
help: 'Whether to use the Flutter SDK on PATH instead of the Flutter SDK '
'contained in the "tool/flutter-sdk" directory.',
BuildCommandArgs.updateFlutter.flagName,
negatable: true,
defaultsTo: true,
help: 'Whether to update the Flutter SDK contained in the '
'"tool/flutter-sdk" directory.',
);
}
}

enum BuildCommandArgs {
buildMode('build-mode'),
pubGet('pub-get'),
useFlutterFromPath('use-flutter-from-path'),
updateFlutter('update-flutter'),
updatePerfetto('update-perfetto');

const BuildCommandArgs(this.flagName);
Expand Down
4 changes: 0 additions & 4 deletions tool/lib/commands/update_flutter_sdk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ class UpdateFlutterSdkCommand extends Command {

if (updateFlutterFromPath) {
final sdk = FlutterSdk.current;
if (sdk == null) {
print('Unable to locate a Flutter sdk.');
return 1;
}

log.stdout('Updating local flutter/flutter repository...');

Expand Down
44 changes: 16 additions & 28 deletions tool/lib/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,21 @@ class FlutterSdk {

/// The current located Flutter SDK (or `null` if one could not be found).
///
/// Tries to locate from the running Dart VM, FLUTTER_ROOT or searching PATH.
/// Tries to locate from the running Dart VM. If not found, will print a
/// warning and use Flutter from PATH.
static final current = _findSdk();

/// Return the Flutter SDK.
/// Finds the Flutter SDK that contains the Dart VM being used to run this
/// script.
///
/// This can return null if the Flutter SDK can't be found.
static FlutterSdk? _findSdk() {
// TODO(dantup): Everywhere that calls this just prints and exits - should
// we just make this throw like DevToolsRepo.getInstance();
/// Throws if the current VM is not inside a Flutter SDK.
static FlutterSdk _findSdk() {
// Look for it relative to the current Dart process.
final dartVmPath = Platform.resolvedExecutable;
final pathSegments = path.split(dartVmPath);
// TODO(dantup): Should we add tool/flutter-sdk to the front here, to
// ensure we _only_ ever use this one, to avoid potentially updating a
// different Flutter if the user runs explicitly with another Flutter?
final expectedSegments = path.posix.split('bin/cache/dart-sdk/bin/dart');

if (pathSegments.length >= expectedSegments.length) {
Expand All @@ -128,31 +131,16 @@ class FlutterSdk {
}

if (expectedSegments.isEmpty) {
return FlutterSdk._(path.joinAll(pathSegments));
final flutterSdkRoot = path.joinAll(pathSegments);
print('Using Flutter SDK from $flutterSdkRoot');
return FlutterSdk._(flutterSdkRoot);
}
}

// Next try FLUTTER_ROOT to allow using a custom flutter (eg. from
// `tool/flutter-sdk`) when invoking this without needing to override `PATH`
// (it's easier to set override an entire env variable than prepend to one
// in some places like the `dart.customDevTools` setting in VS Code).
final flutterRootEnv = Platform.environment['FLUTTER_ROOT'];
if (flutterRootEnv != null && flutterRootEnv.isNotEmpty) {
return FlutterSdk._(flutterRootEnv);
}

// Look to see if we can find the 'flutter' command in the PATH.
final whichCommand = Platform.isWindows ? 'where.exe' : 'which';
final result = Process.runSync(whichCommand, ['flutter']);
if (result.exitCode == 0) {
final sdkPath = result.stdout.toString().split('\n').first.trim();
// 'flutter/bin'
if (path.basename(path.dirname(sdkPath)) == 'bin') {
return FlutterSdk._(path.dirname(path.dirname(sdkPath)));
}
}

return null;
throw Exception(
'Unable to locate the Flutter SDK from the current running Dart VM:\n'
'${Platform.resolvedExecutable}',
);
}

final String sdkPath;
Expand Down
4 changes: 0 additions & 4 deletions tool/lib/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ class CliCommand {
bool throwOnException = true,
}) {
final sdk = FlutterSdk.current;
if (sdk == null) {
throw Exception('Unable to locate a Flutter sdk.');
}

return CliCommand._(
exe: sdk.flutterToolPath,
args: args.split(' '),
Expand Down

0 comments on commit f21229b

Please sign in to comment.