Find .dart_tool/package_config.json by looking in parent directories (#…

…8347)

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -18,6 +18,9 @@ with WebAssembly. - [#8270](https://github.com/flutter/devtools/pull/8270)
 
     ![Wasm opt-in setting](images/wasm_setting.png "DevTools setting to opt into wasm.")
 
+* Added support for loading extensions in pub workspaces
+  [8347](https://github.com/flutter/devtools/pull/8347).
+
 ## Inspector updates
 
 - Added a setting to the Flutter Inspector controls that allows users to opt-in to the newly redesigned Flutter Inspector. - [#8342](https://github.com/flutter/devtools/pull/8342)

packages/devtools_app_shared/lib/src/service/service_manager.dart

@@ -652,6 +652,8 @@ class ServiceManager<T extends VmService> {
 
   // TODO(kenz): consider caching this value for the duration of the VM service
   // connection.
+  // TODO(https://github.com/dart-lang/sdk/issues/56784) there should be a
+  // vm-service call doing this.
   /// Returns the root package directory for the main isolate.
   ///
   /// If a non-null value is returned, the value will be a file URI String and

packages/devtools_shared/lib/src/extensions/extension_manager.dart

@@ -105,13 +105,6 @@ class ExtensionsManager {
           if (root.toString() == rootFileUriString) continue;
 
           await _addExtensionsForRoot(
-            // 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
-            // pub workspaces.
             root.toString(),
             logs: logs,
             parsingErrors: parsingErrors,
@@ -142,27 +135,16 @@ class ExtensionsManager {
   }) async {
     _assertUriFormat(rootFileUriString);
     final List<Extension> extensions;
-    try {
-      // 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,
-        '.dart_tool',
-        'package_config.json',
-      );
-      extensions = await findExtensions(
-        'devtools',
-        packageConfig: Uri.parse(packageConfigPath),
-      );
-      logs.add(
-        'ExtensionsManager._addExtensionsForRoot find extensions for '
-        'config: $packageConfigPath, result: '
-        '${extensions.map((e) => e.package).toList()}',
-      );
-    } catch (e) {
-      rethrow;
-    }
+    final packageConfigPath = findPackageConfig(Uri.parse(rootFileUriString));
+    extensions = await findExtensions(
+      'devtools',
+      packageConfig: packageConfigPath,
+    );
+    logs.add(
+      'ExtensionsManager._addExtensionsForRoot find extensions for '
+      'config: $packageConfigPath, result: '
+      '${extensions.map((e) => e.package).toList()}',
+    );
 
     for (final extension in extensions) {
       final config = extension.config;
@@ -188,12 +170,14 @@ class ExtensionsManager {
         final extensionConfig = DevToolsExtensionConfig.parse({
           ...config,
           DevToolsExtensionConfig.extensionAssetsPathKey: location,
-          // 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),
+          // The [packageConfigPath] will look like
+          // 'pkg/.dart_tool/package_config.json' so we will store the
+          // devtools_options.yaml file at the 'pkg' root:
+          // 'pkg/devtools_options.yaml'.
+          DevToolsExtensionConfig.devtoolsOptionsUriKey: path.url.join(
+            path.url.dirname(path.url.dirname(packageConfigPath.toString())),
+            devtoolsOptionsFileName,
+          ),
           DevToolsExtensionConfig.isPubliclyHostedKey: isPubliclyHosted,
           DevToolsExtensionConfig.detectedFromStaticContextKey:
               staticContext.toString(),

packages/devtools_shared/pubspec.yaml

@@ -12,7 +12,7 @@ dependencies:
   args: ^2.4.2
   collection: ^1.15.0
   dtd: ^2.2.0
-  extension_discovery: ^2.0.0
+  extension_discovery: ^2.1.0
   meta: ^1.9.1
   path: ^1.8.0
   shelf: ^1.1.0

packages/devtools_shared/test/helpers/extension_test_manager.dart

@@ -47,6 +47,12 @@ class ExtensionTestManager {
   ///     .dart_tool/             # Generated from 'pub get'
   ///       package_config.json   # Generated from 'pub get'
   ///     pubspec.yaml
+  ///   workspace_root/
+  ///     .dart_tool/             # Generated from 'pub get'
+  ///       package_config.json   # Generated from 'pub get'
+  ///     pubspec.yaml            # Defines workspace
+  ///     wokspace_member
+  ///       pubspec.yaml
   /// extensions/
   ///   static_extension_1/
   ///     extension/
@@ -90,7 +96,7 @@ class ExtensionTestManager {
     expect(testDirectoryContents.length, 2);
     final packageRoots =
         Directory.fromUri(packagesRootUri).listSync().whereType<Directory>();
-    expect(packageRoots.length, 3);
+    expect(packageRoots.length, 4);
 
     for (final packageRoot in packageRoots) {
       expect(File(p.join(packageRoot.path, 'pubspec.yaml')).existsSync(), true);
@@ -129,6 +135,12 @@ class ExtensionTestManager {
   ///     .dart_tool/             # Generated from 'pub get'
   ///       package_config.json   # Generated from 'pub get'
   ///     pubspec.yaml
+  ///   workspace_root/
+  ///     .dart_tool/             # Generated from 'pub get'
+  ///       package_config.json   # Generated from 'pub get'
+  ///     pubspec.yaml            # Defines workspace
+  ///     workspace_member/
+  ///       pubspec.yaml
   void _setupPackages({
     required bool includeDependenciesWithExtensions,
     required bool includeBadExtension,
@@ -152,6 +164,45 @@ class ExtensionTestManager {
         includeDependenciesWithExtensions: includeDependenciesWithExtensions,
       ),
     );
+    setupWorkspace(
+      createTestPackageFrom(
+        workspaceMember,
+        includeDependenciesWithExtensions: includeDependenciesWithExtensions,
+      ),
+    );
+  }
+
+  void setupWorkspace(TestPackage package) {
+    File(
+      p.join(
+        testDirectory.path,
+        'packages',
+        'workspace_root',
+        'pubspec.yaml',
+      ),
+    )
+      ..createSync(recursive: true)
+      ..writeAsStringSync('''
+name: _
+environment:
+  sdk: "^3.5.0"
+workspace:
+  - ${package.name}
+''');
+    File(
+      p.join(
+        testDirectory.path,
+        'packages',
+        'workspace_root',
+        package.name,
+        'pubspec.yaml',
+      ),
+    )
+      ..createSync(recursive: true)
+      ..writeAsStringSync('''
+${package.pubspecContent}
+resolution: workspace
+''');
   }
 
   /// extensions/
@@ -239,6 +290,7 @@ TestPackage createTestPackageFrom(
     name: originalPackage.name,
     dependencies:
         includeDependenciesWithExtensions ? originalPackage.dependencies : [],
+    pathToExtensions: originalPackage.pathToExtensions,
   );
 }
 
@@ -258,6 +310,11 @@ final otherRoot2Package = TestPackage(
   name: 'other_root_2',
   dependencies: [newerStaticExtension1Package],
 );
+final workspaceMember = TestPackage(
+  name: 'workspace_member',
+  dependencies: [staticExtension1Package],
+  pathToExtensions: '../../../extensions',
+);
 
 final driftPackage = TestPackageWithExtension(
   name: 'drift',
@@ -355,15 +412,20 @@ environment:
 }
 
 class TestPackage {
-  const TestPackage({required this.name, required this.dependencies});
+  const TestPackage({
+    required this.name,
+    required this.dependencies,
+    this.pathToExtensions = '../../extensions',
+  });
 
   final String name;
   final List<TestPackageWithExtension> dependencies;
+  final String pathToExtensions;
 
   String get pubspecContent => '''
 name: $name
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
 ${_dependenciesAsString()}
 ''';
@@ -378,7 +440,7 @@ ${_dependenciesAsString()}
         sb
           ..writeln() // Add a new line for the path dependency.
           ..writeln(
-            '    path: ../../extensions/${dep.relativePathFromExtensions}',
+            '    path: $pathToExtensions/${dep.relativePathFromExtensions}',
           );
       }
     }

packages/devtools_shared/test/helpers/extension_test_manager_test.dart

@@ -179,7 +179,7 @@ materialIconCodePoint: 58634
         '''
 name: my_app
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
   drift: 2.16.0
   provider: 6.1.2
@@ -198,7 +198,7 @@ dependencies:
         '''
 name: my_app
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
   drift: 2.16.0
   provider: 6.1.2
@@ -219,7 +219,7 @@ dependencies:
         '''
 name: other_root_1
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
   static_extension_1:
     path: ../../extensions/static_extension_1
@@ -238,7 +238,7 @@ dependencies:
         '''
 name: other_root_2
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
   static_extension_1:
     path: ../../extensions/newer/static_extension_1
@@ -257,7 +257,7 @@ dependencies:
         '''
 name: my_app
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
 
 ''',
@@ -271,7 +271,7 @@ dependencies:
         '''
 name: my_app
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
 
 ''',
@@ -285,7 +285,7 @@ dependencies:
         '''
 name: other_root_1
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
 
 ''',
@@ -299,7 +299,7 @@ dependencies:
         '''
 name: other_root_2
 environment:
-  sdk: ">=3.4.0-282.1.beta <4.0.0"
+  sdk: "^3.5.0"
 dependencies:
 
 ''',