Skip to content

Commit

Permalink
Use located Flutter instead of PATH flutter for Command.flutter() + s…
Browse files Browse the repository at this point in the history
…upport FLUTTER_ROOT override

This fixes a TODO where Command.flutter() would always run the version of Flutter from PATH and not the same one we would detect.

This will allow me to fix an issue when using a custom version of DevTools in VS Code (calling "devtools_tool serve") without having to ensure my PATH points at the same version (which it probably doesn't when I'm just opening test apps that use my "normal" checkout of Flutter).
  • Loading branch information
DanTup committed Nov 7, 2023
1 parent 51228ba commit 8a0ccff
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 8 deletions.
2 changes: 1 addition & 1 deletion tool/lib/commands/analyze.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class AnalyzeCommand extends Command {

@override
Future run() async {
final sdk = FlutterSdk.getSdk();
final sdk = FlutterSdk.current;
if (sdk == null) {
print('Unable to locate a Flutter sdk.');
return 1;
Expand Down
2 changes: 1 addition & 1 deletion tool/lib/commands/pub_get.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class PubGetCommand extends Command {

@override
Future run() async {
final sdk = FlutterSdk.getSdk();
final sdk = FlutterSdk.current;
if (sdk == null) {
print('Unable to locate a Flutter sdk.');
return 1;
Expand Down
2 changes: 1 addition & 1 deletion tool/lib/commands/update_flutter_sdk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class UpdateFlutterSdkCommand extends Command {
);

if (updateLocalFlutter) {
final sdk = FlutterSdk.getSdk();
final sdk = FlutterSdk.current;
if (sdk == null) {
print('Unable to locate a Flutter sdk.');
return 1;
Expand Down
16 changes: 15 additions & 1 deletion tool/lib/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,15 @@ class DevToolsRepo {
class FlutterSdk {
FlutterSdk._(this.sdkPath);

/// 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.
static final current = _findSdk();

/// Return the Flutter SDK.
///
/// This can return null if the Flutter SDK can't be found.
static FlutterSdk? getSdk() {
static FlutterSdk? _findSdk() {
// TODO(dantup): Everywhere that calls this just prints and exits - should
// we just make this throw like DevToolsRepo.getInstance();
// Look for it relative to the current Dart process.
Expand All @@ -127,6 +132,15 @@ class FlutterSdk {
}
}

// 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.
// TODO(dantup): This won't work on Windows.
final result = Process.runSync('which', ['flutter']);
Expand Down
11 changes: 7 additions & 4 deletions tool/lib/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,13 @@ class CliCommand {
String args, {
bool throwOnException = true,
}) {
final sdk = FlutterSdk.current;
if (sdk == null) {
throw Exception('Unable to locate a Flutter sdk.');
}

return CliCommand._(
// TODO(dantup): Accept an instance of FlutterSdk instead of relying on
// PATH here?
exe: FlutterSdk.flutterExecutableName,
exe: sdk.flutterToolPath,
args: args.split(' '),
throwOnException: throwOnException,
);
Expand Down Expand Up @@ -224,4 +227,4 @@ extension JoinExtension on List<String> {
String joinWithNewLine() {
return '${join('\n')}\n';
}
}
}

0 comments on commit 8a0ccff

Please sign in to comment.