Allow adding and removing dependency overrides (#3716)
diff --git a/lib/src/command/add.dart b/lib/src/command/add.dart index 8245d89..c6e7c84 100644 --- a/lib/src/command/add.dart +++ b/lib/src/command/add.dart
@@ -49,6 +49,8 @@ Add to dev_dependencies by prefixing with "dev:". +Make dependency overrides by prefixing with "override:". + Add packages with specific constraints or other sources by giving a descriptor after a colon. @@ -69,13 +71,17 @@ `$topLevelProgram pub add 'foo:{"sdk":"flutter"}'` * Add a git dependency: `$topLevelProgram pub add 'foo:{"git":"https://github.com/foo/foo"}'` + * Add a dependency override: + `$topLevelProgram pub add 'override:foo:1.0.0` * Add a git dependency with a path and ref specified: `$topLevelProgram pub add \\ - 'foo:{"git":{"url":"../foo.git","ref":"branch","path":"subdir"}}'`'''; + 'foo:{"git":{"url":"../foo.git","ref":"<branch>","path":"<subdir>"}}'`'''; @override String get argumentsDescription => - '[options] [dev:]<package>[:descriptor] [[dev:]<package>[:descriptor] ...]'; + '[options] [<section>:]<package>[:descriptor] ' + '[<section>:]<package2>[:descriptor] ...]'; + @override String get docUrl => 'https://dart.dev/tools/pub/cmd/pub-add'; @@ -287,12 +293,17 @@ final name = package.ref.name; final dependencies = [...original.dependencies.values]; var devDependencies = [...original.devDependencies.values]; + var dependencyOverrides = [...original.dependencyOverrides.values]; + final dependencyNames = dependencies.map((dependency) => dependency.name); final devDependencyNames = devDependencies.map((devDependency) => devDependency.name); final range = package.ref.withConstraint(package.constraint ?? VersionConstraint.any); - if (package.isDev) { + + if (package.isOverride) { + dependencyOverrides.add(range); + } else if (package.isDev) { if (devDependencyNames.contains(name)) { log.message('"$name" is already in "dev_dependencies". ' 'Will try to update the constraint.'); @@ -336,28 +347,66 @@ sdkConstraints: original.sdkConstraints, dependencies: dependencies, devDependencies: devDependencies, - dependencyOverrides: original.dependencyOverrides.values, + dependencyOverrides: dependencyOverrides, ); } + static final _argRegExp = RegExp( + r'^(?:(?<prefix>dev|override):)?' + r'(?<name>[a-zA-Z0-9$.]+)' + r'(?::(?<descriptor>.*))?$', + ); + + static final _lenientArgRegExp = RegExp( + r'^(?:(?<prefix>[^:]*):)?' + r'(?<name>[^:]*)' + r'(?::(?<descriptor>.*))?$', + ); + /// Split [arg] on ':' and interpret it with the flags in [argResult] either as /// an old-style or a new-style descriptor to produce a PackageRef]. _ParseResult _parsePackage(String arg, ArgResults argResults) { var isDev = argResults['dev'] as bool; - if (arg.startsWith('dev:')) { + var isOverride = false; + + final match = _argRegExp.firstMatch(arg); + if (match == null) { + final match2 = _lenientArgRegExp.firstMatch(arg); + if (match2 == null) { + usageException('Could not parse $arg'); + } else { + if (match2.namedGroup('prefix') != null && + match2.namedGroup('descriptor') != null) { + usageException( + 'The only allowed prefixes are "dev:" and "override:"', + ); + } else { + final packageName = match2.namedGroup('descriptor') == null + ? match2.namedGroup('prefix') + : match2.namedGroup('name'); + usageException('Not a valid package name: "$packageName"'); + } + } + } else if (match.namedGroup('prefix') == 'dev') { if (argResults.isDev) { usageException("Cannot combine 'dev:' with --dev"); } isDev = true; - arg = arg.substring('dev:'.length); + } else if (match.namedGroup('prefix') == 'override') { + if (argResults.isDev) { + usageException("Cannot combine 'override:' with --dev"); + } + isOverride = true; } - final nextColon = arg.indexOf(':'); - final packageName = nextColon == -1 ? arg : arg.substring(0, nextColon); + final packageName = match.namedGroup('name')!; if (!packageNameRegExp.hasMatch(packageName)) { usageException('Not a valid package name: "$packageName"'); } - final descriptor = nextColon == -1 ? null : arg.substring(nextColon + 1); + final descriptor = match.namedGroup('descriptor'); + if (isOverride && descriptor == null) { + usageException('A dependency override needs an explicit descriptor.'); + } final _PartialParseResult partial; if (argResults.hasOldStyleOptions) { partial = _parseDescriptorOldStyleArgs( @@ -369,7 +418,12 @@ partial = _parseDescriptorNewStyle(packageName, descriptor); } - return _ParseResult(partial.ref, partial.constraint, isDev: isDev); + return _ParseResult( + partial.ref, + partial.constraint, + isDev: isDev, + isOverride: isOverride, + ); } /// Parse [descriptor] to return the corresponding [_ParseResult] using the @@ -612,7 +666,9 @@ log.fine('Contents:\n$yamlEditor'); for (final update in updates) { - final dependencyKey = update.isDev ? 'dev_dependencies' : 'dependencies'; + final dependencyKey = update.isDev + ? 'dev_dependencies' + : (update.isOverride ? 'dependency_overrides' : 'dependencies'); final constraint = update.constraint; final ref = update.ref; final name = ref.name; @@ -657,7 +713,7 @@ /// Remove the package from dev_dependencies if we are adding it to /// dependencies. Refer to [_addPackageToPubspec] for additional discussion. - if (!update.isDev) { + if (!update.isDev && !update.isOverride) { final devDependenciesNode = yamlEditor .parseAt(['dev_dependencies'], orElse: () => YamlScalar.wrap(null)); @@ -689,7 +745,13 @@ final PackageRef ref; final VersionConstraint? constraint; final bool isDev; - _ParseResult(this.ref, this.constraint, {required this.isDev}); + final bool isOverride; + _ParseResult( + this.ref, + this.constraint, { + required this.isDev, + required this.isOverride, + }); } extension on ArgResults {
diff --git a/lib/src/command/remove.dart b/lib/src/command/remove.dart index 01e10c2..cf65241 100644 --- a/lib/src/command/remove.dart +++ b/lib/src/command/remove.dart
@@ -20,9 +20,18 @@ @override String get name => 'remove'; @override - String get description => 'Removes a dependency from the current package.'; + String get description => ''' +Removes dependencies from `pubspec.yaml`. + +Invoking `dart pub remove foo bar` will remove `foo` and `bar` from either +`dependencies` or `dev_dependencies` in `pubspec.yaml`. + +To remove a dependency override of a package prefix the package name with +'override:'. +'''; + @override - String get argumentsDescription => '<package>'; + String get argumentsDescription => '<package1> [<package2>...]'; @override String get docUrl => 'https://dart.dev/tools/pub/cmd/pub-remove'; @override @@ -68,11 +77,16 @@ usageException('Must specify a package to be removed.'); } - final packages = Set<String>.from(argResults.rest); + final targets = Set<String>.from(argResults.rest).map((descriptor) { + final isOverride = descriptor.startsWith('override:'); + final name = + isOverride ? descriptor.substring('override:'.length) : descriptor; + return _PackageRemoval(name, removeFromOverride: isOverride); + }); if (isDryRun) { final rootPubspec = entrypoint.root.pubspec; - final newPubspec = _removePackagesFromPubspec(rootPubspec, packages); + final newPubspec = _removePackagesFromPubspec(rootPubspec, targets); final newRoot = Package.inMemory(newPubspec); await Entrypoint.inMemory(newRoot, cache, lockFile: entrypoint.lockFile) @@ -84,7 +98,7 @@ ); } else { /// Update the pubspec. - _writeRemovalToPubspec(packages); + _writeRemovalToPubspec(targets); /// Create a new [Entrypoint] since we have to reprocess the updated /// pubspec file. @@ -107,43 +121,53 @@ } } - Pubspec _removePackagesFromPubspec(Pubspec original, Set<String> packages) { - final originalDependencies = original.dependencies.values; - final originalDevDependencies = original.devDependencies.values; + Pubspec _removePackagesFromPubspec( + Pubspec original, + Iterable<_PackageRemoval> packages, + ) { + final dependencies = {...original.dependencies}; + final devDependencies = {...original.devDependencies}; + final overrides = {...original.dependencyOverrides}; - final newDependencies = originalDependencies - .where((dependency) => !packages.contains(dependency.name)); - final newDevDependencies = originalDevDependencies - .where((dependency) => !packages.contains(dependency.name)); - + for (final package in packages) { + if (package.removeFromOverride) { + overrides.remove(package.name); + } else { + dependencies.remove(package.name); + devDependencies.remove(package.name); + } + } return Pubspec( original.name, version: original.version, sdkConstraints: original.sdkConstraints, - dependencies: newDependencies, - devDependencies: newDevDependencies, - dependencyOverrides: original.dependencyOverrides.values, + dependencies: dependencies.values, + devDependencies: devDependencies.values, + dependencyOverrides: overrides.values, ); } /// Writes the changes to the pubspec file - void _writeRemovalToPubspec(Set<String> packages) { + void _writeRemovalToPubspec(Iterable<_PackageRemoval> packages) { ArgumentError.checkNotNull(packages, 'packages'); final yamlEditor = YamlEditor(readTextFile(entrypoint.pubspecPath)); - for (var package in packages) { + for (final package in packages) { + final dependencyKeys = package.removeFromOverride + ? ['dependency_overrides'] + : ['dependencies', 'dev_dependencies']; var found = false; + final name = package.name; /// There may be packages where the dependency is declared both in - /// dependencies and dev_dependencies. - for (final dependencyKey in ['dependencies', 'dev_dependencies']) { + /// dependencies and dev_dependencies - remove it from both in that case. + for (final dependencyKey in dependencyKeys) { final dependenciesNode = yamlEditor .parseAt([dependencyKey], orElse: () => YamlScalar.wrap(null)); - if (dependenciesNode is YamlMap && - dependenciesNode.containsKey(package)) { - yamlEditor.remove([dependencyKey, package]); + if (dependenciesNode is YamlMap && dependenciesNode.containsKey(name)) { + yamlEditor.remove([dependencyKey, name]); found = true; // Check if the dependencies or dev_dependencies map is now empty // If it is empty, remove the key as well @@ -152,13 +176,20 @@ } } } - if (!found) { - log.warning('Package "$package" was not found in pubspec.yaml!'); + log.warning( + 'Package "$name" was not found in ${entrypoint.pubspecPath}!', + ); } - - /// Windows line endings are already handled by [yamlEditor] - writeTextFile(entrypoint.pubspecPath, yamlEditor.toString()); } + + /// Windows line endings are already handled by [yamlEditor] + writeTextFile(entrypoint.pubspecPath, yamlEditor.toString()); } } + +class _PackageRemoval { + final String name; + final bool removeFromOverride; + _PackageRemoval(this.name, {required this.removeFromOverride}); +}
diff --git a/test/add/common/add_test.dart b/test/add/common/add_test.dart index 548f680..512a7ae 100644 --- a/test/add/common/add_test.dart +++ b/test/add/common/add_test.dart
@@ -1010,4 +1010,58 @@ ]), ); }); + + test('adds to overrides', () async { + final server = await servePackages(); + server.serve('foo', '1.0.0', deps: {'bar': '1.0.0'}); + server.serve('bar', '1.0.0'); + server.serve('bar', '2.0.0'); + + await d.dir('local_foo', [d.libPubspec('foo', '1.0.0')]).create(); + + await d.dir(appPath, [ + d.file('pubspec.yaml', ''' +name: myapp +dependencies: + foo: ^1.0.0 +environment: + sdk: '$defaultSdkConstraint' +'''), + ]).create(); + + await pubGet(); + + await pubAdd( + args: ['override:bar'], + exitCode: exit_codes.USAGE, + error: contains('A dependency override needs an explicit descriptor.'), + ); + + // Can override a transitive dependency. + await pubAdd(args: ['override:bar:2.0.0']); + await d.dir(appPath, [ + d.file( + 'pubspec.yaml', + contains(''' +dependency_overrides: + bar: 2.0.0 +'''), + ) + ]).validate(); + + // Can override with a descriptor: + await pubAdd(args: ['override:foo:{"path": "../local_foo"}']); + + await d.dir(appPath, [ + d.file( + 'pubspec.yaml', + contains(''' +dependency_overrides: + bar: 2.0.0 + foo: + path: ../local_foo +'''), + ) + ]).validate(); + }); }
diff --git a/test/remove/remove_test.dart b/test/remove/remove_test.dart index 689b70f..9abdfc6 100644 --- a/test/remove/remove_test.dart +++ b/test/remove/remove_test.dart
@@ -235,6 +235,35 @@ await d.appDir(dependencies: {'bar': '2.0.1'}).validate(); }); + test('removes overrides', () async { + final server = await servePackages(); + server.serve('foo', '1.0.0', deps: {'bar': '1.0.0'}); + server.serve('bar', '1.0.0'); + server.serve('bar', '2.0.0'); + + await d.dir(appPath, [ + d.pubspec({ + 'name': 'myapp', + 'dependencies': {'foo': '^1.0.0'}, + 'dev_dependencies': {'bar': '^2.0.0'}, + 'dependency_overrides': {'bar': '1.0.0'} + }) + ]).create(); + + await pubGet(); + + // Cannot remove the constraint on bar, would create conflict. + await pubRemove( + args: ['override:bar'], + error: contains('version solving failed.'), + exitCode: 1, + ); + await pubRemove(args: ['override:bar', 'foo']); + await d.appPackageConfigFile([ + d.packageConfigEntry(name: 'bar', version: '2.0.0'), + ]).validate(); + }); + test('preserves comments', () async { await servePackages() ..serve('bar', '1.0.0')
diff --git a/test/test_pub.dart b/test/test_pub.dart index e389f29..2bb9002 100644 --- a/test/test_pub.dart +++ b/test/test_pub.dart
@@ -538,7 +538,7 @@ dartArgs, environment: mergedEnvironment, workingDirectory: workingDirectory ?? _pathInSandbox(appPath), - description: args.isEmpty ? 'pub' : 'pub ${args.first}', + description: args.isEmpty ? 'pub' : 'pub ${args.join(' ')}', includeParentEnvironment: false, ); }
diff --git a/test/testdata/goldens/embedding/embedding_test/--help.txt b/test/testdata/goldens/embedding/embedding_test/--help.txt index 780b29e..0211fa4 100644 --- a/test/testdata/goldens/embedding/embedding_test/--help.txt +++ b/test/testdata/goldens/embedding/embedding_test/--help.txt
@@ -24,7 +24,7 @@ logout Log out of pub.dev. outdated Analyze your dependencies to find which ones can be upgraded. publish Publish the current package to pub.dev. - remove Removes a dependency from the current package. + remove Removes dependencies from `pubspec.yaml`. token Manage authentication tokens for hosted pub repositories. upgrade Upgrade the current package's dependencies to latest versions.
diff --git a/test/testdata/goldens/help_test/pub add --help.txt b/test/testdata/goldens/help_test/pub add --help.txt index dfed108..f81cf7a 100644 --- a/test/testdata/goldens/help_test/pub add --help.txt +++ b/test/testdata/goldens/help_test/pub add --help.txt
@@ -9,6 +9,8 @@ Add to dev_dependencies by prefixing with "dev:". +Make dependency overrides by prefixing with "override:". + Add packages with specific constraints or other sources by giving a descriptor after a colon. @@ -29,11 +31,13 @@ `dart pub add 'foo:{"sdk":"flutter"}'` * Add a git dependency: `dart pub add 'foo:{"git":"https://github.com/foo/foo"}'` + * Add a dependency override: + `dart pub add 'override:foo:1.0.0` * Add a git dependency with a path and ref specified: `dart pub add \ - 'foo:{"git":{"url":"../foo.git","ref":"branch","path":"subdir"}}'` + 'foo:{"git":{"url":"../foo.git","ref":"<branch>","path":"<subdir>"}}'` -Usage: pub add [options] [dev:]<package>[:descriptor] [[dev:]<package>[:descriptor] ...] +Usage: pub add [options] [<section>:]<package>[:descriptor] [<section>:]<package2>[:descriptor] ...] -h, --help Print this usage information. --[no-]offline Use cached packages instead of accessing the network. -n, --dry-run Report what dependencies would change but don't change any.
diff --git a/test/testdata/goldens/help_test/pub remove --help.txt b/test/testdata/goldens/help_test/pub remove --help.txt index f8da56e..2032abf 100644 --- a/test/testdata/goldens/help_test/pub remove --help.txt +++ b/test/testdata/goldens/help_test/pub remove --help.txt
@@ -2,9 +2,16 @@ ## Section 0 $ pub remove --help -Removes a dependency from the current package. +Removes dependencies from `pubspec.yaml`. -Usage: pub remove <package> +Invoking `dart pub remove foo bar` will remove `foo` and `bar` from either +`dependencies` or `dev_dependencies` in `pubspec.yaml`. + +To remove a dependency override of a package prefix the package name with +'override:'. + + +Usage: pub remove <package1> [<package2>...] -h, --help Print this usage information. --[no-]offline Use cached packages instead of accessing the network. -n, --dry-run Report what dependencies would change but don't change any.