Launch DevTools from the 'dart devtools' command instead of pub (#90894)

bin/internal/README.md

@@ -25,7 +25,3 @@ of the `flutter/plugins` repository to be used for testing. The
 `flutter/flutter`; it is only used as part of the test suite for
 verification, and the pinned version here makes sure that tests are
 deterministic at each `flutter/flutter` commit.
-
-The `bin/internal/devtools.version` file specifies the version of the
-`devtools` package on `pub` that should be activated when running the
-Flutter command line tool.

bin/internal/devtools.version

-2.6.0

dev/bots/test.dart

@@ -308,25 +308,6 @@ Future<void> _runIntegrationToolTests() async {
       .map<String>((FileSystemEntity entry) => path.relative(entry.path, from: toolsPath))
       .where((String testPath) => path.basename(testPath).endsWith('_test.dart')).toList();
 
-  // Make sure devtools is ready first, because that might take a while and we don't
-  // want any of the tests to time out while they themselves try to activate devtools.
-  final Map<String, String> pubEnvironment = <String, String>{
-    'FLUTTER_ROOT': flutterRoot,
-  };
-  if (Directory(pubCache).existsSync()) {
-    pubEnvironment['PUB_CACHE'] = pubCache;
-  }
-  await runCommand(
-    pub,
-    <String>[
-      'global',
-      'activate',
-      'devtools',
-      File(path.join(flutterRoot, 'bin', 'internal', 'devtools.version')).readAsStringSync(),
-    ],
-    environment: pubEnvironment,
-  );
-
   await _pubRunTest(
     toolsPath,
     forceSingleCore: true,

packages/flutter_tools/lib/executable.dart

@@ -108,11 +108,8 @@ Future<void> main(List<String> args) async {
       // devtools source code.
       DevtoolsLauncher: () => DevtoolsServerLauncher(
         processManager: globals.processManager,
-        fileSystem: globals.fs,
         dartExecutable: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path,
         logger: globals.logger,
-        platform: globals.platform,
-        persistentToolState: globals.persistentToolState,
       ),
       Logger: () {
         final LoggerFactory loggerFactory = LoggerFactory(

packages/flutter_tools/lib/src/context_runner.dart

@@ -215,11 +215,8 @@ Future<T> runInContext<T>(
       ),
       DevtoolsLauncher: () => DevtoolsServerLauncher(
         processManager: globals.processManager,
-        fileSystem: globals.fs,
         dartExecutable: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path,
         logger: globals.logger,
-        platform: globals.platform,
-        persistentToolState: globals.persistentToolState,
       ),
       Doctor: () => Doctor(logger: globals.logger),
       DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,

packages/flutter_tools/lib/src/devtools_launcher.dart

@@ -9,55 +9,31 @@ import 'dart:async';
 import 'package:meta/meta.dart';
 import 'package:process/process.dart';
 
-import 'base/file_system.dart';
 import 'base/io.dart' as io;
 import 'base/logger.dart';
-import 'base/platform.dart';
-import 'cache.dart';
 import 'convert.dart';
-import 'persistent_tool_state.dart';
 import 'resident_runner.dart';
 
 /// An implementation of the devtools launcher that uses `pub global activate` to
 /// start a server instance.
 class DevtoolsServerLauncher extends DevtoolsLauncher {
   DevtoolsServerLauncher({
-    @required Platform platform,
     @required ProcessManager processManager,
-    @required FileSystem fileSystem,
     @required String dartExecutable,
     @required Logger logger,
-    @required PersistentToolState persistentToolState,
-    @visibleForTesting io.HttpClient httpClient,
   })  : _processManager = processManager,
-        _fileSystem = fileSystem,
         _dartExecutable = dartExecutable,
-        _logger = logger,
-        _platform = platform,
-        _persistentToolState = persistentToolState,
-        _httpClient = httpClient ?? io.HttpClient();
+        _logger = logger;
 
   final ProcessManager _processManager;
-  final FileSystem _fileSystem;
   final String _dartExecutable;
   final Logger _logger;
-  final Platform _platform;
-  final PersistentToolState _persistentToolState;
-  final io.HttpClient _httpClient;
   final Completer<void> _processStartCompleter = Completer<void>();
 
   io.Process _devToolsProcess;
 
   static final RegExp _serveDevToolsPattern =
       RegExp(r'Serving DevTools at ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+?)\.?$');
-  static const String _pubHostedUrlKey = 'PUB_HOSTED_URL';
-
-  static String _devtoolsVersion;
-  static String devtoolsVersion(FileSystem fs) {
-    return _devtoolsVersion ??= fs.file(
-      fs.path.join(Cache.flutterRoot, 'bin', 'internal', 'devtools.version'),
-    ).readAsStringSync();
-  }
 
   @override
   Future<void> get processStart => _processStartCompleter.future;
@@ -67,61 +43,8 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
     // Place this entire method in a try/catch that swallows exceptions because
     // this method is guaranteed not to return a Future that throws.
     try {
-      bool offline = false;
-      bool useOverrideUrl = false;
-      try {
-        Uri uri;
-        if (_platform.environment.containsKey(_pubHostedUrlKey)) {
-          useOverrideUrl = true;
-          uri = Uri.parse(_platform.environment[_pubHostedUrlKey]);
-        } else {
-          uri = Uri.https('pub.dev', '');
-        }
-        final io.HttpClientRequest request = await _httpClient.headUrl(uri);
-        final io.HttpClientResponse response = await request.close();
-        await response.drain<void>();
-        if (response.statusCode != io.HttpStatus.ok) {
-          _logger.printTrace(
-            'Skipping devtools launch because pub.dev responded with HTTP '
-            'status code ${response.statusCode} instead of ${io.HttpStatus.ok}.',
-          );
-          offline = true;
-        }
-      } on Exception catch (e) {
-        _logger.printTrace(
-          'Skipping devtools launch because connecting to pub.dev failed with $e',
-        );
-        offline = true;
-      } on ArgumentError {
-        if (!useOverrideUrl) {
-          rethrow;
-        }
-        // The user supplied a custom pub URL that was invalid, pretend to be offline
-        // and inform them that the URL was invalid.
-        offline = true;
-        _logger.printError(
-          'PUB_HOSTED_URL was set to an invalid URL: "${_platform.environment[_pubHostedUrlKey]}".'
-        );
-      }
-
-      bool devToolsActive = await _checkForActiveDevTools();
-      if (!offline) {
-        await _activateDevTools(throttleUpdates: devToolsActive);
-        if (!devToolsActive) {
-          devToolsActive = await _checkForActiveDevTools();
-        }
-      }
-      if (!devToolsActive) {
-        // We don't have devtools installed and installing it failed;
-        // _activateDevTools will have reported the error already.
-        return;
-      }
-
       _devToolsProcess = await _processManager.start(<String>[
         _dartExecutable,
-        'pub',
-        'global',
-        'run',
         'devtools',
         '--no-launch-browser',
         if (vmServiceUri != null) '--vm-uri=$vmServiceUri',
@@ -149,61 +72,6 @@ class DevtoolsServerLauncher extends DevtoolsLauncher {
     }
   }
 
-  static final RegExp _devToolsInstalledPattern = RegExp(r'^devtools ', multiLine: true);
-
-  /// Check if the DevTools package is already active by running "pub global list".
-  Future<bool> _checkForActiveDevTools() async {
-    final io.ProcessResult _pubGlobalListProcess = await _processManager.run(
-      <String>[ _dartExecutable, 'pub', 'global', 'list' ],
-    );
-    return _pubGlobalListProcess.stdout.toString().contains(_devToolsInstalledPattern);
-  }
-
-  /// Helper method to activate the DevTools pub package.
-  ///
-  /// If throttleUpdates is true, then this is a no-op if it was run in
-  /// the last twelve hours. It should be set to true if devtools is known
-  /// to already be installed.
-  ///
-  /// Return value indicates if DevTools was installed or updated.
-  Future<bool> _activateDevTools({@required bool throttleUpdates}) async {
-    assert(throttleUpdates != null);
-    const Duration _throttleDuration = Duration(hours: 12);
-    if (throttleUpdates) {
-      if (_persistentToolState.lastDevToolsActivationTime != null &&
-          DateTime.now().difference(_persistentToolState.lastDevToolsActivationTime) < _throttleDuration) {
-        _logger.printTrace('DevTools activation throttled until ${_persistentToolState.lastDevToolsActivationTime.add(_throttleDuration).toLocal()}.');
-        return false; // Throttled.
-      }
-    }
-    final Status status = _logger.startProgress('Activating Dart DevTools...');
-    try {
-      final io.ProcessResult _devToolsActivateProcess = await _processManager
-          .run(<String>[
-        _dartExecutable,
-        'pub',
-        'global',
-        'activate',
-        'devtools',
-        devtoolsVersion(_fileSystem),
-      ]);
-      if (_devToolsActivateProcess.exitCode != 0) {
-        _logger.printError(
-          'Error running `pub global activate devtools`:\n'
-          '${_devToolsActivateProcess.stderr}'
-        );
-        return false; // Failed to activate.
-      }
-      _persistentToolState.lastDevToolsActivation = DateTime.now();
-      return true; // Activation succeeded!
-    } on Exception catch (e, _) {
-      _logger.printError('Error running `pub global activate devtools`: $e');
-      return false;
-    } finally {
-      status.stop();
-    }
-  }
-
   @override
   Future<DevToolsServerAddress> serve() async {
     if (activeDevToolsServer == null) {

packages/flutter_tools/lib/src/persistent_tool_state.dart

@@ -55,10 +55,6 @@ abstract class PersistentToolState {
   /// Whether this client was already determined to be or not be a bot.
   bool? get isRunningOnBot;
   set runningOnBot(bool value); // Enforced nonnull setter.
-
-  /// The last time the DevTools package was activated from pub.
-  DateTime? get lastDevToolsActivationTime;
-  set lastDevToolsActivation(DateTime value); // Enforced nonnull setter.
 }
 
 class _DefaultPersistentToolState implements PersistentToolState {
@@ -92,7 +88,6 @@ class _DefaultPersistentToolState implements PersistentToolState {
     Channel.stable: 'last-active-stable-version'
   };
   static const String _kBotKey = 'is-bot';
-  static const String _kLastDevToolsActivationTimeKey = 'last-devtools-activation-time';
   static const String _kLicenseHash = 'license-hash';
 
   final Config _config;
@@ -140,14 +135,4 @@ class _DefaultPersistentToolState implements PersistentToolState {
   set runningOnBot(bool value) {
     _config.setValue(_kBotKey, value);
   }
-
-  @override
-  DateTime? get lastDevToolsActivationTime {
-    final String? value = _config.getValue(_kLastDevToolsActivationTimeKey) as String?;
-    return value != null ? DateTime.parse(value) : null;
-  }
-
-  @override
-  set lastDevToolsActivation(DateTime time) =>
-      _config.setValue(_kLastDevToolsActivationTimeKey, time.toString());
 }

packages/flutter_tools/test/general.shard/persistent_tool_state_test.dart

@@ -57,23 +57,4 @@ void main() {
     expect(state2.lastActiveVersion(Channel.beta), 'ghi');
     expect(state2.lastActiveVersion(Channel.stable), 'jkl');
   });
-
-  testWithoutContext('lastDevToolsActivationTime can be cached and stored', () {
-    final MemoryFileSystem fileSystem = MemoryFileSystem.test();
-    final Directory directory = fileSystem.directory('state_dir')..createSync();
-    final PersistentToolState state1 = PersistentToolState.test(
-      directory: directory,
-      logger: BufferLogger.test(),
-    );
-
-    final DateTime time = DateTime.now();
-    state1.lastDevToolsActivation = time;
-
-    final PersistentToolState state2 = PersistentToolState.test(
-      directory: directory,
-      logger: BufferLogger.test(),
-    );
-
-    expect(state2.lastDevToolsActivationTime, equals(time));
-  });
 }

packages/flutter_tools/test/general.shard/resident_devtools_handler_test.dart

@@ -6,9 +6,7 @@
 
 import 'dart:async';
 
-import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/logger.dart';
-import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/device.dart';
@@ -78,10 +76,6 @@ final FakeVmServiceRequest listViews = FakeVmServiceRequest(
 
 void main() {
   Cache.flutterRoot = '';
-  final MemoryFileSystem fakefs = MemoryFileSystem.test()
-    ..directory('bin').createSync()
-    ..directory('bin/internal').createSync()
-    ..file('bin/internal/devtools.version').writeAsStringSync('1.0.0');
 
   testWithoutContext('Does not serve devtools if launcher is null', () async {
     final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
@@ -110,11 +104,8 @@ void main() {
   testWithoutContext('Can use devtools with existing devtools URI', () async {
     final DevtoolsServerLauncher launcher = DevtoolsServerLauncher(
       processManager: FakeProcessManager.empty(),
-      fileSystem: fakefs,
       dartExecutable: 'dart',
       logger: BufferLogger.test(),
-      platform: FakePlatform(),
-      persistentToolState: null,
     );
     final ResidentDevtoolsHandler handler = FlutterResidentDevtoolsHandler(
       // Uses real devtools instance which should be a no-op if