From 1285be893865f644f9fc14ab97e2d3b16cfcc510 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Mon, 10 Aug 2026 17:42:25 -0500 Subject: [PATCH 1/5] [ci] Unblock material_ui release --- .../lib/src/make_deps_path_based_command.dart | 79 ++++++++++++------- .../make_deps_path_based_command_test.dart | 34 +++++++- 2 files changed, 81 insertions(+), 32 deletions(-) diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index 701cc7512e9a..cee984cc4723 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -110,43 +110,61 @@ class MakeDepsPathBasedCommand extends PackageCommand { Map _findLocalPackages(Set packageNames) { final targets = {}; - for (final packageName in packageNames) { - final Directory topLevelCandidate = packagesDir.childDirectory(packageName); - // If packages// exists, then either that directory is the - // package, or packages/// exists and is the - // package (in the case of a federated plugin). - if (topLevelCandidate.existsSync()) { - final Directory appFacingCandidate = topLevelCandidate.childDirectory(packageName); - targets[packageName] = RepositoryPackage( - appFacingCandidate.existsSync() ? appFacingCandidate : topLevelCandidate, - ); + final queue = List.from(packageNames); + + while (queue.isNotEmpty) { + final packageName = queue.removeLast(); + if (targets.containsKey(packageName)) { continue; } - // Check for a match in the third-party packages directory. - final Directory thirdPartyCandidate = thirdPartyPackagesDir.childDirectory(packageName); - if (thirdPartyCandidate.existsSync()) { - targets[packageName] = RepositoryPackage(thirdPartyCandidate); + final RepositoryPackage? package = _findLocalPackage(packageName); + if (package == null) { + if (packageNames.contains(packageName)) { + printError('Unable to find package "$packageName"'); + throw ToolExit(_exitPackageNotFound); + } continue; } - // If there is no packages/ directory, then either the - // packages doesn't exist, or it is a sub-package of a federated plugin. - // If it's the latter, it will be a directory whose name is a prefix. - for (final FileSystemEntity entity in packagesDir.listSync()) { - if (entity is Directory && packageName.startsWith(entity.basename)) { - final Directory subPackageCandidate = entity.childDirectory(packageName); - if (subPackageCandidate.existsSync()) { - targets[packageName] = RepositoryPackage(subPackageCandidate); - break; - } + targets[packageName] = package; + + // Expand to include local in-repo dependencies of target packages to prevent + // pub version solving conflicts when co-dependent packages are tested with path dependencies. + final Pubspec pubspec = package.parsePubspec(); + for (final String depName in [ + ...pubspec.dependencies.keys, + ...pubspec.devDependencies.keys, + ]) { + if (!targets.containsKey(depName)) { + queue.add(depName); } } + } + return targets; + } - if (!targets.containsKey(packageName)) { - printError('Unable to find package "$packageName"'); - throw ToolExit(_exitPackageNotFound); + RepositoryPackage? _findLocalPackage(String packageName) { + final Directory topLevelCandidate = packagesDir.childDirectory(packageName); + if (topLevelCandidate.childFile('pubspec.yaml').existsSync()) { + final Directory appFacingCandidate = topLevelCandidate.childDirectory(packageName); + return RepositoryPackage( + appFacingCandidate.childFile('pubspec.yaml').existsSync() + ? appFacingCandidate + : topLevelCandidate, + ); + } + final Directory thirdPartyCandidate = thirdPartyPackagesDir.childDirectory(packageName); + if (thirdPartyCandidate.childFile('pubspec.yaml').existsSync()) { + return RepositoryPackage(thirdPartyCandidate); + } + for (final FileSystemEntity entity in packagesDir.listSync()) { + if (entity is Directory && packageName.startsWith(entity.basename)) { + final Directory subPackageCandidate = entity.childDirectory(packageName); + if (subPackageCandidate.childFile('pubspec.yaml').existsSync()) { + return RepositoryPackage(subPackageCandidate); + } } } - return targets; + return null; } /// If [pubspecFile] has any non-path dependencies on packages in @@ -276,7 +294,10 @@ ${newOverrideLines.join('\n')} example, localDependencies, versions, - additionalPackagesToOverride: packagesToOverride, + additionalPackagesToOverride: { + ...packagesToOverride, + package.directory.basename, + }, ); } diff --git a/script/tool/test/make_deps_path_based_command_test.dart b/script/tool/test/make_deps_path_based_command_test.dart index d1e99d17a247..c8234aa8cdbd 100644 --- a/script/tool/test/make_deps_path_based_command_test.dart +++ b/script/tool/test/make_deps_path_based_command_test.dart @@ -170,19 +170,21 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} expect(output, isNot(contains(' Modified packages/bar/bar_platform_interface/pubspec.yaml'))); final Map simplePackageOverrides = getDependencyOverrides(simplePackage); - expect(simplePackageOverrides.length, 2); + expect(simplePackageOverrides.length, 3); expect(simplePackageOverrides['bar'], '../../packages/bar/bar'); expect( simplePackageOverrides['bar_platform_interface'], '../../packages/bar/bar_platform_interface', ); + expect(simplePackageOverrides['bar_android'], '../../packages/bar/bar_android'); final Map appFacingPackageOverrides = getDependencyOverrides(pluginAppFacing); - expect(appFacingPackageOverrides.length, 1); + expect(appFacingPackageOverrides.length, 2); expect( appFacingPackageOverrides['bar_platform_interface'], '../../../packages/bar/bar_platform_interface', ); + expect(appFacingPackageOverrides['bar_android'], '../../../packages/bar/bar_android'); }); test('rewrites "dev_dependencies" references', () async { @@ -226,8 +228,12 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} final Map exampleOverrides = getDependencyOverrides( pluginAppFacing.getExamples().first, ); - expect(exampleOverrides.length, 1); + expect(exampleOverrides.length, 2); expect(exampleOverrides['bar_android'], '../../../../packages/bar/bar_android'); + expect( + exampleOverrides['bar_platform_interface'], + '../../../../packages/bar/bar_platform_interface', + ); }); test('example overrides include both local and main-package dependencies', () async { @@ -264,6 +270,28 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} expect(exampleOverrides.length, 0); }); + test( + 'overrides co-dependent packages when target dependencies depend on local packages', + () async { + final RepositoryPackage materialUi = createFakePackage('material_ui', packagesDir); + final RepositoryPackage cupertinoUi = createFakePackage('cupertino_ui', packagesDir); + + addDependencies(materialUi, ['cupertino_ui']); + addDevDependenciesSection(cupertinoUi, ['material_ui']); + + await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies=material_ui', + ]); + + final Map materialOverrides = getDependencyOverrides(materialUi); + expect(materialOverrides['cupertino_ui'], '../../packages/cupertino_ui'); + + final Map cupertinoOverrides = getDependencyOverrides(cupertinoUi); + expect(cupertinoOverrides['material_ui'], '../../packages/material_ui'); + }, + ); + test( 'alphabetizes overrides from different sections to avoid lint warnings in analysis', () async { From b714dbba6a552a3ee8a5ae848f68601ae0160c2d Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Mon, 10 Aug 2026 18:00:08 -0500 Subject: [PATCH 2/5] G feedback --- script/tool/lib/src/make_deps_path_based_command.dart | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index cee984cc4723..5d5cdc02a930 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -157,7 +157,8 @@ class MakeDepsPathBasedCommand extends PackageCommand { return RepositoryPackage(thirdPartyCandidate); } for (final FileSystemEntity entity in packagesDir.listSync()) { - if (entity is Directory && packageName.startsWith(entity.basename)) { + if (entity is Directory && + (packageName == entity.basename || packageName.startsWith('${entity.basename}_'))) { final Directory subPackageCandidate = entity.childDirectory(packageName); if (subPackageCandidate.childFile('pubspec.yaml').existsSync()) { return RepositoryPackage(subPackageCandidate); @@ -294,10 +295,7 @@ ${newOverrideLines.join('\n')} example, localDependencies, versions, - additionalPackagesToOverride: { - ...packagesToOverride, - package.directory.basename, - }, + additionalPackagesToOverride: {...packagesToOverride, package.parsePubspec().name}, ); } From 83e87a64e5483076b00184b72b7d5fb3b356d10b Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Tue, 11 Aug 2026 09:12:05 -0500 Subject: [PATCH 3/5] Fix analyzer --- script/tool/lib/src/make_deps_path_based_command.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index 5d5cdc02a930..d972126898a7 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -113,7 +113,7 @@ class MakeDepsPathBasedCommand extends PackageCommand { final queue = List.from(packageNames); while (queue.isNotEmpty) { - final packageName = queue.removeLast(); + final String packageName = queue.removeLast(); if (targets.containsKey(packageName)) { continue; } @@ -130,7 +130,7 @@ class MakeDepsPathBasedCommand extends PackageCommand { // Expand to include local in-repo dependencies of target packages to prevent // pub version solving conflicts when co-dependent packages are tested with path dependencies. final Pubspec pubspec = package.parsePubspec(); - for (final String depName in [ + for (final depName in [ ...pubspec.dependencies.keys, ...pubspec.devDependencies.keys, ]) { From 866042961dc91a7c07e770ac242076dca3ad2f00 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Tue, 11 Aug 2026 11:32:29 -0500 Subject: [PATCH 4/5] Review feedback, restore safety net --- .../lib/src/make_deps_path_based_command.dart | 85 ++++++++----------- .../make_deps_path_based_command_test.dart | 52 ++++++++---- 2 files changed, 70 insertions(+), 67 deletions(-) diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index d972126898a7..95382c05af7d 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -110,62 +110,43 @@ class MakeDepsPathBasedCommand extends PackageCommand { Map _findLocalPackages(Set packageNames) { final targets = {}; - final queue = List.from(packageNames); - - while (queue.isNotEmpty) { - final String packageName = queue.removeLast(); - if (targets.containsKey(packageName)) { + for (final packageName in packageNames) { + final Directory topLevelCandidate = packagesDir.childDirectory(packageName); + // If packages// exists, then either that directory is the + // package, or packages/// exists and is the + // package (in the case of a federated plugin). + if (topLevelCandidate.existsSync()) { + final Directory appFacingCandidate = topLevelCandidate.childDirectory(packageName); + targets[packageName] = RepositoryPackage( + appFacingCandidate.existsSync() ? appFacingCandidate : topLevelCandidate, + ); continue; } - final RepositoryPackage? package = _findLocalPackage(packageName); - if (package == null) { - if (packageNames.contains(packageName)) { - printError('Unable to find package "$packageName"'); - throw ToolExit(_exitPackageNotFound); - } + // Check for a match in the third-party packages directory. + final Directory thirdPartyCandidate = thirdPartyPackagesDir.childDirectory(packageName); + if (thirdPartyCandidate.existsSync()) { + targets[packageName] = RepositoryPackage(thirdPartyCandidate); continue; } - targets[packageName] = package; - - // Expand to include local in-repo dependencies of target packages to prevent - // pub version solving conflicts when co-dependent packages are tested with path dependencies. - final Pubspec pubspec = package.parsePubspec(); - for (final depName in [ - ...pubspec.dependencies.keys, - ...pubspec.devDependencies.keys, - ]) { - if (!targets.containsKey(depName)) { - queue.add(depName); + // If there is no packages/ directory, then either the + // packages doesn't exist, or it is a sub-package of a federated plugin. + // If it's the latter, it will be a directory whose name is a prefix. + for (final FileSystemEntity entity in packagesDir.listSync()) { + if (entity is Directory && packageName.startsWith(entity.basename)) { + final Directory subPackageCandidate = entity.childDirectory(packageName); + if (subPackageCandidate.existsSync()) { + targets[packageName] = RepositoryPackage(subPackageCandidate); + break; + } } } - } - return targets; - } - RepositoryPackage? _findLocalPackage(String packageName) { - final Directory topLevelCandidate = packagesDir.childDirectory(packageName); - if (topLevelCandidate.childFile('pubspec.yaml').existsSync()) { - final Directory appFacingCandidate = topLevelCandidate.childDirectory(packageName); - return RepositoryPackage( - appFacingCandidate.childFile('pubspec.yaml').existsSync() - ? appFacingCandidate - : topLevelCandidate, - ); - } - final Directory thirdPartyCandidate = thirdPartyPackagesDir.childDirectory(packageName); - if (thirdPartyCandidate.childFile('pubspec.yaml').existsSync()) { - return RepositoryPackage(thirdPartyCandidate); - } - for (final FileSystemEntity entity in packagesDir.listSync()) { - if (entity is Directory && - (packageName == entity.basename || packageName.startsWith('${entity.basename}_'))) { - final Directory subPackageCandidate = entity.childDirectory(packageName); - if (subPackageCandidate.childFile('pubspec.yaml').existsSync()) { - return RepositoryPackage(subPackageCandidate); - } + if (!targets.containsKey(packageName)) { + printError('Unable to find package "$packageName"'); + throw ToolExit(_exitPackageNotFound); } } - return null; + return targets; } /// If [pubspecFile] has any non-path dependencies on packages in @@ -255,7 +236,10 @@ class MakeDepsPathBasedCommand extends PackageCommand { // Find the relative path from the common base to the local package. final List repoRelativePathComponents = path.split( - path.relative(localDependencies[packageName]!.path, from: repoRootPath), + path.relative( + localDependencies[packageName]!.directory.absolute.path, + from: repoRootPath, + ), ); final String pathValue = p.posix.joinAll([ ...relativeBasePathComponents, @@ -291,11 +275,12 @@ ${newOverrideLines.join('\n')} // example app doesn't. Since integration tests are run in the example app, // it needs the overrides in order for tests to pass. for (final RepositoryPackage example in package.getExamples()) { + final String parentPackageName = package.parsePubspec().name; await _addDependencyOverridesIfNecessary( example, - localDependencies, + {...localDependencies, parentPackageName: package}, versions, - additionalPackagesToOverride: {...packagesToOverride, package.parsePubspec().name}, + additionalPackagesToOverride: {...packagesToOverride, parentPackageName}, ); } diff --git a/script/tool/test/make_deps_path_based_command_test.dart b/script/tool/test/make_deps_path_based_command_test.dart index c8234aa8cdbd..c1ca53fa4e98 100644 --- a/script/tool/test/make_deps_path_based_command_test.dart +++ b/script/tool/test/make_deps_path_based_command_test.dart @@ -170,21 +170,19 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} expect(output, isNot(contains(' Modified packages/bar/bar_platform_interface/pubspec.yaml'))); final Map simplePackageOverrides = getDependencyOverrides(simplePackage); - expect(simplePackageOverrides.length, 3); + expect(simplePackageOverrides.length, 2); expect(simplePackageOverrides['bar'], '../../packages/bar/bar'); expect( simplePackageOverrides['bar_platform_interface'], '../../packages/bar/bar_platform_interface', ); - expect(simplePackageOverrides['bar_android'], '../../packages/bar/bar_android'); final Map appFacingPackageOverrides = getDependencyOverrides(pluginAppFacing); - expect(appFacingPackageOverrides.length, 2); + expect(appFacingPackageOverrides.length, 1); expect( appFacingPackageOverrides['bar_platform_interface'], '../../../packages/bar/bar_platform_interface', ); - expect(appFacingPackageOverrides['bar_android'], '../../../packages/bar/bar_android'); }); test('rewrites "dev_dependencies" references', () async { @@ -229,11 +227,8 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} pluginAppFacing.getExamples().first, ); expect(exampleOverrides.length, 2); + expect(exampleOverrides['bar'], '../../../../packages/bar/bar'); expect(exampleOverrides['bar_android'], '../../../../packages/bar/bar_android'); - expect( - exampleOverrides['bar_platform_interface'], - '../../../../packages/bar/bar_platform_interface', - ); }); test('example overrides include both local and main-package dependencies', () async { @@ -254,8 +249,9 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} final Map exampleOverrides = getDependencyOverrides( pluginAppFacing.getExamples().first, ); - expect(exampleOverrides.length, 2); + expect(exampleOverrides.length, 3); expect(exampleOverrides['another_package'], '../../../../packages/another_package'); + expect(exampleOverrides['bar'], '../../../../packages/bar/bar'); expect(exampleOverrides['bar_android'], '../../../../packages/bar/bar_android'); }); @@ -271,12 +267,11 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} }); test( - 'overrides co-dependent packages when target dependencies depend on local packages', + 'example overrides include parent package to resolve dependency overrides cleanly', () async { - final RepositoryPackage materialUi = createFakePackage('material_ui', packagesDir); - final RepositoryPackage cupertinoUi = createFakePackage('cupertino_ui', packagesDir); + createFakePackage('material_ui', packagesDir); + final RepositoryPackage cupertinoUi = createFakePlugin('cupertino_ui', packagesDir); - addDependencies(materialUi, ['cupertino_ui']); addDevDependenciesSection(cupertinoUi, ['material_ui']); await runCapturingPrint(runner, [ @@ -284,11 +279,34 @@ ${overrides.map((String dep) => ' $dep:\n path: $path').join('\n')} '--target-dependencies=material_ui', ]); - final Map materialOverrides = getDependencyOverrides(materialUi); - expect(materialOverrides['cupertino_ui'], '../../packages/cupertino_ui'); + final Map exampleOverrides = getDependencyOverrides( + cupertinoUi.getExamples().first, + ); + expect(exampleOverrides['material_ui'], '../../../packages/material_ui'); + expect(exampleOverrides['cupertino_ui'], '../../../packages/cupertino_ui'); + }, + ); + + test( + 'does not recursively override dependencies of target packages to preserve safety net', + () async { + final RepositoryPackage clientPkg = createFakePackage('pkg_client', packagesDir); + final RepositoryPackage targetPkg = createFakePackage('pkg_target', packagesDir); + createFakePackage('pkg_dependency', packagesDir); + + addDependencies(clientPkg, ['pkg_target', 'pkg_dependency']); + addDependencies(targetPkg, ['pkg_dependency']); + + await runCapturingPrint(runner, [ + 'make-deps-path-based', + '--target-dependencies=pkg_target', + ]); - final Map cupertinoOverrides = getDependencyOverrides(cupertinoUi); - expect(cupertinoOverrides['material_ui'], '../../packages/material_ui'); + final Map clientOverrides = getDependencyOverrides(clientPkg); + expect(clientOverrides['pkg_target'], '../../packages/pkg_target'); + // Dependencies of target packages must not be recursively pathified, + // ensuring clients are tested against published versions to preserve safety. + expect(clientOverrides['pkg_dependency'], isNull); }, ); From 398fc99cf88d2ee63f074550908ca8402f8cc6da Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Tue, 11 Aug 2026 15:00:53 -0500 Subject: [PATCH 5/5] Add comment for future explorers --- script/tool/lib/src/make_deps_path_based_command.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/tool/lib/src/make_deps_path_based_command.dart b/script/tool/lib/src/make_deps_path_based_command.dart index 95382c05af7d..46d46920859a 100644 --- a/script/tool/lib/src/make_deps_path_based_command.dart +++ b/script/tool/lib/src/make_deps_path_based_command.dart @@ -280,6 +280,8 @@ ${newOverrideLines.join('\n')} example, {...localDependencies, parentPackageName: package}, versions, + // Add an override to the parent package in case a transitive dependency has a dependency on it, + // since that (non-path) dependency would conflict with the path-based dependency in the example. additionalPackagesToOverride: {...packagesToOverride, parentPackageName}, ); }