From 9e5e763ebe689795dd4d2e9fb1674e1ed807ca91 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 27 Oct 2020 15:20:07 -0700 Subject: [PATCH] [flutter_tools] document flutter root initialization (#67884) Moves the flutter root initialization to a static method on the cache. This is a small step towards making this functionality non-static and instead injected like normal members - however, completely removing all of the static-ness at once was too large of a change. Instead document and add unit tests and change existing code as little as possible. #47161 --- packages/flutter_tools/lib/src/cache.dart | 65 +++++++++++ .../src/runner/flutter_command_runner.dart | 49 ++------- .../permeable/analyze_once_test.dart | 8 +- .../commands/flutter_root_test.dart | 104 ++++++++++++++++++ .../runner/flutter_command_runner_test.dart | 1 - 5 files changed, 185 insertions(+), 42 deletions(-) create mode 100644 packages/flutter_tools/test/general.shard/commands/flutter_root_test.dart diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index 6588cd16c89..f84d3e24841 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -21,12 +21,14 @@ import 'base/net.dart'; import 'base/os.dart' show OperatingSystemUtils; import 'base/platform.dart'; import 'base/process.dart'; +import 'base/user_messages.dart'; import 'convert.dart'; import 'dart/package_map.dart'; import 'dart/pub.dart'; import 'features.dart'; import 'globals.dart' as globals; import 'runner/flutter_command.dart'; +import 'runner/flutter_command_runner.dart'; /// A tag for a set of development artifacts that need to be cached. class DevelopmentArtifact { @@ -215,6 +217,69 @@ class Cache { // Initialized by FlutterCommandRunner on startup. static String flutterRoot; + + /// Determine the absolute and normalized path for the root of the current + /// Flutter checkout. + /// + /// This method has a series of fallbacks for determining the repo location. The + /// first success will immediately return the root without further checks. + /// + /// The order of these tests is: + /// 1. FLUTTER_ROOT environment variable contains the path. + /// 2. Platform script is a data URI scheme, returning `../..` to support + /// tests run from `packages/flutter_tools`. + /// 3. Platform script is package URI scheme, returning the grandparent directory + /// of the package config file location from `packages/flutter_tools/.packages`. + /// 4. Platform script file path is the snapshot path generated by `bin/flutter`, + /// returning the grandparent directory from `bin/cache`. + /// 5. Platform script file name is the entrypoint in `packages/flutter_tools/bin/flutter_tools.dart`, + /// returning the 4th parent directory. + /// 6. The current directory + /// + /// If an exception is thrown during any of these checks, an error message is + /// printed and `.` is returned by default (6). + static String defaultFlutterRoot({ + @required Platform platform, + @required FileSystem fileSystem, + @required UserMessages userMessages, + }) { + String normalize(String path) { + return fileSystem.path.normalize(fileSystem.path.absolute(path)); + } + if (platform.environment.containsKey(kFlutterRootEnvironmentVariableName)) { + return normalize(platform.environment[kFlutterRootEnvironmentVariableName]); + } + try { + if (platform.script.scheme == 'data') { + return normalize('../..'); // The tool is running as a test. + } + final String Function(String) dirname = fileSystem.path.dirname; + + if (platform.script.scheme == 'package') { + final String packageConfigPath = Uri.parse(platform.packageConfig).toFilePath( + windows: platform.isWindows, + ); + return normalize(dirname(dirname(dirname(packageConfigPath)))); + } + + if (platform.script.scheme == 'file') { + final String script = platform.script.toFilePath( + windows: platform.isWindows, + ); + if (fileSystem.path.basename(script) == kSnapshotFileName) { + return normalize(dirname(dirname(fileSystem.path.dirname(script)))); + } + if (fileSystem.path.basename(script) == kFlutterToolsScriptFileName) { + return normalize(dirname(dirname(dirname(dirname(script))))); + } + } + } on Exception catch (error) { + // There is currently no logger attached since this is computed at startup. + print(userMessages.runnerNoRoot('$error')); + } + return normalize('.'); + } + // Whether to cache artifacts for all platforms. Defaults to only caching // artifacts for the current platform. bool includeAllPlatforms = false; diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart index 13f6436d801..8d6da84389c 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart @@ -156,43 +156,6 @@ class FlutterCommandRunner extends CommandRunner { return '$prefix\n\n$usageWithoutDescription'; } - static String get defaultFlutterRoot { - if (globals.platform.environment.containsKey(kFlutterRootEnvironmentVariableName)) { - return globals.platform.environment[kFlutterRootEnvironmentVariableName]; - } - try { - if (globals.platform.script.scheme == 'data') { - return '../..'; // we're running as a test - } - - if (globals.platform.script.scheme == 'package') { - final String packageConfigPath = Uri.parse(globals.platform.packageConfig).toFilePath(); - return globals.fs.path.dirname(globals.fs.path.dirname(globals.fs.path.dirname(packageConfigPath))); - } - - final String script = globals.platform.script.toFilePath(); - if (globals.fs.path.basename(script) == kSnapshotFileName) { - return globals.fs.path.dirname(globals.fs.path.dirname(globals.fs.path.dirname(script))); - } - if (globals.fs.path.basename(script) == kFlutterToolsScriptFileName) { - return globals.fs.path.dirname(globals.fs.path.dirname(globals.fs.path.dirname(globals.fs.path.dirname(script)))); - } - - // If run from a bare script within the repo. - if (script.contains('flutter/packages/')) { - return script.substring(0, script.indexOf('flutter/packages/') + 8); - } - if (script.contains('flutter/examples/')) { - return script.substring(0, script.indexOf('flutter/examples/') + 8); - } - } on Exception catch (error) { - // we don't have a logger at the time this is run - // (which is why we don't use printTrace here) - print(userMessages.runnerNoRoot('$error')); - } - return '.'; - } - @override ArgResults parse(Iterable args) { try { @@ -264,7 +227,11 @@ class FlutterCommandRunner extends CommandRunner { // We must set Cache.flutterRoot early because other features use it (e.g. // enginePath's initializer uses it). - final String flutterRoot = topLevelResults['flutter-root'] as String ?? defaultFlutterRoot; + final String flutterRoot = topLevelResults['flutter-root'] as String ?? Cache.defaultFlutterRoot( + platform: globals.platform, + fileSystem: globals.fs, + userMessages: globals.userMessages, + ); Cache.flutterRoot = globals.fs.path.normalize(globals.fs.path.absolute(flutterRoot)); // Set up the tooling configuration. @@ -339,7 +306,11 @@ class FlutterCommandRunner extends CommandRunner { @visibleForTesting static void initFlutterRoot() { - Cache.flutterRoot ??= defaultFlutterRoot; + Cache.flutterRoot ??= Cache.defaultFlutterRoot( + platform: globals.platform, + fileSystem: globals.fs, + userMessages: globals.userMessages, + ); } /// Get the root directories of the repo - the directories containing Dart packages. diff --git a/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart b/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart index d1274626abd..09555e08c9c 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:flutter_tools/src/base/error_handling_io.dart'; +import 'package:flutter_tools/src/base/user_messages.dart'; import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/base/common.dart'; @@ -14,7 +15,6 @@ import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/analyze.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; -import 'package:flutter_tools/src/runner/flutter_command_runner.dart'; import 'package:process/process.dart'; import '../../src/common.dart'; @@ -108,7 +108,6 @@ void main() { setUpAll(() { Cache.disableLocking(); - Cache.flutterRoot = FlutterCommandRunner.defaultFlutterRoot; processManager = const LocalProcessManager(); platform = const LocalPlatform(); terminal = AnsiTerminal(platform: platform, stdio: Stdio()); @@ -123,6 +122,11 @@ void main() { fileSystem: fileSystem, platform: platform, ); + Cache.flutterRoot = Cache.defaultFlutterRoot( + fileSystem: fileSystem, + platform: platform, + userMessages: UserMessages(), + ); }); setUp(() { diff --git a/packages/flutter_tools/test/general.shard/commands/flutter_root_test.dart b/packages/flutter_tools/test/general.shard/commands/flutter_root_test.dart new file mode 100644 index 00000000000..5a44537f3ed --- /dev/null +++ b/packages/flutter_tools/test/general.shard/commands/flutter_root_test.dart @@ -0,0 +1,104 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/platform.dart'; +import 'package:flutter_tools/src/base/user_messages.dart'; +import 'package:flutter_tools/src/cache.dart'; + +import '../../src/common.dart'; + +void main() { + testWithoutContext('Cache can initialize flutter root from environment variable', () { + final String defaultFlutterRoot = Cache.defaultFlutterRoot( + fileSystem: MemoryFileSystem.test(), + userMessages: UserMessages(), + platform: FakePlatform( + environment: { + 'FLUTTER_ROOT': 'path/to/flutter' + } + ) + ); + + expect(defaultFlutterRoot, '/path/to/flutter'); + }); + + testWithoutContext('Cache can initialize flutter root data-scheme platform script', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + // For data-uri, the root is initialized to ../.. and then normalized. Change the + // current directory to verify this. + final Directory directory = fileSystem.directory('foo/bar/baz/') + ..createSync(recursive: true); + fileSystem.currentDirectory = directory; + + final String defaultFlutterRoot = Cache.defaultFlutterRoot( + fileSystem: fileSystem, + userMessages: UserMessages(), + platform: FakePlatform( + environment: {}, + script: Uri.parse('data:,Hello%2C%20World!'), + ) + ); + + expect(defaultFlutterRoot, '/foo'); + }); + + testWithoutContext('Cache can initialize flutter root package-scheme platform script', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + final String defaultFlutterRoot = Cache.defaultFlutterRoot( + fileSystem: fileSystem, + userMessages: UserMessages(), + platform: FakePlatform( + environment: {}, + script: Uri.parse('package:flutter_tools/flutter_tools.dart'), + packageConfig: 'flutter/packages/flutter_tools/.packages' + ) + ); + + expect(defaultFlutterRoot, '/flutter'); + }); + + testWithoutContext('Cache can initialize flutter root from snapshot location', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + final String defaultFlutterRoot = Cache.defaultFlutterRoot( + fileSystem: fileSystem, + userMessages: UserMessages(), + platform: FakePlatform( + environment: {}, + script: Uri.parse('file:///flutter/bin/cache/flutter_tools.snapshot'), + ) + ); + + expect(defaultFlutterRoot, '/flutter'); + }); + + testWithoutContext('Cache can initialize flutter root from script file', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + final String defaultFlutterRoot = Cache.defaultFlutterRoot( + fileSystem: fileSystem, + userMessages: UserMessages(), + platform: FakePlatform( + environment: {}, + script: Uri.parse('file:///flutter/packages/flutter_tools/bin/flutter_tools.dart'), + ) + ); + + expect(defaultFlutterRoot, '/flutter'); + }); + + testWithoutContext('Cache will default to current directory if there are no matches', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + final String defaultFlutterRoot = Cache.defaultFlutterRoot( + fileSystem: fileSystem, + userMessages: UserMessages(), + platform: FakePlatform( + environment: {}, + script: Uri.parse('http://foo.bar'), // does not match any heuristics. + ) + ); + + expect(defaultFlutterRoot, '/'); + }); +} diff --git a/packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart b/packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart index c4ce2160743..69adbb95075 100644 --- a/packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart @@ -32,7 +32,6 @@ void main() { setUpAll(() { Cache.disableLocking(); - Cache.flutterRoot = FlutterCommandRunner.defaultFlutterRoot; }); setUp(() {