Support multiple packages in 'dart pub add' (#3283)
diff --git a/lib/src/command/add.dart b/lib/src/command/add.dart index f268381..d540978 100644 --- a/lib/src/command/add.dart +++ b/lib/src/command/add.dart
@@ -13,6 +13,7 @@ import '../exceptions.dart'; import '../git.dart'; import '../io.dart'; +import '../language_version.dart'; import '../log.dart' as log; import '../package.dart'; import '../package_name.dart'; @@ -33,9 +34,10 @@ @override String get name => 'add'; @override - String get description => 'Add a dependency to pubspec.yaml.'; + String get description => 'Add dependencies to pubspec.yaml.'; @override - String get argumentsDescription => '<package>[:<constraint>] [options]'; + String get argumentsDescription => + '<package>[:<constraint>] [<package2>[:<constraint2>]...] [options]'; @override String get docUrl => 'https://dart.dev/tools/pub/cmd/pub-add'; @override @@ -53,19 +55,24 @@ bool get hasGitOptions => gitUrl != null || gitRef != null || gitPath != null; bool get hasHostOptions => hostUrl != null; + bool get isHosted => !hasGitOptions && path == null && path == null; + AddCommand() { argParser.addFlag('dev', abbr: 'd', negatable: false, - help: 'Adds package to the development dependencies instead.'); + help: 'Adds to the development dependencies instead.'); argParser.addOption('git-url', help: 'Git URL of the package'); argParser.addOption('git-ref', help: 'Git branch or commit to be retrieved'); argParser.addOption('git-path', help: 'Path of git package in repository'); argParser.addOption('hosted-url', help: 'URL of package host server'); - argParser.addOption('path', help: 'Local path'); - argParser.addOption('sdk', help: 'SDK source for package'); + argParser.addOption('path', help: 'Add package from local path'); + argParser.addOption('sdk', + help: 'add package from SDK source', + allowed: ['flutter'], + valueHelp: '[flutter]'); argParser.addFlag( 'example', help: @@ -84,23 +91,28 @@ argParser.addFlag('precompile', help: 'Build executables in immediate dependencies.'); argParser.addOption('directory', - abbr: 'C', help: 'Run this in the directory<dir>.', valueHelp: 'dir'); + abbr: 'C', help: 'Run this in the directory <dir>.', valueHelp: 'dir'); } @override Future<void> runProtected() async { if (argResults.rest.isEmpty) { - usageException('Must specify a package to be added.'); - } else if (argResults.rest.length > 1) { - usageException('Takes only a single argument.'); + usageException('Must specify at least one package to be added.'); + } else if (argResults.rest.length > 1 && gitUrl != null) { + usageException('Can only add a single git package at a time.'); + } else if (argResults.rest.length > 1 && path != null) { + usageException('Can only add a single local package at a time.'); } + final languageVersion = entrypoint.root.pubspec.languageVersion; + final updates = + argResults.rest.map((p) => _parsePackage(p, languageVersion)).toList(); - final packageInformation = _parsePackage(argResults.rest.first); - final package = packageInformation.first; - - /// Perform version resolution in-memory. - final updatedPubSpec = - await _addPackageToPubspec(entrypoint.root.pubspec, package); + var updatedPubSpec = entrypoint.root.pubspec; + for (final update in updates) { + /// Perform version resolution in-memory. + updatedPubSpec = + await _addPackageToPubspec(updatedPubSpec, update.packageRange); + } late SolveResult solveResult; @@ -113,7 +125,9 @@ solveResult = await resolveVersions( SolveType.upgrade, cache, Package.inMemory(updatedPubSpec)); } on GitException { - dataError('Unable to resolve package "${package.name}" with the given ' + final packageRange = updates.first.packageRange; + dataError( + 'Unable to resolve package "${packageRange.name}" with the given ' 'git parameters.'); } on SolveFailure catch (e) { dataError(e.message); @@ -122,24 +136,26 @@ dataError(e.message); } - final resultPackage = solveResult.packages - .firstWhere((packageId) => packageId.name == package.name); + /// Verify the results for each package. + for (final update in updates) { + final packageRange = update.packageRange; + final name = packageRange.name; + final resultPackage = solveResult.packages + .firstWhere((packageId) => packageId.name == name); - /// Assert that [resultPackage] is within the original user's expectations. - var constraint = package.constraint; - if (!constraint.allows(resultPackage.version)) { - var dependencyOverrides = updatedPubSpec.dependencyOverrides; - if (dependencyOverrides.isNotEmpty) { - dataError( - '"${package.name}" resolved to "${resultPackage.version}" which ' - 'does not satisfy constraint "${package.constraint}". This could be ' - 'caused by "dependency_overrides".'); + /// Assert that [resultPackage] is within the original user's expectations. + var constraint = packageRange.constraint; + if (!constraint.allows(resultPackage.version)) { + var dependencyOverrides = updatedPubSpec.dependencyOverrides; + if (dependencyOverrides.isNotEmpty) { + dataError('"$name" resolved to "${resultPackage.version}" which ' + 'does not satisfy constraint "$constraint". This could be ' + 'caused by "dependency_overrides".'); + } + dataError('"$name" resolved to "${resultPackage.version}" which ' + 'does not satisfy constraint "$constraint".'); } - dataError( - '"${package.name}" resolved to "${resultPackage.version}" which ' - 'does not satisfy constraint "${package.constraint}".'); } - if (isDryRun) { /// Even if it is a dry run, run `acquireDependencies` so that the user /// gets a report on the other packages that might change version due @@ -158,7 +174,7 @@ /// ensure that the modification timestamp on `pubspec.lock` and /// `.dart_tool/package_config.json` is newer than `pubspec.yaml`, /// ensuring that [entrypoint.assertUptoDate] will pass. - _updatePubspec(resultPackage, packageInformation, isDev); + _updatePubspec(solveResult.packages, updates, isDev); /// Create a new [Entrypoint] since we have to reprocess the updated /// pubspec file. @@ -274,9 +290,7 @@ /// /// If any of the other git options are defined when `--git-url` is not /// defined, an error will be thrown. - Pair<PackageRange, dynamic> _parsePackage(String package) { - ArgumentError.checkNotNull(package, 'package'); - + _ParseResult _parsePackage(String package, LanguageVersion languageVersion) { final _conflictingFlagSets = [ ['git-url', 'git-ref', 'git-path'], ['hosted-url'], @@ -380,13 +394,18 @@ .withConstraint(constraint ?? VersionConstraint.any); pubspecInformation = {'sdk': sdk}; } else { - final hostInfo = - hasHostOptions ? {'url': hostUrl, 'name': packageName} : null; - - if (hostInfo == null) { - pubspecInformation = constraint?.toString(); + // Hosted + final Object? hostInfo; + if (hasHostOptions) { + hostInfo = languageVersion.supportsShorterHostedSyntax + ? hostUrl + : {'url': hostUrl, 'name': packageName}; + pubspecInformation = { + 'hosted': hostInfo, + }; } else { - pubspecInformation = {'hosted': hostInfo}; + hostInfo = null; + pubspecInformation = constraint?.toString(); } packageRange = cache.hosted.source @@ -408,66 +427,64 @@ }; } - return Pair(packageRange, pubspecInformation); + return _ParseResult(packageRange, pubspecInformation); } /// Writes the changes to the pubspec file. - void _updatePubspec(PackageId resultPackage, - Pair<PackageRange, dynamic> packageInformation, bool isDevelopment) { - ArgumentError.checkNotNull(resultPackage, 'resultPackage'); - ArgumentError.checkNotNull(packageInformation, 'pubspecInformation'); - - final package = packageInformation.first; - var pubspecInformation = packageInformation.last; - - if ((sdk != null || hasHostOptions) && - pubspecInformation is Map && - pubspecInformation['version'] == null) { - /// We cannot simply assign the value of version since it is likely that - /// [pubspecInformation] takes on the type - /// [Map<String, Map<String, String>>] - pubspecInformation = { - ...pubspecInformation, - 'version': '^${resultPackage.version}' - }; - } - - final dependencyKey = isDevelopment ? 'dev_dependencies' : 'dependencies'; - final packagePath = [dependencyKey, package.name]; - + void _updatePubspec(List<PackageId> resultPackages, + List<_ParseResult> updates, bool isDevelopment) { final yamlEditor = YamlEditor(readTextFile(entrypoint.pubspecPath)); log.io('Reading ${entrypoint.pubspecPath}.'); log.fine('Contents:\n$yamlEditor'); - /// Handle situations where the user might not have the dependencies or - /// dev_dependencies map. - if (yamlEditor.parseAt([dependencyKey], - orElse: () => YamlScalar.wrap(null)).value == - null) { - yamlEditor.update([dependencyKey], - {package.name: pubspecInformation ?? '^${resultPackage.version}'}); - } else { - yamlEditor.update( - packagePath, pubspecInformation ?? '^${resultPackage.version}'); - } + for (final update in updates) { + final packageRange = update.packageRange; + final name = packageRange.name; + final resultId = resultPackages.firstWhere((id) => id.name == name); + var description = update.description; - log.fine('Added ${package.name} to "$dependencyKey".'); - - /// Remove the package from dev_dependencies if we are adding it to - /// dependencies. Refer to [_addPackageToPubspec] for additional discussion. - if (!isDevelopment) { - final devDependenciesNode = yamlEditor - .parseAt(['dev_dependencies'], orElse: () => YamlScalar.wrap(null)); - - if (devDependenciesNode is YamlMap && - devDependenciesNode.containsKey(package.name)) { - if (devDependenciesNode.length == 1) { - yamlEditor.remove(['dev_dependencies']); - } else { - yamlEditor.remove(['dev_dependencies', package.name]); + if (isHosted) { + final inferredConstraint = + VersionConstraint.compatibleWith(resultId.version).toString(); + if (description == null) { + description = inferredConstraint; + } else if (description is Map && description['version'] == null) { + /// We cannot simply assign the value of version since it is likely that + /// [description] takes on the type + /// [Map<String, Map<String, String>>] + description = {...description, 'version': '^${resultId.version}'}; } + } - log.fine('Removed ${package.name} from "dev_dependencies".'); + final dependencyKey = isDevelopment ? 'dev_dependencies' : 'dependencies'; + final packagePath = [dependencyKey, name]; + + /// Ensure we have a [dependencyKey] map in the `pubspec.yaml`. + if (yamlEditor.parseAt([dependencyKey], + orElse: () => YamlScalar.wrap(null)).value == + null) { + yamlEditor.update([dependencyKey], {}); + } + yamlEditor.update(packagePath, description); + + log.fine('Added ${packageRange.name} to "$dependencyKey".'); + + /// Remove the package from dev_dependencies if we are adding it to + /// dependencies. Refer to [_addPackageToPubspec] for additional discussion. + if (!isDevelopment) { + final devDependenciesNode = yamlEditor + .parseAt(['dev_dependencies'], orElse: () => YamlScalar.wrap(null)); + + if (devDependenciesNode is YamlMap && + devDependenciesNode.containsKey(name)) { + if (devDependenciesNode.length == 1) { + yamlEditor.remove(['dev_dependencies']); + } else { + yamlEditor.remove(['dev_dependencies', name]); + } + + log.fine('Removed $name from "dev_dependencies".'); + } } } @@ -475,3 +492,9 @@ writeTextFile(entrypoint.pubspecPath, yamlEditor.toString()); } } + +class _ParseResult { + PackageRange packageRange; + Object? description; + _ParseResult(this.packageRange, this.description); +}
diff --git a/lib/src/language_version.dart b/lib/src/language_version.dart index e294014..ad331b2 100644 --- a/lib/src/language_version.dart +++ b/lib/src/language_version.dart
@@ -69,6 +69,21 @@ bool get supportsNullSafety => this >= firstVersionWithNullSafety; + /// Minimum language version at which short hosted syntax is supported. + /// + /// This allows `hosted` dependencies to be expressed as: + /// ```yaml + /// dependencies: + /// foo: + /// hosted: https://some-pub.com/path + /// version: ^1.0.0 + /// ``` + /// + /// At older versions, `hosted` dependencies had to be a map with a `url` and + /// a `name` key. + bool get supportsShorterHostedSyntax => + this >= firstVersionWithShorterHostedSyntax; + @override int compareTo(LanguageVersion other) { if (major != other.major) return major.compareTo(other.major); @@ -89,6 +104,7 @@ static const defaultLanguageVersion = LanguageVersion(2, 7); static const firstVersionWithNullSafety = LanguageVersion(2, 12); + static const firstVersionWithShorterHostedSyntax = LanguageVersion(2, 15); /// Transform language version to string that can be parsed with /// [LanguageVersion.parse].
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index e371363..b3fa729 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart
@@ -232,8 +232,7 @@ return _HostedDescription(packageName, defaultUrl); } - final canUseShorthandSyntax = - languageVersion >= _minVersionForShorterHostedSyntax; + final canUseShorthandSyntax = languageVersion.supportsShorterHostedSyntax; if (description is String) { // Old versions of pub (pre Dart 2.15) interpret `hosted: foo` as @@ -256,7 +255,7 @@ } else { throw FormatException( 'Using `hosted: <url>` is only supported with a minimum SDK ' - 'constraint of $_minVersionForShorterHostedSyntax.', + 'constraint of ${LanguageVersion.firstVersionWithShorterHostedSyntax}.', ); } } @@ -271,7 +270,7 @@ if (name is! String) { throw FormatException("The 'name' key must have a string value without " - 'a minimum Dart SDK constraint of $_minVersionForShorterHostedSyntax.0 or higher.'); + 'a minimum Dart SDK constraint of ${LanguageVersion.firstVersionWithShorterHostedSyntax}.0 or higher.'); } var url = defaultUrl; @@ -286,21 +285,6 @@ return _HostedDescription(name, url); } - /// Minimum language version at which short hosted syntax is supported. - /// - /// This allows `hosted` dependencies to be expressed as: - /// ```yaml - /// dependencies: - /// foo: - /// hosted: https://some-pub.com/path - /// version: ^1.0.0 - /// ``` - /// - /// At older versions, `hosted` dependencies had to be a map with a `url` and - /// a `name` key. - static const LanguageVersion _minVersionForShorterHostedSyntax = - LanguageVersion(2, 15); - static final RegExp _looksLikePackageName = RegExp(r'^[a-zA-Z_]+[a-zA-Z0-9_]*$'); }
diff --git a/test/add/common/add_test.dart b/test/add/common/add_test.dart index 69f33fe..1a1e9df 100644 --- a/test/add/common/add_test.dart +++ b/test/add/common/add_test.dart
@@ -37,29 +37,6 @@ }); group('normally', () { - test('fails if extra arguments are passed', () async { - final server = await servePackages(); - server.serve('foo', '1.2.2'); - - await d.dir(appPath, [ - d.pubspec({'name': 'myapp'}) - ]).create(); - - await pubAdd( - args: ['foo', '^1.2.2'], - exitCode: exit_codes.USAGE, - error: contains('Takes only a single argument.')); - - await d.dir(appPath, [ - d.pubspec({ - 'name': 'myapp', - }), - d.nothing('.dart_tool/package_config.json'), - d.nothing('pubspec.lock'), - d.nothing('.packages'), - ]).validate(); - }); - test('adds a package from a pub server', () async { final server = await servePackages(); server.serve('foo', '1.2.3'); @@ -73,6 +50,24 @@ await d.appDir({'foo': '1.2.3'}).validate(); }); + test('adds multiple package from a pub server', () async { + final server = await servePackages(); + server.serve('foo', '1.2.3'); + server.serve('bar', '1.1.0'); + server.serve('baz', '2.5.3'); + + await d.appDir({}).create(); + + await pubAdd(args: ['foo:1.2.3', 'bar:1.1.0', 'baz:2.5.3']); + + await d.cacheDir( + {'foo': '1.2.3', 'bar': '1.1.0', 'baz': '2.5.3'}).validate(); + await d.appPackagesFile( + {'foo': '1.2.3', 'bar': '1.1.0', 'baz': '2.5.3'}).validate(); + await d + .appDir({'foo': '1.2.3', 'bar': '1.1.0', 'baz': '2.5.3'}).validate(); + }); + test( 'does not remove empty dev_dependencies while adding to normal dependencies', () async {
diff --git a/test/add/git/git_test.dart b/test/add/git/git_test.dart index 7bc51f0..00bd334 100644 --- a/test/add/git/git_test.dart +++ b/test/add/git/git_test.dart
@@ -184,4 +184,18 @@ }) ]).validate(); }); + + test('fails if multiple packages passed for git source', () async { + ensureGit(); + + await d.git( + 'foo.git', [d.libDir('foo'), d.libPubspec('foo', '1.0.0')]).create(); + + await d.appDir({}).create(); + + await pubAdd( + args: ['foo', 'bar', 'baz', '--git-url', '../foo.git'], + exitCode: exit_codes.USAGE, + error: contains('Can only add a single git package at a time.')); + }); }
diff --git a/test/add/hosted/non_default_pub_server_test.dart b/test/add/hosted/non_default_pub_server_test.dart index eea6c09..0a83ac5 100644 --- a/test/add/hosted/non_default_pub_server_test.dart +++ b/test/add/hosted/non_default_pub_server_test.dart
@@ -35,6 +35,46 @@ }).validate(); }); + test('adds multiple packages from a non-default pub server', () async { + // Make the default server serve errors. Only the custom server should + // be accessed. + (await servePackages()).serveErrors(); + + final server = await servePackages(); + server.serve('foo', '1.1.0'); + server.serve('foo', '1.2.3'); + server.serve('bar', '0.2.5'); + server.serve('bar', '3.2.3'); + server.serve('baz', '0.1.3'); + server.serve('baz', '1.3.5'); + + await d.appDir({}).create(); + + final url = server.url; + + await pubAdd( + args: ['foo:1.2.3', 'bar:3.2.3', 'baz:1.3.5', '--hosted-url', url]); + + await d.cacheDir({'foo': '1.2.3', 'bar': '3.2.3', 'baz': '1.3.5'}, + port: server.port).validate(); + await d.appPackagesFile( + {'foo': '1.2.3', 'bar': '3.2.3', 'baz': '1.3.5'}).validate(); + await d.appDir({ + 'foo': { + 'version': '1.2.3', + 'hosted': {'name': 'foo', 'url': url} + }, + 'bar': { + 'version': '3.2.3', + 'hosted': {'name': 'bar', 'url': url} + }, + 'baz': { + 'version': '1.3.5', + 'hosted': {'name': 'baz', 'url': url} + } + }).validate(); + }); + test('fails when adding from an invalid url', () async { ensureGit();
diff --git a/test/add/path/absolute_path_test.dart b/test/add/path/absolute_path_test.dart index 5b7e726..b15ec2d 100644 --- a/test/add/path/absolute_path_test.dart +++ b/test/add/path/absolute_path_test.dart
@@ -41,6 +41,28 @@ }).validate(); }); + test('fails when adding multiple packages through local path', () async { + ensureGit(); + + await d.git( + 'foo.git', [d.libDir('foo'), d.libPubspec('foo', '1.0.0')]).create(); + + await d.appDir({}).create(); + final absolutePath = path.join(d.sandbox, 'foo'); + + await pubAdd( + args: ['foo:2.0.0', 'bar:0.1.3', 'baz:1.3.1', '--path', absolutePath], + error: contains('Can only add a single local package at a time.'), + exitCode: exit_codes.USAGE); + + await d.appDir({}).validate(); + await d.dir(appPath, [ + d.nothing('.dart_tool/package_config.json'), + d.nothing('pubspec.lock'), + d.nothing('.packages'), + ]).validate(); + }); + test('fails when adding with an invalid version constraint', () async { ensureGit();
diff --git a/test/testdata/goldens/help_test/pub add --help.txt b/test/testdata/goldens/help_test/pub add --help.txt index 5d32e5e..2a37c21 100644 --- a/test/testdata/goldens/help_test/pub add --help.txt +++ b/test/testdata/goldens/help_test/pub add --help.txt
@@ -2,22 +2,23 @@ ## Section 0 $ pub add --help -Add a dependency to pubspec.yaml. +Add dependencies to pubspec.yaml. -Usage: pub add <package>[:<constraint>] [options] +Usage: pub add <package>[:<constraint>] [<package2>[:<constraint2>]...] [options] -h, --help Print this usage information. --d, --dev Adds package to the development dependencies instead. +-d, --dev Adds to the development dependencies instead. --git-url Git URL of the package --git-ref Git branch or commit to be retrieved --git-path Path of git package in repository --hosted-url URL of package host server - --path Local path - --sdk SDK source for package + --path Add package from local path + --sdk=<[flutter]> add package from SDK source + [flutter] --[no-]offline Use cached packages instead of accessing the network. -n, --dry-run Report what dependencies would change but don't change any. --[no-]precompile Build executables in immediate dependencies. --C, --directory=<dir> Run this in the directory<dir>. +-C, --directory=<dir> Run this in the directory <dir>. Run "pub help" to see global options. See https://dart.dev/tools/pub/cmd/pub-add for detailed documentation.