[flutter_plugin_tool] Allow re-pathifying dependencies (#5376)
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index 367f258..bfc0caf 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,3 +1,8 @@
+## NEXT
+
+- Allows `make-deps-path-based` to skip packages it has alredy rewritten, so
+ that running multiple times won't fail after the first time.
+
## 0.8.2+1
- Adds a new `readme-check` command.
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 c090603..45a4427 100644
--- a/script/tool/lib/src/make_deps_path_based_command.dart
+++ b/script/tool/lib/src/make_deps_path_based_command.dart
@@ -16,6 +16,8 @@
const int _exitPackageNotFound = 3;
const int _exitCannotUpdatePubspec = 4;
+enum _RewriteOutcome { changed, noChangesNeeded, alreadyChanged }
+
/// Converts all dependencies on target packages to path-based dependencies.
///
/// This is to allow for pre-publish testing of changes that could affect other
@@ -48,6 +50,10 @@
static const String _targetDependenciesWithNonBreakingUpdatesArg =
'target-dependencies-with-non-breaking-updates';
+ // The comment to add to temporary dependency overrides.
+ static const String _dependencyOverrideWarningComment =
+ '# FOR TESTING ONLY. DO NOT MERGE.';
+
@override
final String name = 'make-deps-path-based';
@@ -73,12 +79,19 @@
final String repoRootPath = (await gitDir).path;
for (final File pubspec in await _getAllPubspecs()) {
- if (await _addDependencyOverridesIfNecessary(
- pubspec, localDependencyPackages)) {
- // Print the relative path of the changed pubspec.
- final String displayPath = p.posix.joinAll(path
- .split(path.relative(pubspec.absolute.path, from: repoRootPath)));
- print(' Modified $displayPath');
+ final String displayPath = p.posix.joinAll(
+ path.split(path.relative(pubspec.absolute.path, from: repoRootPath)));
+ final _RewriteOutcome outcome = await _addDependencyOverridesIfNecessary(
+ pubspec, localDependencyPackages);
+ switch (outcome) {
+ case _RewriteOutcome.changed:
+ print(' Modified $displayPath');
+ break;
+ case _RewriteOutcome.alreadyChanged:
+ print(' Skipped $displayPath - Already rewritten');
+ break;
+ case _RewriteOutcome.noChangesNeeded:
+ break;
}
}
}
@@ -125,16 +138,18 @@
/// If [pubspecFile] has any dependencies on packages in [localDependencies],
/// adds dependency_overrides entries to redirect them to the local version
/// using path-based dependencies.
- ///
- /// Returns true if any changes were made.
- Future<bool> _addDependencyOverridesIfNecessary(File pubspecFile,
+ Future<_RewriteOutcome> _addDependencyOverridesIfNecessary(File pubspecFile,
Map<String, RepositoryPackage> localDependencies) async {
final String pubspecContents = pubspecFile.readAsStringSync();
final Pubspec pubspec = Pubspec.parse(pubspecContents);
- // Fail if there are any dependency overrides already. If support for that
- // is needed at some point, it can be added, but currently it's not and
- // relying on that makes the logic here much simpler.
+ // Fail if there are any dependency overrides already, other than ones
+ // created by this script. If support for that is needed at some point, it
+ // can be added, but currently it's not and relying on that makes the logic
+ // here much simpler.
if (pubspec.dependencyOverrides.isNotEmpty) {
+ if (pubspecContents.contains(_dependencyOverrideWarningComment)) {
+ return _RewriteOutcome.alreadyChanged;
+ }
printError(
'Plugins with dependency overrides are not currently supported.');
throw ToolExit(_exitCannotUpdatePubspec);
@@ -158,7 +173,7 @@
String newPubspecContents = pubspecContents +
'''
-# FOR TESTING ONLY. DO NOT MERGE.
+$_dependencyOverrideWarningComment
dependency_overrides:
''';
for (final String packageName in packagesToOverride) {
@@ -175,9 +190,9 @@
''';
}
pubspecFile.writeAsStringSync(newPubspecContents);
- return true;
+ return _RewriteOutcome.changed;
}
- return false;
+ return _RewriteOutcome.noChangesNeeded;
}
/// Returns all pubspecs anywhere under the packages directory.
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 2021f24..da241c3 100644
--- a/script/tool/test/make_deps_path_based_command_test.dart
+++ b/script/tool/test/make_deps_path_based_command_test.dart
@@ -144,6 +144,52 @@
]));
});
+ // This test case ensures that running CI using this command on an interim
+ // PR that itself used this command won't fail on the rewrite step.
+ test('running a second time no-ops without failing', () async {
+ final RepositoryPackage simplePackage = RepositoryPackage(
+ createFakePackage('foo', packagesDir, isFlutter: true));
+ final Directory pluginGroup = packagesDir.childDirectory('bar');
+
+ RepositoryPackage(createFakePackage('bar_platform_interface', pluginGroup,
+ isFlutter: true));
+ final RepositoryPackage pluginImplementation =
+ RepositoryPackage(createFakePlugin('bar_android', pluginGroup));
+ final RepositoryPackage pluginAppFacing =
+ RepositoryPackage(createFakePlugin('bar', pluginGroup));
+
+ _addDependencies(simplePackage, <String>[
+ 'bar',
+ 'bar_android',
+ 'bar_platform_interface',
+ ]);
+ _addDependencies(pluginAppFacing, <String>[
+ 'bar_platform_interface',
+ 'bar_android',
+ ]);
+ _addDependencies(pluginImplementation, <String>[
+ 'bar_platform_interface',
+ ]);
+
+ await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies=bar,bar_platform_interface'
+ ]);
+ final List<String> output = await runCapturingPrint(runner, <String>[
+ 'make-deps-path-based',
+ '--target-dependencies=bar,bar_platform_interface'
+ ]);
+
+ expect(
+ output,
+ containsAll(<String>[
+ 'Rewriting references to: bar, bar_platform_interface...',
+ ' Skipped packages/bar/bar/pubspec.yaml - Already rewritten',
+ ' Skipped packages/bar/bar_android/pubspec.yaml - Already rewritten',
+ ' Skipped packages/foo/pubspec.yaml - Already rewritten',
+ ]));
+ });
+
group('target-dependencies-with-non-breaking-updates', () {
test('no-ops for no published changes', () async {
final Directory package = createFakePackage('foo', packagesDir);
