Make cache repair only repair packages from package_config.json by default (#4744)
diff --git a/lib/src/command/cache_repair.dart b/lib/src/command/cache_repair.dart index e5ac233..2d72c10 100644 --- a/lib/src/command/cache_repair.dart +++ b/lib/src/command/cache_repair.dart
@@ -4,10 +4,15 @@ import 'dart:async'; +import 'package:pub_semver/pub_semver.dart'; + import '../command.dart'; import '../exit_codes.dart' as exit_codes; +import '../io.dart'; import '../log.dart' as log; -import '../source/cached.dart'; +import '../package_name.dart'; +import '../source/git.dart'; +import '../source/hosted.dart'; import '../utils.dart'; /// Handles the `cache repair` pub command. @@ -21,17 +26,64 @@ @override bool get takesArguments => false; + CacheRepairCommand() { + argParser.addFlag( + 'all', + help: + 'Repair all cached packages instead of only packages in the ' + 'current pubspec.lock.', + negatable: false, + ); + } + @override Future<void> runProtected() async { + final repairAll = argResults.flag('all'); + + // Get the filters for packages to repair (from lockfile if not --all). + bool Function(String, Version)? hostedPackageFilter; + bool Function(String, Version)? gitPackageFilter; + if (!repairAll) { + if (!entrypoint.canFindWorkspaceRoot) { + log.message( + 'No pubspec.yaml found. ' + 'Run from a Dart project or use --all to repair all cached packages.', + ); + return; + } + + if (!fileExists(entrypoint.lockFilePath)) { + log.message( + 'No pubspec.lock found. ' + 'Run "pub get" first or use --all to repair all cached packages.', + ); + return; + } + + final lockFile = entrypoint.lockFile; + final packages = lockFile.packages.values.toList(); + if (packages.isEmpty) { + log.message('No packages found in pubspec.lock.'); + return; + } + + (hostedPackageFilter, gitPackageFilter) = _buildPackageFilters(packages); + } + // Delete any eventual temp-files left in the cache. cache.deleteTempDir(); + // Repair every cached source. - final repairResults = (await Future.wait( - <CachedSource>[ - cache.hosted, - cache.git, - ].map((source) => source.repairCachedPackages(cache)), - )).expand((x) => x); + final repairResults = [ + ...await cache.hosted.repairCachedPackages( + cache, + packageFilter: hostedPackageFilter, + ), + ...await cache.git.repairCachedPackages( + cache, + packageFilter: gitPackageFilter, + ), + ]; final successes = repairResults.where((result) => result.success); final failures = repairResults.where((result) => !result.success); @@ -83,11 +135,34 @@ } if (successes.isEmpty && failures.isEmpty) { - log.message('No packages in cache, so nothing to repair.'); + if (repairAll) { + log.message('No packages in cache, so nothing to repair.'); + } else { + log.message('No packages from pubspec.lock found in cache.'); + } } if (failures.isNotEmpty || repairFailures.isNotEmpty) { overrideExitCode(exit_codes.UNAVAILABLE); } } + + /// Builds source-specific package filters from the lockfile packages. + /// + /// Returns a tuple of (hostedFilter, gitFilter). + /// - Hosted filter: matches by name AND version. + /// - Git filter: matches by name only (version isn't reliably derivable from + /// cache directory names). + (bool Function(String, Version), bool Function(String, Version)) + _buildPackageFilters(List<PackageId> packages) { + final hostedPackages = + packages.where((p) => p.source is HostedSource).toList(); + final gitPackages = packages.where((p) => p.source is GitSource).toList(); + + return ( + (name, version) => + hostedPackages.any((p) => p.name == name && p.version == version), + (name, version) => gitPackages.any((p) => p.name == name), + ); + } }
diff --git a/lib/src/source/cached.dart b/lib/src/source/cached.dart index 533009b..21a4db9 100644 --- a/lib/src/source/cached.dart +++ b/lib/src/source/cached.dart
@@ -73,9 +73,15 @@ /// Reinstalls all packages that have been previously installed into the /// system cache by this source. /// + /// If [packageFilter] is provided, only packages for which the filter returns + /// `true` will be repaired. The filter receives the package name and version. + /// /// Returns a list of results indicating for each if that package was /// successfully repaired. - Future<Iterable<RepairResult>> repairCachedPackages(SystemCache cache); + Future<Iterable<RepairResult>> repairCachedPackages( + SystemCache cache, { + bool Function(String name, Version version)? packageFilter, + }); /// Return all directories inside this source that can be removed while /// preserving the packages given by [alivePackages] a list of package root
diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart index 9b7af47..0cddeac 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart
@@ -532,8 +532,14 @@ /// Resets all cached packages back to the pristine state of the Git /// repository at the revision they are pinned to. + /// + /// If [packageFilter] is provided, only packages whose names are in the set + /// will be repaired. @override - Future<Iterable<RepairResult>> repairCachedPackages(SystemCache cache) async { + Future<Iterable<RepairResult>> repairCachedPackages( + SystemCache cache, { + bool Function(String name, Version version)? packageFilter, + }) async { final rootDir = cache.rootDirForSource(this); if (!dirExists(rootDir)) return []; @@ -550,13 +556,22 @@ final packageDir = p.join(revisionCachePath, relative); try { - return Package.load( + final package = Package.load( packageDir, loadPubspec: Pubspec.loadRootWithSources(cache.sources), ); + if (packageFilter != null && + !packageFilter(package.name, package.version)) { + return null; + } + return package; } catch (error, stackTrace) { - log.error('Failed to load package', error, stackTrace); final name = p.basename(revisionCachePath).split('-').first; + if (packageFilter != null && + !packageFilter(name, Version.none)) { + return null; + } + log.error('Failed to load package', error, stackTrace); result.add( RepairResult(name, Version.none, this, success: false), );
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index c3fded5..6b58d92 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart
@@ -1307,8 +1307,14 @@ /// Re-downloads all packages that have been previously downloaded into the /// system cache from any server. + /// + /// If [packageFilter] is provided, only packages whose names are in the set + /// will be repaired. @override - Future<Iterable<RepairResult>> repairCachedPackages(SystemCache cache) async { + Future<Iterable<RepairResult>> repairCachedPackages( + SystemCache cache, { + bool Function(String name, Version version)? packageFilter, + }) async { final rootDir = cache.rootDirForSource(this); if (!dirExists(rootDir)) return []; @@ -1331,16 +1337,19 @@ final results = <RepairResult>[]; final packages = <Package>[]; for (var entry in listDir(serverDir)) { + final id = _idForBasename(p.basename(entry), url); + if (packageFilter != null && !packageFilter(id.name, id.version)) { + continue; + } + try { - packages.add( - Package.load( - entry, - loadPubspec: Pubspec.loadRootWithSources(cache.sources), - ), + final package = Package.load( + entry, + loadPubspec: Pubspec.loadRootWithSources(cache.sources), ); + packages.add(package); } catch (error, stackTrace) { log.error('Failed to load package', error, stackTrace); - final id = _idForBasename(p.basename(entry), url); results.add( RepairResult(id.name, id.version, this, success: false), );
diff --git a/test/cache/repair/empty_cache_test.dart b/test/cache/repair/empty_cache_test.dart index 29f6843..5bd49c0 100644 --- a/test/cache/repair/empty_cache_test.dart +++ b/test/cache/repair/empty_cache_test.dart
@@ -10,7 +10,7 @@ test('does nothing if the cache is empty', () { // Repair them. return runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], output: 'No packages in cache, so nothing to repair.', ); });
diff --git a/test/cache/repair/git_test.dart b/test/cache/repair/git_test.dart index 0e896be..d3d6256 100644 --- a/test/cache/repair/git_test.dart +++ b/test/cache/repair/git_test.dart
@@ -51,7 +51,7 @@ // Repair them. await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], output: ''' Resetting Git repository for foo 1.0.0... Resetting Git repository for foo 1.0.1... @@ -82,7 +82,7 @@ } await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: allOf([ contains('Failed to load package:'), contains('Could not find a file named "pubspec.yaml" in '), @@ -113,7 +113,7 @@ } await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: allOf([ contains('Failed to load package:'), contains('Error on line 1, column 2 of '), @@ -173,7 +173,7 @@ // Repair them. await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], output: ''' Resetting Git repository for sub 1.0.0... Resetting Git repository for sub 1.0.1... @@ -206,7 +206,7 @@ } await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: allOf([ contains('Failed to load package:'), contains('Could not find a file named "pubspec.yaml" in '),
diff --git a/test/cache/repair/handles_corrupted_binstub_test.dart b/test/cache/repair/handles_corrupted_binstub_test.dart index 11f4d7e..20989be 100644 --- a/test/cache/repair/handles_corrupted_binstub_test.dart +++ b/test/cache/repair/handles_corrupted_binstub_test.dart
@@ -25,7 +25,7 @@ ]).create(); await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: contains('Error reading binstub for "script":'), ); });
diff --git a/test/cache/repair/handles_corrupted_global_lockfile_test.dart b/test/cache/repair/handles_corrupted_global_lockfile_test.dart index fe684c4..1a4dfdd 100644 --- a/test/cache/repair/handles_corrupted_global_lockfile_test.dart +++ b/test/cache/repair/handles_corrupted_global_lockfile_test.dart
@@ -15,7 +15,7 @@ ]).create(); await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: contains('Failed to reactivate foo:'), output: contains( 'Failed to reactivate 1 package:\n'
diff --git a/test/cache/repair/handles_failure_test.dart b/test/cache/repair/handles_failure_test.dart index d00119d..c24cfea 100644 --- a/test/cache/repair/handles_failure_test.dart +++ b/test/cache/repair/handles_failure_test.dart
@@ -37,7 +37,7 @@ ]).create(); // Repair them. - final pub = await startPub(args: ['cache', 'repair']); + final pub = await startPub(args: ['cache', 'repair', '--all']); expect(pub.stderr, emits(startsWith('Failed to repair foo 1.2.4. Error:'))); expect(
diff --git a/test/cache/repair/handles_orphaned_binstub_test.dart b/test/cache/repair/handles_orphaned_binstub_test.dart index 1df3d16..5c9f31f 100644 --- a/test/cache/repair/handles_orphaned_binstub_test.dart +++ b/test/cache/repair/handles_orphaned_binstub_test.dart
@@ -24,7 +24,7 @@ ]).create(); await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: allOf([ contains('Binstubs exist for non-activated packages:'), contains('From foo: foo-script'),
diff --git a/test/cache/repair/hosted.dart b/test/cache/repair/hosted.dart index 8e5781b..abe9927 100644 --- a/test/cache/repair/hosted.dart +++ b/test/cache/repair/hosted.dart
@@ -20,6 +20,157 @@ ..serve('bar', '1.2.4'); }); + test('repairs only packages from pubspec.lock by default', () async { + // Create a project with foo dependency. + await d.appDir(dependencies: {'foo': '1.2.3'}).create(); + await pubGet(); + + // Set up a cache with some broken packages (including bar which is not in + // the project's dependencies). + await d.dir(cachePath, [ + d.dir('hosted', [ + d.dir('localhost%58${globalServer.port}', [ + d.dir('foo-1.2.3', [ + d.libPubspec('foo', '1.2.3'), + d.file('broken.txt'), + ]), + d.dir('bar-1.2.4', [ + d.libPubspec('bar', '1.2.4'), + d.file('broken.txt'), + ]), + ]), + ]), + ]).create(); + + // Repair without --all should only repair foo (from pubspec.lock). + await runPub(args: ['cache', 'repair'], output: 'Reinstalled 1 package.'); + + // foo should be repaired, bar should still have broken.txt. + await d.hostedCache([ + d.dir('foo-1.2.3', [d.nothing('broken.txt')]), + d.dir('bar-1.2.4', [d.file('broken.txt')]), + ]).validate(); + }); + + test('repairs only the specific version from pubspec.lock', () async { + // Create a project with foo 1.2.3 dependency. + await d.appDir(dependencies: {'foo': '1.2.3'}).create(); + await pubGet(); + + // Set up a cache with multiple versions of foo (both broken). + await d.dir(cachePath, [ + d.dir('hosted', [ + d.dir('localhost%58${globalServer.port}', [ + d.dir('foo-1.2.3', [ + d.libPubspec('foo', '1.2.3'), + d.file('broken.txt'), + ]), + d.dir('foo-1.2.5', [ + d.libPubspec('foo', '1.2.5'), + d.file('broken.txt'), + ]), + ]), + ]), + ]).create(); + + // Repair without --all should only repair foo 1.2.3. + await runPub(args: ['cache', 'repair'], output: 'Reinstalled 1 package.'); + + // Only foo 1.2.3 should be repaired, foo 1.2.5 should still be broken. + await d.hostedCache([ + d.dir('foo-1.2.3', [d.nothing('broken.txt')]), + d.dir('foo-1.2.5', [d.file('broken.txt')]), + ]).validate(); + }); + + test('handles missing pubspec.lock', () async { + await d.appDir().create(); + // Don't run pub get, so there's no pubspec.lock + + await runPub( + args: ['cache', 'repair'], + output: contains('No pubspec.lock found'), + ); + }); + + test('does not repair cached packages for path dependencies', () async { + // Create foo as a path dependency. + await d.dir('foo', [d.libPubspec('foo', '1.0.0')]).create(); + await d + .appDir( + dependencies: { + 'foo': {'path': '../foo'}, + }, + ) + .create(); + await pubGet(); + + // Set up a broken cached version of foo (same name, but different source). + await d.dir(cachePath, [ + d.dir('hosted', [ + d.dir('localhost%58${globalServer.port}', [ + d.dir('foo-1.2.3', [ + d.libPubspec('foo', '1.2.3'), + d.file('broken.txt'), + ]), + ]), + ]), + ]).create(); + + // Repair without --all should NOT repair foo-1.2.3 because the project's + // foo dependency is a path dep, not a hosted dep. + await runPub( + args: ['cache', 'repair'], + output: 'No packages from pubspec.lock found in cache.', + ); + + // foo-1.2.3 should still have broken.txt. + await d.hostedCache([ + d.dir('foo-1.2.3', [d.file('broken.txt')]), + ]).validate(); + }); + + test('git dep does not repair same-named hosted package', () async { + // Create foo as a git dependency. + await d.git('foo.git', [ + d.libDir('foo'), + d.libPubspec('foo', '1.0.0'), + ]).create(); + + await d + .appDir( + dependencies: { + 'foo': {'git': '../foo.git'}, + }, + ) + .create(); + await pubGet(); + + // Set up a broken cached hosted version of foo (same name, but hosted). + await d.dir(cachePath, [ + d.dir('hosted', [ + d.dir('localhost%58${globalServer.port}', [ + d.dir('foo-1.2.3', [ + d.libPubspec('foo', '1.2.3'), + d.file('broken.txt'), + ]), + ]), + ]), + ]).create(); + + // Repair without --all should NOT repair hosted foo-1.2.3 because the + // project's foo dependency is a git dep, not a hosted dep. + await runPub( + args: ['cache', 'repair'], + output: contains('Reinstalled 1 package'), + ); + + // hosted foo-1.2.3 should still have broken.txt. + await d.hostedCache([ + d.dir('foo-1.2.3', [d.file('broken.txt')]), + ]).validate(); + }); + test('reinstalls previously cached hosted packages', () async { // Set up a cache with some broken packages. await d.dir(cachePath, [ @@ -43,10 +194,8 @@ // Repair them. await runPub( - args: ['cache', 'repair'], - output: ''' - - Reinstalled 3 packages.''', + args: ['cache', 'repair', '--all'], + output: 'Reinstalled 3 packages.', silent: allOf([ contains('Downloading bar 1.2.4...'), contains('Downloading foo 1.2.3...'), @@ -80,7 +229,7 @@ ]).create(); await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: allOf([ contains('Failed to load package:'), contains('Could not find a file named "pubspec.yaml" in '), @@ -117,7 +266,7 @@ ]).create(); await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], error: allOf([ contains('Failed to load package:'), contains('Error on line 1, column 2 of '),
diff --git a/test/cache/repair/recompiles_snapshots_test.dart b/test/cache/repair/recompiles_snapshots_test.dart index c6ac04b..30396bc 100644 --- a/test/cache/repair/recompiles_snapshots_test.dart +++ b/test/cache/repair/recompiles_snapshots_test.dart
@@ -27,7 +27,7 @@ ]).create(); await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], output: ''' Reinstalled 1 package. Reactivating foo 1.0.0...
diff --git a/test/cache/repair/updates_binstubs_test.dart b/test/cache/repair/updates_binstubs_test.dart index 74fef30..3bc03d7 100644 --- a/test/cache/repair/updates_binstubs_test.dart +++ b/test/cache/repair/updates_binstubs_test.dart
@@ -41,7 +41,7 @@ // Repair them. await runPub( - args: ['cache', 'repair'], + args: ['cache', 'repair', '--all'], output: ''' Reinstalled 1 package. Reactivating foo 1.0.0...
diff --git a/test/get/hosted/warn_about_discontinued_test.dart b/test/get/hosted/warn_about_discontinued_test.dart index 08a7c5a..ce62b22 100644 --- a/test/get/hosted/warn_about_discontinued_test.dart +++ b/test/get/hosted/warn_about_discontinued_test.dart
@@ -78,7 +78,7 @@ Got dependencies!''', ); // Repairing the cache should reset the package listing caches. - await runPub(args: ['cache', 'repair']); + await runPub(args: ['cache', 'repair', '--all']); await pubGet( output: ''' Resolving dependencies... @@ -179,7 +179,7 @@ Got dependencies!''', ); // Repairing the cache should reset the package listing caches. - await runPub(args: ['cache', 'repair']); + await runPub(args: ['cache', 'repair', '--all']); await pubGet( output: ''' Resolving dependencies...
diff --git a/test/testdata/goldens/help_test/pub cache repair --help.txt b/test/testdata/goldens/help_test/pub cache repair --help.txt index ac09115..efa6800 100644 --- a/test/testdata/goldens/help_test/pub cache repair --help.txt +++ b/test/testdata/goldens/help_test/pub cache repair --help.txt
@@ -6,6 +6,7 @@ Usage: pub cache repair <subcommand> [arguments...] -h, --help Print this usage information. + --all Repair all cached packages instead of only packages in the current pubspec.lock. Run "pub help" to see global options. See https://dart.dev/tools/pub/cmd/pub-cache for detailed documentation.