From b194db9df706907ee2f2db285476f878175c53ed Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 17 Jun 2024 14:48:43 -0700 Subject: [PATCH 01/11] Lookup the package config location for Dart and Flutter test targets --- .../lib/src/service/service_manager.dart | 47 ++++++++++++------- packages/devtools_shared/lib/src/common.dart | 15 ++++++ .../lib/src/extensions/extension_manager.dart | 4 +- .../lib/src/utils/file_utils.dart | 4 +- .../test/helpers/extension_test_manager.dart | 6 +-- 5 files changed, 51 insertions(+), 25 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index daeebf2b589..b138c396d1c 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'dart:core'; -import 'package:collection/collection.dart'; import 'package:dds_service_extensions/dds_service_extensions.dart'; import 'package:devtools_shared/devtools_shared.dart'; import 'package:flutter/foundation.dart'; @@ -16,6 +15,7 @@ import 'package:vm_service/vm_service.dart' hide Error; import '../utils/utils.dart'; import 'connected_app.dart'; import 'dtd_manager.dart'; +import 'eval_on_dart_library.dart' hide SentinelException; import 'flutter_version.dart'; import 'isolate_manager.dart'; import 'isolate_state.dart'; @@ -521,24 +521,35 @@ class ServiceManager { // VM service connection spawned from `dart test` or `flutter test`). if (packageRootUriString?.endsWith('.dart') ?? false) { final rootLibrary = await _mainIsolateRootLibrary(); - final testTargetFileUriString = - (rootLibrary?.dependencies ?? []) - .firstWhereOrNull((dep) => dep.prefix == 'test') - ?.target - ?.uri; - if (testTargetFileUriString != null) { - _log.fine( - '[connectedAppPackageRoot] detected test library from root library ' - 'imports: $testTargetFileUriString', - ); - packageRootUriString = await packageRootFromFileUriString( - testTargetFileUriString, - dtd: dtdManager.connection.value, - ); - _log.fine( - '[connectedAppPackageRoot] package root for test target: ' - '$packageRootUriString', + if (rootLibrary != null) { + final eval = EvalOnDartLibrary( + rootLibrary.uri!, + this.service! as VmService, + serviceManager: this, ); + final evalDisposable = Disposable(); + final packageConfig = (await eval.evalInstance( + 'packageConfigLocationn', + isAlive: evalDisposable, + )) + .valueAsString; + evalDisposable.dispose(); + + if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { + _log.fine( + '[connectedAppPackageRoot] detected test package config from root ' + 'library eval: $packageConfig.', + ); + packageRootUriString = packageConfig!.substring( + 0, + // Minus 1 to remove the trailing slash. + packageConfig.length - packageConfigIdentifier.length - 1, + ); + _log.fine( + '[connectedAppPackageRoot] package root for test target: ' + '$packageRootUriString', + ); + } } } diff --git a/packages/devtools_shared/lib/src/common.dart b/packages/devtools_shared/lib/src/common.dart index 88ea352eedd..0023465cd9c 100644 --- a/packages/devtools_shared/lib/src/common.dart +++ b/packages/devtools_shared/lib/src/common.dart @@ -2,5 +2,20 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:path/path.dart' as path; + /// Describes an instance of the Dart Tooling Daemon. typedef DTDConnectionInfo = ({String? uri, String? secret}); + +/// The name of the Directory where a Dart application's package config file is +/// stored. +const dartToolDirectoryName = '.dart_tool'; + +/// The name of the package config file for a Dart application. +const packageConfigFileName = 'package_config.json'; + +/// The path identifier for the package config URI for a Dart application. +/// +/// The package config file lives at '.dart_tool/package_config.json'. +final packageConfigIdentifier = + path.join(dartToolDirectoryName, packageConfigFileName); diff --git a/packages/devtools_shared/lib/src/extensions/extension_manager.dart b/packages/devtools_shared/lib/src/extensions/extension_manager.dart index e43f591997d..bb2b688300d 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_manager.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_manager.dart @@ -146,8 +146,8 @@ class ExtensionsManager { // may be an incorrect assumption for monorepos. final packageConfigPath = path.posix.join( rootFileUriString, - '.dart_tool', - 'package_config.json', + dartToolDirectoryName, + packageConfigFileName, ); extensions = await findExtensions( 'devtools', diff --git a/packages/devtools_shared/lib/src/utils/file_utils.dart b/packages/devtools_shared/lib/src/utils/file_utils.dart index b29aef26003..e1560c834d7 100644 --- a/packages/devtools_shared/lib/src/utils/file_utils.dart +++ b/packages/devtools_shared/lib/src/utils/file_utils.dart @@ -5,6 +5,8 @@ import 'package:dtd/dtd.dart'; import 'package:meta/meta.dart'; +import '../common.dart'; + const _fileUriPrefix = 'file://'; /// Attempts to detect the package root of [fileUriString], which is expected to @@ -52,7 +54,7 @@ Future packageRootFromFileUriString( try { final directoryContents = await dtd.listDirectoryContents(uri); final containsDartToolDirectory = (directoryContents.uris ?? const []) - .any((uri) => uri.path.endsWith('.dart_tool/')); + .any((uri) => uri.path.endsWith('$dartToolDirectoryName/')); if (containsDartToolDirectory) { final uriAsString = uri.toString(); return _assertUriFormatAndReturn( diff --git a/packages/devtools_shared/test/helpers/extension_test_manager.dart b/packages/devtools_shared/test/helpers/extension_test_manager.dart index 31271d5a03c..eb1622e65cc 100644 --- a/packages/devtools_shared/test/helpers/extension_test_manager.dart +++ b/packages/devtools_shared/test/helpers/extension_test_manager.dart @@ -4,6 +4,7 @@ import 'dart:io'; +import 'package:devtools_shared/devtools_shared.dart'; import 'package:path/path.dart' as p; import 'package:test/test.dart'; @@ -109,10 +110,7 @@ class ExtensionTestManager { ); } - final packageConfigFile = File( - p.join(packageRoot.path, '.dart_tool', 'package_config.json'), - ); - expect(packageConfigFile.existsSync(), isTrue); + expect(File(packageConfigIdentifier).existsSync(), isTrue); } } From 8e2621fe8b4123259a81a3d7e13a859c2d974d5f Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 17 Jun 2024 14:57:53 -0700 Subject: [PATCH 02/11] add back fallback --- .../lib/src/service/service_manager.dart | 85 +++++++++++++------ 1 file changed, 57 insertions(+), 28 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index b138c396d1c..ebfdb08055e 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:core'; +import 'package:collection/collection.dart'; import 'package:dds_service_extensions/dds_service_extensions.dart'; import 'package:devtools_shared/devtools_shared.dart'; import 'package:flutter/foundation.dart'; @@ -522,34 +523,12 @@ class ServiceManager { if (packageRootUriString?.endsWith('.dart') ?? false) { final rootLibrary = await _mainIsolateRootLibrary(); if (rootLibrary != null) { - final eval = EvalOnDartLibrary( - rootLibrary.uri!, - this.service! as VmService, - serviceManager: this, - ); - final evalDisposable = Disposable(); - final packageConfig = (await eval.evalInstance( - 'packageConfigLocationn', - isAlive: evalDisposable, - )) - .valueAsString; - evalDisposable.dispose(); - - if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { - _log.fine( - '[connectedAppPackageRoot] detected test package config from root ' - 'library eval: $packageConfig.', - ); - packageRootUriString = packageConfig!.substring( - 0, - // Minus 1 to remove the trailing slash. - packageConfig.length - packageConfigIdentifier.length - 1, - ); - _log.fine( - '[connectedAppPackageRoot] package root for test target: ' - '$packageRootUriString', - ); - } + packageRootUriString = + (await _lookupPackageConfigByEval(rootLibrary)) ?? + // TODO(kenz): remove this fallback once all test bootstrap + // generators are including the `packageConfigLocation` constant + // we can evaluate. + await _lookupTestLibraryByPrefix(rootLibrary, dtdManager); } } @@ -558,6 +537,56 @@ class ServiceManager { : Uri.parse(packageRootUriString); } + Future _lookupPackageConfigByEval(Library rootLibrary) async { + final eval = EvalOnDartLibrary( + rootLibrary.uri!, + this.service! as VmService, + serviceManager: this, + ); + final evalDisposable = Disposable(); + final packageConfig = (await eval.evalInstance( + 'packageConfigLocation', + isAlive: evalDisposable, + )) + .valueAsString; + evalDisposable.dispose(); + + if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { + _log.fine( + '[connectedAppPackageRoot] detected test package config from root ' + 'library eval: $packageConfig.', + ); + return packageConfig!.substring( + 0, + // Minus 1 to remove the trailing slash. + packageConfig.length - packageConfigIdentifier.length - 1, + ); + } + return null; + } + + Future _lookupTestLibraryByPrefix( + Library rootLibrary, + DTDManager dtdManager, + ) async { + final testTargetFileUriString = + (rootLibrary.dependencies ?? []) + .firstWhereOrNull((dep) => dep.prefix == 'test') + ?.target + ?.uri; + if (testTargetFileUriString != null) { + _log.fine( + '[connectedAppPackageRoot] detected test library from root library ' + 'imports: $testTargetFileUriString', + ); + return await packageRootFromFileUriString( + testTargetFileUriString, + dtd: dtdManager.connection.value, + ); + } + return null; + } + Future _mainIsolateRootLibrary() async { final ref = (await isolateManager.waitForMainIsolateState())?.isolateNow?.rootLib; From 5fe69ddf68fd60b90b19a46390119c9099ee8723 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 17 Jun 2024 14:59:07 -0700 Subject: [PATCH 03/11] fix comment --- .../devtools_app_shared/lib/src/service/service_manager.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index ebfdb08055e..ef4f52512e8 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -526,8 +526,8 @@ class ServiceManager { packageRootUriString = (await _lookupPackageConfigByEval(rootLibrary)) ?? // TODO(kenz): remove this fallback once all test bootstrap - // generators are including the `packageConfigLocation` constant - // we can evaluate. + // generators include the `packageConfigLocation` constant we + // can evaluate. await _lookupTestLibraryByPrefix(rootLibrary, dtdManager); } } From 8a3151f011b24560226b0b11f5459a930f057532 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 17 Jun 2024 15:00:07 -0700 Subject: [PATCH 04/11] add back log --- .../devtools_app_shared/lib/src/service/service_manager.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index ef4f52512e8..1c2fb08d765 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -531,7 +531,10 @@ class ServiceManager { await _lookupTestLibraryByPrefix(rootLibrary, dtdManager); } } - + _log.fine( + '[connectedAppPackageRoot] package root for test target: ' + '$packageRootUriString', + ); return packageRootUriString == null ? null : Uri.parse(packageRootUriString); From e67d5e02de85b9b548639100ee9ef4c77e11fef6 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 17 Jun 2024 15:22:59 -0700 Subject: [PATCH 05/11] error handling --- packages/devtools_app_shared/CHANGELOG.md | 8 +++- .../lib/src/service/eval_on_dart_library.dart | 6 ++- .../lib/src/service/service_manager.dart | 43 +++++++++++-------- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/packages/devtools_app_shared/CHANGELOG.md b/packages/devtools_app_shared/CHANGELOG.md index fa87159ca70..72f6431e318 100644 --- a/packages/devtools_app_shared/CHANGELOG.md +++ b/packages/devtools_app_shared/CHANGELOG.md @@ -1,4 +1,10 @@ -## 0.2.0 +## 0.2.2-wip +* Lookup the connected app package root from an expression evaluation when +the connected app is a Dart or Flutter test. +* Added a field `logExceptions` to `EvalOnDartLibrary` that defaults to true but +can be disabled to prevent exceptions from being logged to console. + +## 0.2.1 * Add `navigateToCode` utility method for jumping to code in IDEs. * Add `FlutterEvent` and `DeveloperServiceEvent` constants. * Add `connectedAppPackageRoot`, `rootPackageDirectoryForMainIsolate`, and diff --git a/packages/devtools_app_shared/lib/src/service/eval_on_dart_library.dart b/packages/devtools_app_shared/lib/src/service/eval_on_dart_library.dart index d6be243a7e3..66d66f91981 100644 --- a/packages/devtools_app_shared/lib/src/service/eval_on_dart_library.dart +++ b/packages/devtools_app_shared/lib/src/service/eval_on_dart_library.dart @@ -40,6 +40,7 @@ class EvalOnDartLibrary extends DisposableController ValueListenable? isolate, this.disableBreakpoints = true, this.oneRequestAtATime = false, + this.logExceptions = true, }) : _clientId = Random().nextInt(1000000000) { _libraryRef = Completer(); @@ -60,6 +61,9 @@ class EvalOnDartLibrary extends DisposableController /// Whether to disable breakpoints triggered while evaluating expressions. final bool disableBreakpoints; + /// Whether to log exceptions to stdout on failed evaluations. + final bool logExceptions; + /// An ID unique to this instance, so that [asyncEval] keeps working even if /// the devtool is opened on multiple tabs at the same time. final int _clientId; @@ -261,7 +265,7 @@ class EvalOnDartLibrary extends DisposableController } void _handleError(Object e, StackTrace stack) { - if (_disposed) return; + if (_disposed || !logExceptions) return; if (e is RPCError) { _log.shout('RPCError: $e', e, stack); diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index 1c2fb08d765..5339fd01b98 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -545,25 +545,34 @@ class ServiceManager { rootLibrary.uri!, this.service! as VmService, serviceManager: this, + // Swallow exceptions since this evaluation may be called on an older + // version of package:test where we do not expect the evaluation to + // succeed. + logExceptions: false, ); final evalDisposable = Disposable(); - final packageConfig = (await eval.evalInstance( - 'packageConfigLocation', - isAlive: evalDisposable, - )) - .valueAsString; - evalDisposable.dispose(); - - if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { - _log.fine( - '[connectedAppPackageRoot] detected test package config from root ' - 'library eval: $packageConfig.', - ); - return packageConfig!.substring( - 0, - // Minus 1 to remove the trailing slash. - packageConfig.length - packageConfigIdentifier.length - 1, - ); + try { + final packageConfig = (await eval.evalInstance( + 'packageConfigLocationn', + isAlive: evalDisposable, + )) + .valueAsString; + + if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { + _log.fine( + '[connectedAppPackageRoot] detected test package config from root ' + 'library eval: $packageConfig.', + ); + return packageConfig!.substring( + 0, + // Minus 1 to remove the trailing slash. + packageConfig.length - packageConfigIdentifier.length - 1, + ); + } + } catch (_) { + // Fail gracefully if the evaluation fails. + } finally { + evalDisposable.dispose(); } return null; } From f2418a84abc2be9d1a72ebb9169f835eb29709a8 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Tue, 18 Jun 2024 09:44:31 -0700 Subject: [PATCH 06/11] add and clean up some todos --- .../lib/src/shared/server/_extensions_api.dart | 3 +++ .../lib/src/extensions/extension_manager.dart | 17 +++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/server/_extensions_api.dart b/packages/devtools_app/lib/src/shared/server/_extensions_api.dart index 13e67aa1e4f..e9e1a661000 100644 --- a/packages/devtools_app/lib/src/shared/server/_extensions_api.dart +++ b/packages/devtools_app/lib/src/shared/server/_extensions_api.dart @@ -8,6 +8,9 @@ part of 'server.dart'; /// serve their assets on the server, and return the list of available /// extensions here. Future> refreshAvailableExtensions( + // TODO(https://github.com/flutter/devtools/issues/7944): pass the URI to the + // package config file instead of passing the app root and rebulding the URI + // to the package config file. Uri? appRoot, ) async { _log.fine('refreshAvailableExtensions for app root: ${appRoot.toString()}'); diff --git a/packages/devtools_shared/lib/src/extensions/extension_manager.dart b/packages/devtools_shared/lib/src/extensions/extension_manager.dart index bb2b688300d..7a03a252965 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_manager.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_manager.dart @@ -103,13 +103,13 @@ class ExtensionsManager { if (root.toString() == rootFileUriString) continue; await _addExtensionsForRoot( - // TODO(https://github.com/dart-lang/pub/issues/4218): this logic + // TODO(https://github.com/flutter/devtools/issues/7944): this logic // assumes that the .dart_tool folder containing the // package_config.json file is in the same directory as the // pubspec.yaml file (since `dartToolingDaemon.getProjectRoots` // returns all directories within the IDE workspace roots that have // a pubspec.yaml file). This may be an incorrect assumption for - // monorepos. + // pub workspaces. root.toString(), logs: logs, parsingErrors: parsingErrors, @@ -141,9 +141,9 @@ class ExtensionsManager { _assertUriFormat(rootFileUriString); final List extensions; try { - // TODO(https://github.com/dart-lang/pub/issues/4218): this assumes that - // the .dart_tool/package_config.json file is in the package root, which - // may be an incorrect assumption for monorepos. + // TODO(https://github.com/flutter/devtools/issues/7944): this assumes + // that the .dart_tool/package_config.json file is in the package root, + // which may be an incorrect assumption for pub workspaces. final packageConfigPath = path.posix.join( rootFileUriString, dartToolDirectoryName, @@ -186,9 +186,10 @@ class ExtensionsManager { final extensionConfig = DevToolsExtensionConfig.parse({ ...config, DevToolsExtensionConfig.extensionAssetsPathKey: location, - // TODO(kenz): for monorepos, we may want to store the - // devtools_options.yaml at the same location as the workspace's - // .dart_tool/package_config.json file. + // TODO(https://github.com/flutter/devtools/issues/7944): for pub + // workspaces, we may want to store the devtools_options.yaml at the + // same location as the workspace's .dart_tool/package_config.json + // file. DevToolsExtensionConfig.devtoolsOptionsUriKey: path.join(rootFileUriString, devtoolsOptionsFileName), DevToolsExtensionConfig.isPubliclyHostedKey: isPubliclyHosted, From a3530bb9dad7815f3c6c3d6f0eab9eb831e7b194 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Tue, 25 Jun 2024 09:07:27 -0700 Subject: [PATCH 07/11] fix lint --- packages/devtools_app_shared/lib/src/utils/web_utils.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/utils/web_utils.dart b/packages/devtools_app_shared/lib/src/utils/web_utils.dart index 515de5fc331..c6239db665a 100644 --- a/packages/devtools_app_shared/lib/src/utils/web_utils.dart +++ b/packages/devtools_app_shared/lib/src/utils/web_utils.dart @@ -7,10 +7,7 @@ import 'dart:js_interop'; import 'package:web/web.dart'; extension MessageExtension on Event { - bool get isMessageEvent => - // TODO(srujzs): This is necessary in order to support package:web 0.4.0. - // This was not needed with 0.3.0, hence the lint. - (this as JSObject).instanceOfString('MessageEvent'); + bool get isMessageEvent => instanceOfString('MessageEvent'); } extension NodeListExtension on NodeList { From 0105a2927cc783cf7511b6ae1a16ef1833b8de42 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Tue, 25 Jun 2024 09:40:10 -0700 Subject: [PATCH 08/11] fix eval string --- .../devtools_app_shared/lib/src/service/service_manager.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index 5339fd01b98..eb31d2246ee 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -553,7 +553,7 @@ class ServiceManager { final evalDisposable = Disposable(); try { final packageConfig = (await eval.evalInstance( - 'packageConfigLocationn', + 'packageConfigLocation', isAlive: evalDisposable, )) .valueAsString; From 92af18b0ed48fc0a517f72f755a803c696746c6d Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Tue, 25 Jun 2024 09:46:00 -0700 Subject: [PATCH 09/11] renames and todos --- .../lib/src/service/service_manager.dart | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index eb31d2246ee..a868617beae 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -523,12 +523,14 @@ class ServiceManager { if (packageRootUriString?.endsWith('.dart') ?? false) { final rootLibrary = await _mainIsolateRootLibrary(); if (rootLibrary != null) { - packageRootUriString = - (await _lookupPackageConfigByEval(rootLibrary)) ?? - // TODO(kenz): remove this fallback once all test bootstrap - // generators include the `packageConfigLocation` constant we - // can evaluate. - await _lookupTestLibraryByPrefix(rootLibrary, dtdManager); + packageRootUriString = (await _lookupPackageRootByEval(rootLibrary)) ?? + // TODO(kenz): remove this fallback once all test bootstrap + // generators include the `packageConfigLocation` constant we + // can evaluate. + await _lookupPackageRootByImportPrefix( + rootLibrary, + dtdManager, + ); } } _log.fine( @@ -540,7 +542,7 @@ class ServiceManager { : Uri.parse(packageRootUriString); } - Future _lookupPackageConfigByEval(Library rootLibrary) async { + Future _lookupPackageRootByEval(Library rootLibrary) async { final eval = EvalOnDartLibrary( rootLibrary.uri!, this.service! as VmService, @@ -563,6 +565,8 @@ class ServiceManager { '[connectedAppPackageRoot] detected test package config from root ' 'library eval: $packageConfig.', ); + // TODO(https://github.com/flutter/devtools/issues/7944): return the + // unmodified package config location. return packageConfig!.substring( 0, // Minus 1 to remove the trailing slash. @@ -577,7 +581,7 @@ class ServiceManager { return null; } - Future _lookupTestLibraryByPrefix( + Future _lookupPackageRootByImportPrefix( Library rootLibrary, DTDManager dtdManager, ) async { @@ -591,6 +595,8 @@ class ServiceManager { '[connectedAppPackageRoot] detected test library from root library ' 'imports: $testTargetFileUriString', ); + // TODO(https://github.com/flutter/devtools/issues/7944): return the + // unmodified package config location. return await packageRootFromFileUriString( testTargetFileUriString, dtd: dtdManager.connection.value, From 7b2b3aa8ff2e089bee2f9475b3618220eaf16036 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Tue, 25 Jun 2024 09:52:30 -0700 Subject: [PATCH 10/11] remove the common strings --- .../lib/src/service/service_manager.dart | 7 +++++-- packages/devtools_app_shared/pubspec.yaml | 1 + packages/devtools_shared/lib/src/common.dart | 15 --------------- .../lib/src/extensions/extension_manager.dart | 4 ++-- .../devtools_shared/lib/src/utils/file_utils.dart | 4 +--- .../test/helpers/extension_test_manager.dart | 6 ++++-- 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index a868617beae..3add8e09855 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -11,6 +11,7 @@ import 'package:devtools_shared/devtools_shared.dart'; import 'package:flutter/foundation.dart'; import 'package:logging/logging.dart'; import 'package:meta/meta.dart'; +import 'package:path/path.dart' as path; import 'package:vm_service/vm_service.dart' hide Error; import '../utils/utils.dart'; @@ -560,13 +561,15 @@ class ServiceManager { )) .valueAsString; + // TODO(https://github.com/flutter/devtools/issues/7944): return the + // unmodified package config location. + final packageConfigIdentifier = + path.join('.dart_tool', 'package_config.json'); if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { _log.fine( '[connectedAppPackageRoot] detected test package config from root ' 'library eval: $packageConfig.', ); - // TODO(https://github.com/flutter/devtools/issues/7944): return the - // unmodified package config location. return packageConfig!.substring( 0, // Minus 1 to remove the trailing slash. diff --git a/packages/devtools_app_shared/pubspec.yaml b/packages/devtools_app_shared/pubspec.yaml index 9ad4b1966b1..fd5d121bc03 100644 --- a/packages/devtools_app_shared/pubspec.yaml +++ b/packages/devtools_app_shared/pubspec.yaml @@ -16,6 +16,7 @@ dependencies: sdk: flutter logging: ^1.1.1 meta: ^1.9.1 + path: ^1.8.0 pointer_interceptor: ^0.9.3+3 url_launcher: ^6.1.0 vm_service: ^14.2.1 diff --git a/packages/devtools_shared/lib/src/common.dart b/packages/devtools_shared/lib/src/common.dart index 0023465cd9c..88ea352eedd 100644 --- a/packages/devtools_shared/lib/src/common.dart +++ b/packages/devtools_shared/lib/src/common.dart @@ -2,20 +2,5 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:path/path.dart' as path; - /// Describes an instance of the Dart Tooling Daemon. typedef DTDConnectionInfo = ({String? uri, String? secret}); - -/// The name of the Directory where a Dart application's package config file is -/// stored. -const dartToolDirectoryName = '.dart_tool'; - -/// The name of the package config file for a Dart application. -const packageConfigFileName = 'package_config.json'; - -/// The path identifier for the package config URI for a Dart application. -/// -/// The package config file lives at '.dart_tool/package_config.json'. -final packageConfigIdentifier = - path.join(dartToolDirectoryName, packageConfigFileName); diff --git a/packages/devtools_shared/lib/src/extensions/extension_manager.dart b/packages/devtools_shared/lib/src/extensions/extension_manager.dart index 7a03a252965..f38f496e36e 100644 --- a/packages/devtools_shared/lib/src/extensions/extension_manager.dart +++ b/packages/devtools_shared/lib/src/extensions/extension_manager.dart @@ -146,8 +146,8 @@ class ExtensionsManager { // which may be an incorrect assumption for pub workspaces. final packageConfigPath = path.posix.join( rootFileUriString, - dartToolDirectoryName, - packageConfigFileName, + '.dart_tool', + 'package_config.json', ); extensions = await findExtensions( 'devtools', diff --git a/packages/devtools_shared/lib/src/utils/file_utils.dart b/packages/devtools_shared/lib/src/utils/file_utils.dart index e1560c834d7..b29aef26003 100644 --- a/packages/devtools_shared/lib/src/utils/file_utils.dart +++ b/packages/devtools_shared/lib/src/utils/file_utils.dart @@ -5,8 +5,6 @@ import 'package:dtd/dtd.dart'; import 'package:meta/meta.dart'; -import '../common.dart'; - const _fileUriPrefix = 'file://'; /// Attempts to detect the package root of [fileUriString], which is expected to @@ -54,7 +52,7 @@ Future packageRootFromFileUriString( try { final directoryContents = await dtd.listDirectoryContents(uri); final containsDartToolDirectory = (directoryContents.uris ?? const []) - .any((uri) => uri.path.endsWith('$dartToolDirectoryName/')); + .any((uri) => uri.path.endsWith('.dart_tool/')); if (containsDartToolDirectory) { final uriAsString = uri.toString(); return _assertUriFormatAndReturn( diff --git a/packages/devtools_shared/test/helpers/extension_test_manager.dart b/packages/devtools_shared/test/helpers/extension_test_manager.dart index eb1622e65cc..31271d5a03c 100644 --- a/packages/devtools_shared/test/helpers/extension_test_manager.dart +++ b/packages/devtools_shared/test/helpers/extension_test_manager.dart @@ -4,7 +4,6 @@ import 'dart:io'; -import 'package:devtools_shared/devtools_shared.dart'; import 'package:path/path.dart' as p; import 'package:test/test.dart'; @@ -110,7 +109,10 @@ class ExtensionTestManager { ); } - expect(File(packageConfigIdentifier).existsSync(), isTrue); + final packageConfigFile = File( + p.join(packageRoot.path, '.dart_tool', 'package_config.json'), + ); + expect(packageConfigFile.existsSync(), isTrue); } } From 369457609da0421bb592f66a4dfbc1c7fec48c47 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Tue, 25 Jun 2024 09:55:06 -0700 Subject: [PATCH 11/11] version number and comment --- .../devtools_app_shared/lib/src/service/service_manager.dart | 3 ++- packages/devtools_app_shared/pubspec.yaml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index 3add8e09855..99fb34a6599 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -562,7 +562,8 @@ class ServiceManager { .valueAsString; // TODO(https://github.com/flutter/devtools/issues/7944): return the - // unmodified package config location. + // unmodified package config location. For this case, be sure to handle + // invalid values like the empty String or 'null'. final packageConfigIdentifier = path.join('.dart_tool', 'package_config.json'); if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) { diff --git a/packages/devtools_app_shared/pubspec.yaml b/packages/devtools_app_shared/pubspec.yaml index fd5d121bc03..16a85c644e0 100644 --- a/packages/devtools_app_shared/pubspec.yaml +++ b/packages/devtools_app_shared/pubspec.yaml @@ -1,6 +1,6 @@ name: devtools_app_shared description: Package of Dart & Flutter structures shared between devtools_app and devtools extensions. -version: 0.2.1 +version: 0.2.2-wip repository: https://github.com/flutter/devtools/tree/master/packages/devtools_app_shared environment: