pub upgrade --major-versions (#2784)
* Naive implementation
* Added test, works for dev_dependencies
* Changes per review comments
* Only remove upper bounds
* Function renaming
* Documentation
* Updated documentation
* Fix upgrade --breaking tests
* clean up argument parsing
* Improved dry-run output
* Fixed tests
* Removed unnecessary alias
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Update lib/src/command/upgrade.dart
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
* Added warning for dependency overrides
* nit: fix comment
* Nits and tests for warning
Co-authored-by: Garett Tok <garettok@hotmail.com>
Co-authored-by: Sigurd Meldgaard <sigurdm@google.com>
diff --git a/lib/src/command/upgrade.dart b/lib/src/command/upgrade.dart
index e51124f..36713e1 100644
--- a/lib/src/command/upgrade.dart
+++ b/lib/src/command/upgrade.dart
@@ -16,6 +16,7 @@
import '../package.dart';
import '../package_name.dart';
import '../pubspec.dart';
+import '../pubspec_utils.dart';
import '../solver.dart';
import '../source/hosted.dart';
import '../yaml_edit/editor.dart';
@@ -53,6 +54,13 @@
argParser.addFlag('nullsafety', negatable: false, hide: true);
argParser.addFlag('packages-dir', hide: true);
+
+ argParser.addFlag(
+ 'major-versions',
+ help: 'Upgrades packages to their latest resolvable versions, '
+ 'and updates pubspec.yaml.',
+ negatable: false,
+ );
}
/// Avoid showing spinning progress messages when not in a terminal.
@@ -60,6 +68,13 @@
bool get _dryRun => argResults['dry-run'];
+ bool get _precompile => argResults['precompile'];
+
+ bool get _upgradeNullSafety =>
+ argResults['nullsafety'] || argResults['null-safety'];
+
+ bool get _upgradeMajorVersions => argResults['major-versions'];
+
@override
Future<void> runProtected() async {
if (argResults.wasParsed('packages-dir')) {
@@ -67,10 +82,18 @@
'The --packages-dir flag is no longer used and does nothing.'));
}
- if (argResults['nullsafety'] || argResults['null-safety']) {
+ if (_upgradeNullSafety && _upgradeMajorVersions) {
+ usageException('--major-versions and --null-safety cannot be combined');
+ }
+
+ if (_upgradeNullSafety) {
return await _runUpgradeNullSafety();
}
+ if (_upgradeMajorVersions) {
+ return await _runUpgradeMajorVersions();
+ }
+
return await _runUpgrade();
}
@@ -79,49 +102,164 @@
SolveType.UPGRADE,
unlock: argResults.rest,
dryRun: _dryRun,
- precompile: argResults['precompile'],
+ precompile: _precompile,
);
_showOfflineWarning();
}
- Future<void> _runUpgradeNullSafety() async {
+ /// Return names of packages to be upgraded, and throws [UsageException] if
+ /// any package names not in the direct dependencies or dev_dependencies are given.
+ ///
+ /// This assumes that either `--major-versions` or `--null-safety` was passed.
+ List<String> _directDependenciesToUpgrade() {
+ assert(_upgradeNullSafety || _upgradeMajorVersions);
+
final directDeps = [
...entrypoint.root.pubspec.dependencies.keys,
...entrypoint.root.pubspec.devDependencies.keys
];
- final upgradeOnly = argResults.rest.isEmpty ? directDeps : argResults.rest;
+ final toUpgrade = argResults.rest.isEmpty ? directDeps : argResults.rest;
// Check that all package names in upgradeOnly are direct-dependencies
- if (upgradeOnly.any((name) => !directDeps.contains(name))) {
- final notDirectDeps =
- upgradeOnly.where((name) => !directDeps.contains(name)).toList();
+ final notInDeps = toUpgrade.where((n) => !directDeps.contains(n));
+ if (toUpgrade.any(notInDeps.contains)) {
+ var modeFlag = '';
+ if (_upgradeNullSafety) {
+ modeFlag = '--null-safety';
+ }
+ if (_upgradeMajorVersions) {
+ modeFlag = '--major-versions';
+ }
+
usageException('''
-Dependencies specified in `dart pub upgrade --nullsafety <dependencies>` must
+Dependencies specified in `dart pub upgrade $modeFlag <dependencies>` must
be direct 'dependencies' or 'dev_dependencies', following packages are not:
- - ${notDirectDeps.join('\n - ')}
+ - ${notInDeps.join('\n - ')}
''');
}
+ return toUpgrade;
+ }
+
+ Future<void> _runUpgradeMajorVersions() async {
+ final toUpgrade = _directDependenciesToUpgrade();
+
+ final resolvablePubspec = stripVersionUpperBounds(
+ entrypoint.root.pubspec,
+ stripOnly: toUpgrade,
+ );
+
+ // Solve [resolvablePubspec] in-memory and consolidate the resolved
+ // versions of the packages into a map for quick searching.
+ final resolvedPackages = <String, PackageId>{};
+ final solveResult = await log.spinner('Resolving dependencies', () async {
+ return await resolveVersions(
+ SolveType.UPGRADE,
+ cache,
+ Package.inMemory(resolvablePubspec),
+ );
+ }, condition: _shouldShowSpinner);
+ for (final resolvedPackage in solveResult?.packages ?? []) {
+ resolvedPackages[resolvedPackage.name] = resolvedPackage;
+ }
+
+ // Changes to be made to `pubspec.yaml`.
+ // Mapping from original to changed value.
+ final changes = <PackageRange, PackageRange>{};
+ final declaredHostedDependencies = [
+ ...entrypoint.root.pubspec.dependencies.values,
+ ...entrypoint.root.pubspec.devDependencies.values,
+ ].where((dep) => dep.source is HostedSource);
+ for (final dep in declaredHostedDependencies) {
+ final resolvedPackage = resolvedPackages[dep.name];
+ assert(resolvedPackage != null);
+ if (resolvedPackage == null || !toUpgrade.contains(dep.name)) {
+ // If we're not to upgrade this package, or it wasn't in the
+ // resolution somehow, then we ignore it.
+ continue;
+ }
+
+ // Skip [dep] if it has a dependency_override.
+ if (entrypoint.root.dependencyOverrides.containsKey(dep.name)) {
+ continue;
+ }
+
+ if (dep.constraint.allowsAll(resolvedPackage.version)) {
+ // If constraint allows the resolvable version we found, then there is
+ // no need to update the `pubspec.yaml`
+ continue;
+ }
+
+ changes[dep] = dep.withConstraint(VersionConstraint.compatibleWith(
+ resolvedPackage.version,
+ ));
+ }
+
+ if (_dryRun) {
+ // Even if it is a dry run, run `acquireDependencies` so that the user
+ // gets a report on changes.
+ // TODO(jonasfj): Stop abusing Entrypoint.global for dry-run output
+ await Entrypoint.global(
+ Package.inMemory(resolvablePubspec),
+ entrypoint.lockFile,
+ cache,
+ solveResult: solveResult,
+ ).acquireDependencies(
+ SolveType.UPGRADE,
+ dryRun: true,
+ precompile: _precompile,
+ );
+ } else {
+ await _updatePubspec(changes);
+
+ // TODO: Allow Entrypoint to be created with in-memory pubspec, so that
+ // we can show the changes when not in --dry-run mode. For now we only show
+ // the changes made to pubspec.yaml in dry-run mode.
+ await Entrypoint.current(cache).acquireDependencies(
+ SolveType.UPGRADE,
+ precompile: _precompile,
+ );
+ }
+
+ _outputChangeSummary(changes);
+
+ // If any of the packages to upgrade are dependency overrides, then we
+ // show a warning.
+ final toUpgradeOverrides =
+ toUpgrade.where(entrypoint.root.dependencyOverrides.containsKey);
+ if (toUpgradeOverrides.isNotEmpty) {
+ log.warning(
+ 'Warning: dependency_overrides prevents upgrades for: '
+ '${toUpgradeOverrides.join(', ')}',
+ );
+ }
+
+ _showOfflineWarning();
+ }
+
+ Future<void> _runUpgradeNullSafety() async {
+ final toUpgrade = _directDependenciesToUpgrade();
+
final nullsafetyPubspec = await _upgradeToNullSafetyConstraints(
entrypoint.root.pubspec,
- upgradeOnly,
+ toUpgrade,
);
/// Solve [nullsafetyPubspec] in-memory and consolidate the resolved
/// versions of the packages into a map for quick searching.
final resolvedPackages = <String, PackageId>{};
- await log.spinner('Resolving dependencies', () async {
- final solveResult = await resolveVersions(
+ final solveResult = await log.spinner('Resolving dependencies', () async {
+ return await resolveVersions(
SolveType.UPGRADE,
cache,
Package.inMemory(nullsafetyPubspec),
);
- for (final resolvedPackage in solveResult?.packages ?? []) {
- resolvedPackages[resolvedPackage.name] = resolvedPackage;
- }
}, condition: _shouldShowSpinner);
+ for (final resolvedPackage in solveResult?.packages ?? []) {
+ resolvedPackages[resolvedPackage.name] = resolvedPackage;
+ }
/// Changes to be made to `pubspec.yaml`.
/// Mapping from original to changed value.
@@ -133,7 +271,7 @@
for (final dep in declaredHostedDependencies) {
final resolvedPackage = resolvedPackages[dep.name];
assert(resolvedPackage != null);
- if (resolvedPackage == null || !upgradeOnly.contains(dep.name)) {
+ if (resolvedPackage == null || !toUpgrade.contains(dep.name)) {
// If we're not to upgrade this package, or it wasn't in the
// resolution somehow, then we ignore it.
continue;
@@ -152,7 +290,21 @@
changes[dep] = dep.withConstraint(constraint);
}
- if (!_dryRun) {
+ if (_dryRun) {
+ // Even if it is a dry run, run `acquireDependencies` so that the user
+ // gets a report on changes.
+ // TODO(jonasfj): Stop abusing Entrypoint.global for dry-run output
+ await Entrypoint.global(
+ Package.inMemory(nullsafetyPubspec),
+ entrypoint.lockFile,
+ cache,
+ solveResult: solveResult,
+ ).acquireDependencies(
+ SolveType.UPGRADE,
+ dryRun: true,
+ precompile: _precompile,
+ );
+ } else {
await _updatePubspec(changes);
// TODO: Allow Entrypoint to be created with in-memory pubspec, so that
@@ -160,7 +312,7 @@
// the changes made to pubspec.yaml in dry-run mode.
await Entrypoint.current(cache).acquireDependencies(
SolveType.UPGRADE,
- precompile: argResults['precompile'],
+ precompile: _precompile,
);
}
@@ -173,6 +325,10 @@
// - root has git or path dependencies,
// - root has dependency_overrides
final nonMigratedDirectDeps = <String>[];
+ final directDeps = [
+ ...entrypoint.root.pubspec.dependencies.keys,
+ ...entrypoint.root.pubspec.devDependencies.keys
+ ];
await Future.wait(directDeps.map((name) async {
final resolvedPackage = resolvedPackages[name];
assert(resolvedPackage != null);
@@ -196,6 +352,8 @@
* Find other packages to use.
''');
}
+
+ _showOfflineWarning();
}
/// Updates `pubspec.yaml` with given [changes].
diff --git a/test/upgrade/dry_run_does_not_apply_changes_test.dart b/test/upgrade/dry_run_does_not_apply_changes_test.dart
index 5698b39..5304e5e 100644
--- a/test/upgrade/dry_run_does_not_apply_changes_test.dart
+++ b/test/upgrade/dry_run_does_not_apply_changes_test.dart
@@ -11,7 +11,7 @@
import '../test_pub.dart';
void main() {
- test('--dry-run shows report but does not apply changes', () async {
+ test('--dry-run: shows report, changes nothing', () async {
await servePackages((builder) {
builder.serve('foo', '1.0.0');
builder.serve('foo', '2.0.0');
@@ -22,25 +22,90 @@
await pubGet();
+ await d.dir(appPath, [
+ d.file('pubspec.lock', contains('1.0.0')),
+ d.dir('.dart_tool'),
+ ]).validate();
+
// Change the pubspec.
await d.appDir({'foo': 'any'}).create();
- // Also delete the "packages" directory.
- deleteEntry(path.join(d.sandbox, appPath, 'packages'));
+ // Also delete the ".dart_tool" directory.
+ deleteEntry(path.join(d.sandbox, appPath, '.dart_tool'));
// Do the dry run.
await pubUpgrade(
- args: ['--dry-run'],
- output: allOf([
- contains('> foo 2.0.0 (was 1.0.0)'),
- contains('Would change 1 dependency.')
- ]));
+ args: ['--dry-run'],
+ output: allOf([
+ contains('> foo 2.0.0 (was 1.0.0)'),
+ contains('Would change 1 dependency.'),
+ ]),
+ );
await d.dir(appPath, [
// The lockfile should be unmodified.
d.file('pubspec.lock', contains('1.0.0')),
- // The "packages" directory should not have been regenerated.
- d.nothing('packages')
+ // The ".dart_tool" directory should not have been regenerated.
+ d.nothing('.dart_tool')
+ ]).validate();
+ });
+
+ test('--dry-run --major-versions: shows report, changes nothing', () async {
+ await servePackages((builder) {
+ builder.serve('foo', '1.0.0');
+ builder.serve('foo', '2.0.0');
+ });
+
+ await d.appDir({'foo': '^1.0.0'}).create();
+
+ await pubGet();
+
+ await d.dir(appPath, [
+ d.file('pubspec.lock', contains('1.0.0')),
+ d.dir('.dart_tool'),
+ ]).validate();
+
+ // Also delete the ".dart_tool" directory.
+ deleteEntry(path.join(d.sandbox, appPath, '.dart_tool'));
+
+ // Do the dry run.
+ await pubUpgrade(
+ args: ['--dry-run', '--major-versions'],
+ output: allOf([
+ contains('Resolving dependencies...'),
+ contains('> foo 2.0.0 (was 1.0.0)'),
+ contains('Would change 1 dependency.'),
+ contains('Would change 1 constraint in pubspec.yaml:'),
+ contains('foo: ^1.0.0 -> ^2.0.0'),
+ ]),
+ );
+
+ await d.dir(appPath, [
+ // The pubspec should not be modified.
+ d.appPubspec({'foo': '^1.0.0'}),
+ // The lockfile should not be modified.
+ d.file('pubspec.lock', contains('1.0.0')),
+ // The ".dart_tool" directory should not have been regenerated.
+ d.nothing('.dart_tool')
+ ]).validate();
+
+ // Try without --dry-run
+ await pubUpgrade(
+ args: ['--major-versions'],
+ output: allOf([
+ contains('Resolving dependencies...'),
+ contains('> foo 2.0.0 (was 1.0.0)'),
+ contains('Downloading foo 2.0.0...'),
+ contains('Changed 1 dependency!'),
+ contains('Changed 1 constraint in pubspec.yaml:'),
+ contains('foo: ^1.0.0 -> ^2.0.0'),
+ ]),
+ );
+
+ await d.dir(appPath, [
+ d.appPubspec({'foo': '^2.0.0'}),
+ d.file('pubspec.lock', contains('2.0.0')),
+ d.dir('.dart_tool')
]).validate();
});
}
diff --git a/test/upgrade/upgrade_major_versions_test.dart b/test/upgrade/upgrade_major_versions_test.dart
new file mode 100644
index 0000000..c312717
--- /dev/null
+++ b/test/upgrade/upgrade_major_versions_test.dart
@@ -0,0 +1,264 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:test/test.dart';
+
+import '../descriptor.dart' as d;
+import '../test_pub.dart';
+
+void main() {
+ group('pub upgrade --major-versions', () {
+ test('bumps dependency constraints and shows summary report', () async {
+ await servePackages((builder) {
+ builder.serve('foo', '1.0.0');
+ builder.serve('foo', '2.0.0');
+ builder.serve('bar', '0.1.0');
+ builder.serve('bar', '0.2.0');
+ builder.serve('baz', '1.0.0');
+ builder.serve('baz', '1.0.1');
+ });
+
+ await d.appDir({
+ 'foo': '^1.0.0',
+ 'bar': '^0.1.0',
+ 'baz': '^1.0.0',
+ }).create();
+
+ await pubGet();
+
+ // 2 constraints should be updated
+ await pubUpgrade(
+ args: ['--major-versions'],
+ output: allOf([
+ contains('Changed 2 constraints in pubspec.yaml:'),
+ contains('foo: ^1.0.0 -> ^2.0.0'),
+ contains('bar: ^0.1.0 -> ^0.2.0'),
+ ]),
+ );
+
+ await d.appDir({
+ 'foo': '^2.0.0',
+ 'bar': '^0.2.0',
+ 'baz': '^1.0.0',
+ }).validate();
+
+ await d.appPackagesFile({
+ 'foo': '2.0.0',
+ 'bar': '0.2.0',
+ 'baz': '1.0.1',
+ }).validate();
+ });
+
+ test('bumps dev_dependency constraints and shows summary report', () async {
+ await servePackages((builder) {
+ builder.serve('foo', '1.0.0');
+ builder.serve('foo', '2.0.0');
+ builder.serve('bar', '0.1.0');
+ builder.serve('bar', '0.2.0');
+ builder.serve('baz', '1.0.0');
+ builder.serve('baz', '1.0.1');
+ });
+
+ await d.dir(appPath, [
+ d.pubspec({
+ 'name': 'myapp',
+ 'dev_dependencies': {
+ 'foo': '^1.0.0',
+ 'bar': '^0.1.0',
+ 'baz': '^1.0.0',
+ }
+ }),
+ ]).create();
+
+ await pubGet();
+
+ // 2 constraints should be updated
+ await pubUpgrade(
+ args: ['--major-versions'],
+ output: allOf([
+ contains('Changed 2 constraints in pubspec.yaml:'),
+ contains('foo: ^1.0.0 -> ^2.0.0'),
+ contains('bar: ^0.1.0 -> ^0.2.0'),
+ ]),
+ );
+
+ await d.dir(appPath, [
+ d.pubspec({
+ 'name': 'myapp',
+ 'dev_dependencies': {
+ 'foo': '^2.0.0', // bumped
+ 'bar': '^0.2.0', // bumped
+ 'baz': '^1.0.0',
+ }
+ }),
+ ]).validate();
+
+ await d.appPackagesFile({
+ 'foo': '2.0.0',
+ 'bar': '0.2.0',
+ 'baz': '1.0.1',
+ }).validate();
+ });
+
+ test('upgrades only the selected package', () async {
+ await servePackages((builder) {
+ builder.serve('foo', '1.0.0');
+ builder.serve('foo', '2.0.0');
+ builder.serve('bar', '0.1.0');
+ builder.serve('bar', '0.2.0');
+ });
+
+ await d.appDir({
+ 'foo': '^1.0.0',
+ 'bar': '^0.1.0',
+ }).create();
+
+ await pubGet();
+
+ // 1 constraint should be updated
+ await pubUpgrade(
+ args: ['--major-versions', 'foo'],
+ output: allOf([
+ contains('Changed 1 constraint in pubspec.yaml:'),
+ contains('foo: ^1.0.0 -> ^2.0.0'),
+ ]),
+ );
+
+ await d.appDir({
+ 'foo': '^2.0.0', // bumped
+ 'bar': '^0.1.0',
+ }).validate();
+
+ await d.appPackagesFile({'foo': '2.0.0', 'bar': '0.1.0'}).validate();
+ });
+
+ test('chooses the latest version where possible', () async {
+ await servePackages((builder) {
+ builder.serve('foo', '1.0.0');
+ builder.serve('foo', '2.0.0');
+ builder.serve('foo', '3.0.0');
+ });
+
+ await d.appDir({'foo': '^1.0.0'}).create();
+
+ await pubGet();
+
+ await pubUpgrade(
+ args: ['--major-versions'],
+ output: allOf([
+ contains('Changed 1 constraint in pubspec.yaml:'),
+ contains('foo: ^1.0.0 -> ^3.0.0'),
+ ]),
+ );
+
+ await d.dir(appPath, [
+ // The pubspec file should be modified.
+ d.pubspec({
+ 'name': 'myapp',
+ 'dependencies': {'foo': '^3.0.0'}
+ }),
+ // The lockfile should be modified.
+ d.file('pubspec.lock', contains('3.0.0'))
+ ]).validate();
+
+ await d.appPackagesFile({'foo': '3.0.0'}).validate();
+ });
+
+ test('overridden dependencies - no resolution', () async {
+ await servePackages(
+ (builder) => builder
+ ..serve('foo', '1.0.0', deps: {'bar': '^2.0.0'})
+ ..serve('foo', '2.0.0', deps: {'bar': '^1.0.0'})
+ ..serve('bar', '1.0.0', deps: {'foo': '^1.0.0'})
+ ..serve('bar', '2.0.0', deps: {'foo': '^2.0.0'}),
+ );
+
+ await d.dir(appPath, [
+ d.pubspec({
+ 'name': 'myapp',
+ 'version': '1.0.1',
+ 'dependencies': {
+ 'foo': 'any',
+ 'bar': 'any',
+ },
+ 'dependency_overrides': {
+ 'foo': '1.0.0',
+ 'bar': '1.0.0',
+ },
+ })
+ ]).create();
+
+ await pubGet();
+
+ await pubUpgrade(
+ args: ['--major-versions'],
+ output: contains('No changes to pubspec.yaml!'),
+ warning: allOf([
+ contains('Warning: dependency_overrides prevents upgrades for: '),
+ contains('foo'), // ordering not ensured
+ contains('bar'),
+ ]),
+ );
+
+ await d.dir(appPath, [
+ d.pubspec({
+ 'name': 'myapp',
+ 'version': '1.0.1',
+ 'dependencies': {
+ 'foo': 'any',
+ 'bar': 'any',
+ },
+ 'dependency_overrides': {
+ 'foo': '1.0.0',
+ 'bar': '1.0.0',
+ },
+ })
+ ]).validate();
+
+ await d.appPackagesFile({'foo': '1.0.0', 'bar': '1.0.0'}).validate();
+ });
+
+ test('upgrade should not downgrade any versions', () async {
+ /// The version solver solves the packages with the least number of
+ /// versions remaining, so we add more 'bar' packages to force 'foo' to be
+ /// resolved first
+ await servePackages((builder) {
+ builder.serve('foo', '1.0.0');
+ builder.serve('foo', '2.0.0', pubspec: {
+ 'dependencies': {'bar': '1.0.0'}
+ });
+ builder.serve('bar', '1.0.0');
+ builder.serve('bar', '2.0.0');
+ builder.serve('bar', '3.0.0');
+ builder.serve('bar', '4.0.0');
+ });
+
+ await d.appDir({
+ 'foo': '^1.0.0',
+ 'bar': '^2.0.0',
+ }).create();
+
+ await pubGet();
+
+ // 1 constraint should be updated
+ await pubUpgrade(
+ args: ['--major-versions'],
+ output: allOf([
+ contains('Changed 1 constraint in pubspec.yaml:'),
+ contains('bar: ^2.0.0 -> ^4.0.0'),
+ ]),
+ );
+
+ await d.appDir({
+ 'foo': '^1.0.0',
+ 'bar': '^4.0.0',
+ }).validate();
+
+ await d.appPackagesFile({
+ 'foo': '1.0.0',
+ 'bar': '4.0.0',
+ }).validate();
+ });
+ });
+}
