Avoid raceconditions in `global activate`, `run` and `global run` (#3285)
diff --git a/analysis_options.yaml b/analysis_options.yaml index f07a060..6177eba 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml
@@ -31,6 +31,7 @@ - sort_pub_dependencies - test_types_in_equals - throw_in_finally + - unawaited_futures - unnecessary_lambdas - unnecessary_null_aware_assignments - unnecessary_parenthesis
diff --git a/lib/src/command/add.dart b/lib/src/command/add.dart index d540978..328d935 100644 --- a/lib/src/command/add.dart +++ b/lib/src/command/add.dart
@@ -162,9 +162,8 @@ /// to this new dependency. final newRoot = Package.inMemory(updatedPubSpec); - // TODO(jonasfj): Stop abusing Entrypoint.global for dry-run output - await Entrypoint.global(newRoot, entrypoint.lockFile, cache, - solveResult: solveResult) + await Entrypoint.inMemory(newRoot, cache, + solveResult: solveResult, lockFile: entrypoint.lockFile) .acquireDependencies(SolveType.get, dryRun: true, precompile: argResults['precompile'],
diff --git a/lib/src/command/remove.dart b/lib/src/command/remove.dart index af91d8c..f3b27bd 100644 --- a/lib/src/command/remove.dart +++ b/lib/src/command/remove.dart
@@ -65,7 +65,7 @@ final newPubspec = _removePackagesFromPubspec(rootPubspec, packages); final newRoot = Package.inMemory(newPubspec); - await Entrypoint.global(newRoot, entrypoint.lockFile, cache) + await Entrypoint.inMemory(newRoot, cache, lockFile: entrypoint.lockFile) .acquireDependencies(SolveType.get, precompile: argResults['precompile'], dryRun: true,
diff --git a/lib/src/command/upgrade.dart b/lib/src/command/upgrade.dart index 34e061d..3063770 100644 --- a/lib/src/command/upgrade.dart +++ b/lib/src/command/upgrade.dart
@@ -222,11 +222,10 @@ 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( + await Entrypoint.inMemory( Package.inMemory(resolvablePubspec), - entrypoint.lockFile, cache, + lockFile: entrypoint.lockFile, solveResult: solveResult, ).acquireDependencies( SolveType.upgrade, @@ -317,10 +316,10 @@ // 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( + await Entrypoint.inMemory( Package.inMemory(nullsafetyPubspec), - entrypoint.lockFile, cache, + lockFile: entrypoint.lockFile, solveResult: solveResult, ).acquireDependencies( SolveType.upgrade,
diff --git a/lib/src/dart.dart b/lib/src/dart.dart index 97219dc..f4f8686 100644 --- a/lib/src/dart.dart +++ b/lib/src/dart.dart
@@ -146,11 +146,14 @@ String toString() => errors.join('\n'); } -/// Precompiles the Dart executable at [executablePath] to a kernel file at -/// [outputPath]. +/// Precompiles the Dart executable at [executablePath]. /// -/// This file is also cached at [incrementalDillOutputPath] which is used to -/// initialize the compiler on future runs. +/// If the compilation succeeds it is saved to a kernel file at [outputPath]. +/// +/// If compilation fails, the output is cached at [incrementalDillOutputPath]. +/// +/// Whichever of [incrementalDillOutputPath] and [outputPath] already exists is +/// used to initialize the compiler run. /// /// The [packageConfigPath] should point at the package config file to be used /// for `package:` uri resolution. @@ -158,39 +161,65 @@ /// The [name] is used to describe the executable in logs and error messages. Future<void> precompile({ required String executablePath, - required String incrementalDillOutputPath, + required String incrementalDillPath, required String name, required String outputPath, required String packageConfigPath, }) async { ensureDir(p.dirname(outputPath)); - ensureDir(p.dirname(incrementalDillOutputPath)); + ensureDir(p.dirname(incrementalDillPath)); + const platformDill = 'lib/_internal/vm_platform_strong.dill'; final sdkRoot = p.relative(p.dirname(p.dirname(Platform.resolvedExecutable))); - var client = await FrontendServerClient.start( - executablePath, - incrementalDillOutputPath, - platformDill, - sdkRoot: sdkRoot, - packagesJson: packageConfigPath, - printIncrementalDependencies: false, - ); + String? tempDir; + FrontendServerClient? client; try { - var result = await client.compile(); + tempDir = createTempDir(p.dirname(incrementalDillPath), 'tmp'); + // To avoid potential races we copy the incremental data to a temporary file + // for just this compilation. + final temporaryIncrementalDill = + p.join(tempDir, '${p.basename(incrementalDillPath)}.incremental.dill'); + try { + if (fileExists(incrementalDillPath)) { + copyFile(incrementalDillPath, temporaryIncrementalDill); + } else if (fileExists(outputPath)) { + copyFile(outputPath, temporaryIncrementalDill); + } + } on FileSystemException { + // Not able to copy existing file, compilation will start from scratch. + } + + client = await FrontendServerClient.start( + executablePath, + temporaryIncrementalDill, + platformDill, + sdkRoot: sdkRoot, + packagesJson: packageConfigPath, + printIncrementalDependencies: false, + ); + final result = await client.compile(); final highlightedName = log.bold(name); if (result?.errorCount == 0) { log.message('Built $highlightedName.'); - await File(incrementalDillOutputPath).copy(outputPath); + // By using rename we ensure atomicity. An external observer will either + // see the old or the new snapshot. + renameFile(temporaryIncrementalDill, outputPath); } else { - // Don't leave partial results. - deleteEntry(outputPath); + // By using rename we ensure atomicity. An external observer will either + // see the old or the new snapshot. + renameFile(temporaryIncrementalDill, incrementalDillPath); + // If compilation failed we don't want to leave an incorrect snapshot. + tryDeleteEntry(outputPath); throw ApplicationException( log.yellow('Failed to build $highlightedName:\n') + (result?.compilerOutputLines.join('\n') ?? '')); } } finally { - client.kill(); + client?.kill(); + if (tempDir != null) { + tryDeleteEntry(tempDir); + } } }
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index b54ec5e..26a8243 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart
@@ -16,7 +16,6 @@ import 'dart.dart' as dart; import 'exceptions.dart'; import 'executable.dart'; -import 'http.dart' as http; import 'io.dart'; import 'language_version.dart'; import 'lock_file.dart'; @@ -73,8 +72,14 @@ /// but may be the entrypoint when you're running its tests. class Entrypoint { /// The root package this entrypoint is associated with. + /// + /// For a global package, this is the activated package. final Package root; + /// For a global package, this is the directory that the package is installed + /// in. Non-global packages have null. + final String? globalDir; + /// The system-wide cache which caches packages that need to be fetched over /// the network. final SystemCache cache; @@ -83,7 +88,8 @@ bool get isCached => !root.isInMemory && p.isWithin(cache.rootDir, root.dir); /// Whether this is an entrypoint for a globally-activated package. - final bool isGlobal; + // final bool isGlobal; + bool get isGlobal => globalDir != null; /// The lockfile for the entrypoint. /// @@ -123,8 +129,7 @@ /// /// Global packages (except those from path source) /// store these in the global cache. - String? get _configRoot => - isCached ? p.join(cache.rootDir, 'global_packages', root.name) : root.dir; + String? get _configRoot => isCached ? globalDir : root.dir; /// The path to the entrypoint's ".packages" file. /// @@ -153,11 +158,7 @@ /// but the configuration is stored at the package itself. String get cachePath { if (isGlobal) { - return p.join( - cache.rootDir, - 'global_packages', - root.name, - ); + return globalDir!; } else { var newPath = root.path('.dart_tool/pub'); var oldPath = root.path('.pub'); @@ -174,15 +175,25 @@ String get _incrementalDillsPath => p.join(cachePath, 'incremental'); /// Loads the entrypoint from a package at [rootDir]. - Entrypoint(String rootDir, this.cache) - : root = Package.load(null, rootDir, cache.sources), - isGlobal = false; + Entrypoint( + String rootDir, + this.cache, + ) : root = Package.load(null, rootDir, cache.sources), + globalDir = null; + + Entrypoint.inMemory(this.root, this.cache, + {required LockFile? lockFile, SolveResult? solveResult}) + : _lockFile = lockFile, + globalDir = null { + if (solveResult != null) { + _packageGraph = PackageGraph.fromSolveResult(this, solveResult); + } + } /// Creates an entrypoint given package and lockfile objects. /// If a SolveResult is already created it can be passed as an optimization. - Entrypoint.global(this.root, this._lockFile, this.cache, - {SolveResult? solveResult}) - : isGlobal = true { + Entrypoint.global(this.globalDir, this.root, this._lockFile, this.cache, + {SolveResult? solveResult}) { if (solveResult != null) { _packageGraph = PackageGraph.fromSolveResult(this, solveResult); } @@ -203,18 +214,20 @@ /// Writes .packages and .dart_tool/package_config.json Future<void> writePackagesFiles() async { + final entrypointName = isGlobal ? null : root.name; writeTextFile( packagesFile, lockFile.packagesFile(cache, - entrypoint: root.name, relativeFrom: root.dir)); + entrypoint: entrypointName, + relativeFrom: isGlobal ? null : root.dir)); ensureDir(p.dirname(packageConfigFile)); writeTextFile( packageConfigFile, await lockFile.packageConfigFile(cache, - entrypoint: root.name, + entrypoint: entrypointName, entrypointSdkConstraint: root.pubspec.sdkConstraints[sdk.identifier], - relativeFrom: root.dir)); + relativeFrom: isGlobal ? null : root.dir)); } /// Gets all dependencies of the [root] package. @@ -294,7 +307,7 @@ await result.showReport(type, cache); } if (!dryRun) { - await Future.wait(result.packages.map(_get)); + await result.downloadCachedPackages(cache); _saveLockFile(result); } if (onlyReportSuccessOrFailure) { @@ -387,10 +400,11 @@ Future<void> _precompileExecutable(Executable executable) async { final package = executable.package; + await dart.precompile( executablePath: resolveExecutable(executable), outputPath: pathOfExecutable(executable), - incrementalDillOutputPath: incrementalDillPathOfExecutable(executable), + incrementalDillPath: incrementalDillPathOfExecutable(executable), packageConfigPath: packageConfigFile, name: '$package:${p.basenameWithoutExtension(executable.relativePath)}'); @@ -470,21 +484,6 @@ } } - /// Makes sure the package at [id] is locally available. - /// - /// This automatically downloads the package to the system-wide cache as well - /// if it requires network access to retrieve (specifically, if the package's - /// source is a [CachedSource]). - Future<void> _get(PackageId id) async { - return await http.withDependencyType(root.dependencyType(id.name), - () async { - if (id.isRoot) return; - - var source = cache.source(id.source); - if (source is CachedSource) await source.downloadToSystemCache(id); - }); - } - /// Throws a [DataError] if the `.dart_tool/package_config.json` file doesn't /// exist or if it's out-of-date relative to the lockfile or the pubspec. ///
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index a868b4b..773f2ec 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart
@@ -12,7 +12,6 @@ import 'entrypoint.dart'; import 'exceptions.dart'; import 'executable.dart' as exec; -import 'http.dart' as http; import 'io.dart'; import 'lock_file.dart'; import 'log.dart' as log; @@ -62,6 +61,8 @@ /// The directory where the lockfiles for activated packages are stored. String get _directory => p.join(cache.rootDir, 'global_packages'); + String _packageDir(String packageName) => p.join(_directory, packageName); + /// The directory where binstubs for global package executables are stored. String get _binStubDir => p.join(cache.rootDir, 'bin'); @@ -151,26 +152,19 @@ // Get the package's dependencies. await entrypoint.acquireDependencies(SolveType.get, analytics: analytics); var name = entrypoint.root.name; - - try { - var originalLockFile = - LockFile.load(_getLockFilePath(name), cache.sources); - // Call this just to log what the current active package is, if any. - _describeActive(originalLockFile, name); - } on IOException { - // Couldn't read the lock file. It probably doesn't exist. - } + _describeActive(name, cache); // Write a lockfile that points to the local package. var fullPath = canonicalize(entrypoint.root.dir); var id = cache.path.source.idFor(name, entrypoint.root.version, fullPath); + final tempDir = cache.createTempDir(); // TODO(rnystrom): Look in "bin" and display list of binaries that // user can run. - _writeLockFile(name, LockFile([id])); + _writeLockFile(tempDir, LockFile([id])); - var binDir = p.join(_directory, name, 'bin'); - if (dirExists(binDir)) deleteEntry(binDir); + tryDeleteEntry(_packageDir(name)); + tryRenameDir(tempDir, _packageDir(name)); _updateBinStubs(entrypoint, entrypoint.root, executables, overwriteBinStubs: overwriteBinStubs); @@ -178,17 +172,12 @@ } /// Installs the package [dep] and its dependencies into the system cache. + /// + /// If [silent] less logging will be printed. Future<void> _installInCache(PackageRange dep, List<String>? executables, - {required bool overwriteBinStubs}) async { - LockFile? originalLockFile; - try { - originalLockFile = - LockFile.load(_getLockFilePath(dep.name), cache.sources); - // Call this just to log what the current active package is, if any. - _describeActive(originalLockFile, dep.name); - } on IOException { - // Couldn't read the lock file. It probably doesn't exist. - } + {required bool overwriteBinStubs, bool silent = false}) async { + final name = dep.name; + LockFile? originalLockFile = _describeActive(name, cache); // Create a dummy package with just [dep] so we can do resolution on it. var root = Package.inMemory(Pubspec('pub global activate', @@ -200,103 +189,89 @@ // being available, report that as a [dataError]. SolveResult result; try { - result = await log.progress('Resolving dependencies', - () => resolveVersions(SolveType.get, cache, root)); + result = await log.spinner( + 'Resolving dependencies', + () => resolveVersions(SolveType.get, cache, root), + condition: !silent, + ); } on SolveFailure catch (error) { for (var incompatibility in error.incompatibility.externalIncompatibilities) { if (incompatibility.cause != IncompatibilityCause.noVersions) continue; - if (incompatibility.terms.single.package.name != dep.name) continue; + if (incompatibility.terms.single.package.name != name) continue; dataError(error.toString()); } rethrow; } + // We want the entrypoint to be rooted at 'dep' not the dummy-package. + result.packages.removeWhere((id) => id.name == 'pub global activate'); final sameVersions = originalLockFile != null && originalLockFile.samePackageIds(result.lockFile); + final PackageId id = result.lockFile.packages[name]!; if (sameVersions) { log.message(''' -The package ${dep.name} is already activated at newest available version. -To recompile executables, first run `$topLevelProgram pub global deactivate ${dep.name}`. +The package $name is already activated at newest available version. +To recompile executables, first run `$topLevelProgram pub global deactivate $name`. '''); } else { - await result.showReport(SolveType.get, cache); + // Only precompile binaries if we have a new resolution. + if (!silent) await result.showReport(SolveType.get, cache); + + await result.downloadCachedPackages(cache); + + final lockFile = result.lockFile; + final tempDir = cache.createTempDir(); + _writeLockFile(tempDir, lockFile); + + // Load the package graph from [result] so we don't need to re-parse all + // the pubspecs. + final entrypoint = Entrypoint.global( + tempDir, + cache.loadCached(id), + lockFile, + cache, + solveResult: result, + ); + + await entrypoint.writePackagesFiles(); + + await entrypoint.precompileExecutables(); + + tryDeleteEntry(_packageDir(name)); + tryRenameDir(tempDir, _packageDir(name)); } - - // Make sure all of the dependencies are locally installed. - await Future.wait(result.packages.map((id) { - return http.withDependencyType(root.dependencyType(id.name), () async { - if (id.isRoot) return; - - var source = cache.source(id.source); - if (source is CachedSource) await source.downloadToSystemCache(id); - }); - })); - - var lockFile = result.lockFile; - _writeLockFile(dep.name, lockFile); - await _writePackageConfigFiles(dep.name, lockFile); - - // We want the entrypoint to be rooted at 'dep' not the dummy-package. - result.packages.removeWhere((id) => id.name == 'pub global activate'); - - var id = lockFile.packages[dep.name]!; - // Load the package graph from [result] so we don't need to re-parse all - // the pubspecs. final entrypoint = Entrypoint.global( - Package( - result.pubspecs[dep.name]!, - (cache.source(dep.source) as CachedSource).getDirectoryInCache(id), - ), - lockFile, + _packageDir(id.name), + cache.loadCached(id), + result.lockFile, cache, solveResult: result, ); - if (!sameVersions) { - // Only precompile binaries if we have a new resolution. - await entrypoint.precompileExecutables(); - } - _updateBinStubs( entrypoint, cache.load(entrypoint.lockFile.packages[dep.name]!), executables, overwriteBinStubs: overwriteBinStubs, ); - - log.message('Activated ${_formatPackage(id)}.'); - } - - Future<void> _writePackageConfigFiles( - String package, LockFile lockFile) async { - // TODO(sigurdm): Use [Entrypoint.writePackagesFiles] instead. - final packagesFilePath = _getPackagesFilePath(package); - final packageConfigFilePath = _getPackageConfigFilePath(package); - final dir = p.dirname(packagesFilePath); - writeTextFile( - packagesFilePath, lockFile.packagesFile(cache, relativeFrom: dir)); - ensureDir(p.dirname(packageConfigFilePath)); - writeTextFile(packageConfigFilePath, - await lockFile.packageConfigFile(cache, relativeFrom: dir)); + if (!silent) log.message('Activated ${_formatPackage(id)}.'); } /// Finishes activating package [package] by saving [lockFile] in the cache. - void _writeLockFile(String package, LockFile lockFile) { - ensureDir(p.join(_directory, package)); - - // TODO(nweiz): This cleans up Dart 1.6's old lockfile location. Remove it - // when Dart 1.6 is old enough that we don't think anyone will have these - // lockfiles anymore (issue 20703). - var oldPath = p.join(_directory, '$package.lock'); - if (fileExists(oldPath)) deleteEntry(oldPath); - - writeTextFile(_getLockFilePath(package), - lockFile.serialize(p.join(_directory, package))); + void _writeLockFile(String dir, LockFile lockFile) { + writeTextFile(p.join(dir, 'pubspec.lock'), lockFile.serialize(null)); } /// Shows the user the currently active package with [name], if any. - void _describeActive(LockFile lockFile, String? name) { + LockFile? _describeActive(String name, SystemCache cache) { + late final LockFile lockFile; + try { + lockFile = LockFile.load(_getLockFilePath(name), cache.sources); + } on IOException { + // Couldn't read the lock file. It probably doesn't exist. + return null; + } var id = lockFile.packages[name]!; var source = id.source; @@ -312,6 +287,7 @@ log.message('Package ${log.bold(name)} is currently active at version ' '${log.bold(id.version)}.'); } + return lockFile; } /// Deactivates a previously-activated package named [name]. @@ -341,22 +317,8 @@ try { lockFile = LockFile.load(lockFilePath, cache.sources); } on IOException { - var oldLockFilePath = p.join(_directory, '$name.lock'); - try { - // TODO(nweiz): This looks for Dart 1.6's old lockfile location. - // Remove it when Dart 1.6 is old enough that we don't think anyone - // will have these lockfiles anymore (issue 20703). - lockFile = LockFile.load(oldLockFilePath, cache.sources); - } on IOException { - // If we couldn't read the lock file, it's not activated. - dataError('No active package ${log.bold(name)}.'); - } - - // Move the old lockfile to its new location. - ensureDir(p.dirname(lockFilePath)); - File(oldLockFilePath).renameSync(lockFilePath); - // Just make sure these files are created as well. - await _writePackageConfigFiles(name, lockFile); + // If we couldn't read the lock file, it's not activated. + dataError('No active package ${log.bold(name)}.'); } // Remove the package itself from the lockfile. We put it in there so we @@ -370,7 +332,8 @@ if (source is CachedSource) { // For cached sources, the package itself is in the cache and the // lockfile is the one we just loaded. - entrypoint = Entrypoint.global(cache.loadCached(id), lockFile, cache); + entrypoint = Entrypoint.global( + _packageDir(id.name), cache.loadCached(id), lockFile, cache); } else { // For uncached sources (i.e. path), the ID just points to the real // directory for the package. @@ -446,16 +409,6 @@ String _getLockFilePath(String name) => p.join(_directory, name, 'pubspec.lock'); - /// Gets the path to the .packages file for an activated cached package with - /// [name]. - String _getPackagesFilePath(String name) => - p.join(_directory, name, '.packages'); - - /// Gets the path to the `package_config.json` file for an - /// activated cached package with [name]. - String _getPackageConfigFilePath(String name) => - p.join(_directory, name, '.dart_tool', 'package_config.json'); - /// Shows the user a formatted list of globally activated packages. void listActivePackages() { if (!dirExists(_directory)) return; @@ -542,17 +495,24 @@ log.message('Reactivating ${log.bold(id.name)} ${id.version}...'); var entrypoint = await find(id.name); + final packageExecutables = executables.remove(id.name) ?? []; - await _writePackageConfigFiles(id.name, entrypoint.lockFile); - await entrypoint.precompileExecutables(); - var packageExecutables = executables.remove(id.name) ?? []; - _updateBinStubs( - entrypoint, - cache.load(id), - packageExecutables, - overwriteBinStubs: true, - suggestIfNotOnPath: false, - ); + if (entrypoint.isCached) { + deleteEntry(entrypoint.globalDir!); + await _installInCache( + id.toRange(), + packageExecutables, + overwriteBinStubs: true, + silent: true, + ); + } else { + await activatePath( + entrypoint.root.dir, + packageExecutables, + overwriteBinStubs: true, + analytics: null, + ); + } successes.add(id.name); } catch (error, stackTrace) { var message = 'Failed to reactivate ' @@ -706,10 +666,7 @@ // Show errors for any missing scripts. // TODO(rnystrom): This can print false positives since a script may be // produced by a transformer. Do something better. - var binFiles = package - .listFiles(beneath: 'bin', recursive: false) - .map(package.relative) - .toList(); + var binFiles = package.executablePaths; for (var executable in installed) { var script = package.pubspec.executables[executable]; var scriptPath = p.join('bin', '$script.dart'); @@ -761,6 +718,7 @@ // If the script was built to a snapshot, just try to invoke that // directly and skip pub global run entirely. String invocation; + late String binstub; if (Platform.isWindows) { if (fileExists(snapshot)) { // We expect absolute paths from the precompiler since relative ones @@ -786,7 +744,7 @@ } else { invocation = 'dart pub global run ${package.name}:$script %*'; } - var batch = ''' + binstub = ''' @echo off rem This file was created by pub v${sdk.version}. rem Package: ${package.name} @@ -795,7 +753,6 @@ rem Script: $script $invocation '''; - writeTextFile(binStubPath, batch); } else { if (fileExists(snapshot)) { // We expect absolute paths from the precompiler since relative ones @@ -818,7 +775,7 @@ } else { invocation = 'dart pub global run ${package.name}:$script "\$@"'; } - var bash = ''' + binstub = ''' #!/usr/bin/env sh # This file was created by pub v${sdk.version}. # Package: ${package.name} @@ -827,25 +784,31 @@ # Script: $script $invocation '''; + } - // Write this as the system encoding since the system is going to execute - // it and it might contain non-ASCII characters in the pathnames. - writeTextFile(binStubPath, bash, encoding: const SystemEncoding()); + // Write the binstub to a temporary location, make it executable and move + // it into place afterwards to avoid races. + final tempDir = cache.createTempDir(); + try { + final tmpPath = p.join(tempDir, binStubPath); - // Make it executable. - var result = Process.runSync('chmod', ['+x', binStubPath]); - if (result.exitCode != 0) { - // Couldn't make it executable so don't leave it laying around. - try { - deleteEntry(binStubPath); - } on IOException catch (err) { - // Do nothing. We're going to fail below anyway. - log.fine('Could not delete binstub:\n$err'); + // Write this as the system encoding since the system is going to + // execute it and it might contain non-ASCII characters in the + // pathnames. + writeTextFile(tmpPath, binstub, encoding: const SystemEncoding()); + + if (Platform.isLinux || Platform.isMacOS) { + // Make it executable. + var result = Process.runSync('chmod', ['+x', tmpPath]); + if (result.exitCode != 0) { + // Couldn't make it executable so don't leave it laying around. + fail('Could not make "$tmpPath" executable (exit code ' + '${result.exitCode}):\n${result.stderr}'); } - - fail('Could not make "$binStubPath" executable (exit code ' - '${result.exitCode}):\n${result.stderr}'); } + File(tmpPath).renameSync(binStubPath); + } finally { + deleteEntry(tempDir); } return previousPackage;
diff --git a/lib/src/io.dart b/lib/src/io.dart index 6d0bf83..280dcbd 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart
@@ -383,7 +383,7 @@ } // ERROR_DIR_NOT_EMPTY - if (!ignoreEmptyDir && isDirectoryNotEmptyException(error)) { + if (!ignoreEmptyDir && _isDirectoryNotEmptyException(error)) { return 'of dart-lang/sdk#25353'; } @@ -457,7 +457,35 @@ }, ignoreEmptyDir: true); } -bool isDirectoryNotEmptyException(FileSystemException e) { +/// Renames directory [from] to [to]. +/// If it fails with "destination not empty" we log and continue, assuming +/// another process got there before us. +void tryRenameDir(String from, String to) { + ensureDir(path.dirname(to)); + try { + renameDir(from, to); + } on FileSystemException catch (e) { + tryDeleteEntry(from); + if (!_isDirectoryNotEmptyException(e)) { + rethrow; + } + log.fine(''' +Destination directory $to already existed. +Assuming a concurrent pub invocation installed it.'''); + } +} + +void copyFile(String from, String to) { + log.io('Copying "$from" to "$to".'); + File(from).copySync(to); +} + +void renameFile(String from, String to) { + log.io('Renaming "$from" to "$to".'); + File(from).renameSync(to); +} + +bool _isDirectoryNotEmptyException(FileSystemException e) { final errorCode = e.osError?.errorCode; return // On Linux rename will fail with ENOTEMPTY if directory exists:
diff --git a/lib/src/lock_file.dart b/lib/src/lock_file.dart index e2723c7..8f563ab 100644 --- a/lib/src/lock_file.dart +++ b/lib/src/lock_file.dart
@@ -218,7 +218,7 @@ String packagesFile( SystemCache cache, { String? entrypoint, - required String relativeFrom, + String? relativeFrom, }) { var header = ''' This file is deprecated. Tools should instead consume @@ -256,7 +256,7 @@ SystemCache cache, { String? entrypoint, VersionConstraint? entrypointSdkConstraint, - required String relativeFrom, + String? relativeFrom, }) async { final entries = <PackageConfigEntry>[]; for (final name in ordered(packages.keys)) { @@ -306,8 +306,9 @@ /// Returns the serialized YAML text of the lock file. /// /// [packageDir] is the containing directory of the root package, used to - /// properly serialize package descriptions. - String serialize(String packageDir) { + /// serialize relative path package descriptions. If it is null, they will be + /// serialized as absolute. + String serialize(String? packageDir) { // Convert the dependencies to a simple object. var packageMap = {}; packages.forEach((name, package) {
diff --git a/lib/src/package.dart b/lib/src/package.dart index 0bc6f89..d100157 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart
@@ -83,10 +83,12 @@ ..addAll(dependencyOverrides); } - /// Returns a list of asset ids for all Dart executables in this package's bin + /// Returns a list of paths to all Dart executables in this package's bin /// directory. List<String> get executablePaths { - return ordered(listFiles(beneath: 'bin', recursive: false)) + final binDir = p.join(dir, 'bin'); + if (!dirExists(binDir)) return <String>[]; + return ordered(listDir(p.join(dir, 'bin'), includeDirs: false)) .where((executable) => p.extension(executable) == '.dart') .map((executable) => p.relative(executable, from: dir)) .toList();
diff --git a/lib/src/solver/result.dart b/lib/src/solver/result.dart index b8ff0e5..bc01162 100644 --- a/lib/src/solver/result.dart +++ b/lib/src/solver/result.dart
@@ -5,6 +5,7 @@ import 'package:collection/collection.dart'; import 'package:pub_semver/pub_semver.dart'; +import '../http.dart'; import '../io.dart'; import '../lock_file.dart'; import '../log.dart' as log; @@ -12,6 +13,7 @@ import '../package_name.dart'; import '../pub_embeddable_command.dart'; import '../pubspec.dart'; +import '../source/cached.dart'; import '../source/hosted.dart'; import '../source_registry.dart'; import '../system_cache.dart'; @@ -78,6 +80,18 @@ final LockFile _previousLockFile; + /// Downloads all cached packages in [packages]. + Future<void> downloadCachedPackages(SystemCache cache) async { + await Future.wait(packages.map((id) async { + if (id.source == null) return; + final source = cache.source(id.source); + if (source is! CachedSource) return; + return await withDependencyType(_root.dependencyType(id.name), () async { + await source.downloadToSystemCache(id); + }); + })); + } + /// Returns the names of all packages that were changed. /// /// This includes packages that were added or removed.
diff --git a/lib/src/source.dart b/lib/src/source.dart index add55fa..ecbc5eb 100644 --- a/lib/src/source.dart +++ b/lib/src/source.dart
@@ -116,7 +116,7 @@ /// [description] in the right format. /// /// [containingPath] is the containing directory of the root package. - dynamic serializeDescription(String containingPath, description) { + dynamic serializeDescription(String? containingPath, description) { return description; }
diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart index 0d98ae9..3be752c 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart
@@ -108,10 +108,10 @@ /// For the descriptions where `relative` attribute is `true`, tries to make /// `url` relative to the specified [containingPath]. @override - dynamic serializeDescription(String containingPath, description) { + dynamic serializeDescription(String? containingPath, description) { final copy = Map.from(description); copy.remove('relative'); - if (description['relative'] == true) { + if (description['relative'] == true && containingPath != null) { copy['url'] = p.url.relative(description['url'], from: Uri.file(containingPath).toString()); }
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index b3fa729..2cfc52a 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart
@@ -159,7 +159,7 @@ } @override - dynamic serializeDescription(String containingPath, description) { + dynamic serializeDescription(String? containingPath, description) { final desc = _asDescription(description); return _serializedDescriptionFor(desc.packageName, desc.uri); } @@ -803,17 +803,7 @@ // If this fails with a "directory not empty" exception we assume that // another pub process has installed the same package version while we // downloaded. - try { - renameDir(tempDir, destPath); - } on io.FileSystemException catch (e) { - tryDeleteEntry(tempDir); - if (!isDirectoryNotEmptyException(e)) { - rethrow; - } - log.fine(''' -Destination directory $destPath already existed. -Assuming a concurrent pub invocation installed it.'''); - } + tryRenameDir(tempDir, destPath); }); }
diff --git a/lib/src/source/path.dart b/lib/src/source/path.dart index 0401ebd..4679f98 100644 --- a/lib/src/source/path.dart +++ b/lib/src/source/path.dart
@@ -130,9 +130,11 @@ /// /// For the descriptions where `relative` attribute is `true`, tries to make /// `path` relative to the specified [containingPath]. + /// + /// If [containingPath] is `null` they are serialized as absolute. @override - dynamic serializeDescription(String containingPath, description) { - if (description['relative']) { + dynamic serializeDescription(String? containingPath, description) { + if (description['relative'] == true && containingPath != null) { return { 'path': relativePathWithPosixSeparators( p.relative(description['path'], from: containingPath)),
diff --git a/test/global/activate/activate_hosted_after_git_test.dart b/test/global/activate/activate_hosted_after_git_test.dart index 58c65fb..1546280 100644 --- a/test/global/activate/activate_hosted_after_git_test.dart +++ b/test/global/activate/activate_hosted_after_git_test.dart
@@ -2,6 +2,7 @@ // 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:path/path.dart' as p; import 'package:test/test.dart'; import '../../descriptor.dart' as d; @@ -21,8 +22,9 @@ await runPub(args: ['global', 'activate', '-sgit', '../foo.git']); + final locationUri = p.toUri(p.join(d.sandbox, 'foo.git')); await runPub(args: ['global', 'activate', 'foo'], output: ''' - Package foo is currently active from Git repository "../foo.git". + Package foo is currently active from Git repository "$locationUri". Resolving dependencies... + foo 2.0.0 Downloading foo 2.0.0...
diff --git a/test/global/activate/activate_hosted_twice_test.dart b/test/global/activate/activate_hosted_twice_test.dart index 9baed5b..a4d1337 100644 --- a/test/global/activate/activate_hosted_twice_test.dart +++ b/test/global/activate/activate_hosted_twice_test.dart
@@ -24,15 +24,7 @@ d.dir('lib', [d.file('bar.dart', 'final version = "1.0.0";')]) ]); - await runPub(args: ['global', 'activate', 'foo'], output: ''' -Resolving dependencies... -+ bar 1.0.0 -+ foo 1.0.0 -Downloading foo 1.0.0... -Downloading bar 1.0.0... -Building package executables... -Built foo:foo. -Activated foo 1.0.0.'''); + await runPub(args: ['global', 'activate', 'foo'], output: anything); await runPub(args: ['global', 'activate', 'foo'], output: ''' Package foo is currently active at version 1.0.0.
diff --git a/test/global/activate/activate_path_after_hosted_test.dart b/test/global/activate/activate_path_after_hosted_test.dart index cfc28fd..01d493d 100644 --- a/test/global/activate/activate_path_after_hosted_test.dart +++ b/test/global/activate/activate_path_after_hosted_test.dart
@@ -10,7 +10,7 @@ import '../../test_pub.dart'; void main() { - test('activating a hosted package deactivates the path one', () async { + test('activating a path package deactivates the hosted one', () async { final server = await servePackages(); server.serve('foo', '1.0.0', contents: [ d.dir('bin', [d.file('foo.dart', "main(args) => print('hosted');")])
diff --git a/test/global/activate/reactivating_git_upgrades_test.dart b/test/global/activate/reactivating_git_upgrades_test.dart index 100aaea..68102da 100644 --- a/test/global/activate/reactivating_git_upgrades_test.dart +++ b/test/global/activate/reactivating_git_upgrades_test.dart
@@ -2,6 +2,7 @@ // 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:path/path.dart' as p; import 'package:test/test.dart'; import '../../descriptor.dart' as d; @@ -29,11 +30,12 @@ await d.git('foo.git', [d.libPubspec('foo', '1.0.1')]).commit(); // Activating it again pulls down the latest commit. + final locationUri = p.toUri(p.join(d.sandbox, 'foo.git')); await runPub( args: ['global', 'activate', '-sgit', '../foo.git'], output: allOf( startsWith('Package foo is currently active from Git repository ' - '"../foo.git".\n' + '"$locationUri".\n' 'Resolving dependencies...\n' '+ foo 1.0.1 from git ../foo.git at '), // Specific revision number goes here.
diff --git a/test/global/activate/removes_old_lockfile_test.dart b/test/global/activate/removes_old_lockfile_test.dart deleted file mode 100644 index 3392aff..0000000 --- a/test/global/activate/removes_old_lockfile_test.dart +++ /dev/null
@@ -1,33 +0,0 @@ -// Copyright (c) 2014, 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() { - test('removes the 1.6-style lockfile', () async { - final server = await servePackages(); - server.serve('foo', '1.0.0'); - - await d.dir(cachePath, [ - d.dir('global_packages', [ - d.file( - 'foo.lock', - 'packages: {foo: {description: foo, source: hosted, ' - 'version: "1.0.0"}}}') - ]) - ]).create(); - - await runPub(args: ['global', 'activate', 'foo']); - - await d.dir(cachePath, [ - d.dir('global_packages', [ - d.nothing('foo.lock'), - d.dir('foo', [d.file('pubspec.lock', contains('1.0.0'))]) - ]) - ]).validate(); - }); -}
diff --git a/test/global/deactivate/git_package_test.dart b/test/global/deactivate/git_package_test.dart index f06ce53..7fc2855 100644 --- a/test/global/deactivate/git_package_test.dart +++ b/test/global/deactivate/git_package_test.dart
@@ -2,6 +2,7 @@ // 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:path/path.dart' as p; import 'package:test/test.dart'; import '../../descriptor.dart' as d; @@ -18,9 +19,10 @@ await runPub(args: ['global', 'activate', '-sgit', '../foo.git']); + final locationUri = p.toUri(p.join(d.sandbox, 'foo.git')); await runPub( args: ['global', 'deactivate', 'foo'], output: - 'Deactivated package foo 1.0.0 from Git repository "../foo.git".'); + 'Deactivated package foo 1.0.0 from Git repository "$locationUri".'); }); }
diff --git a/test/global/list_test.dart b/test/global/list_test.dart index 8d4d02e..00ca22c 100644 --- a/test/global/list_test.dart +++ b/test/global/list_test.dart
@@ -30,9 +30,10 @@ await runPub(args: ['global', 'activate', '-sgit', '../foo.git']); + final locationUri = p.toUri(p.join(d.sandbox, 'foo.git')); await runPub( args: ['global', 'list'], - output: 'foo 1.0.0 from Git repository "../foo.git"'); + output: 'foo 1.0.0 from Git repository "$locationUri"'); }); test('lists an activated Path package', () async {
diff --git a/test/global/run/uses_old_lockfile_test.dart b/test/global/run/uses_old_lockfile_test.dart deleted file mode 100644 index 1afab81..0000000 --- a/test/global/run/uses_old_lockfile_test.dart +++ /dev/null
@@ -1,54 +0,0 @@ -// Copyright (c) 2014, 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() { - test('uses the 1.6-style lockfile if necessary', () async { - await servePackages() - ..serve('bar', '1.0.0') - ..serve('foo', '1.0.0', deps: { - 'bar': 'any' - }, contents: [ - d.dir('bin', [ - d.file('script.dart', """ - import 'package:bar/bar.dart' as bar; - - main(args) => print(bar.main());""") - ]) - ]); - - await runPub(args: ['cache', 'add', 'foo']); - await runPub(args: ['cache', 'add', 'bar']); - - await d.dir(cachePath, [ - d.dir('global_packages', [ - d.file('foo.lock', ''' -packages: - foo: - description: foo - source: hosted - version: "1.0.0" - bar: - description: bar - source: hosted - version: "1.0.0"''') - ]) - ]).create(); - - var pub = await pubRun(global: true, args: ['foo:script']); - expect(pub.stdout, emitsThrough('bar 1.0.0')); - await pub.shouldExit(); - - await d.dir(cachePath, [ - d.dir('global_packages', [ - d.nothing('foo.lock'), - d.dir('foo', [d.file('pubspec.lock', contains('1.0.0'))]) - ]) - ]).validate(); - }); -}
diff --git a/tool/test.dart b/tool/test.dart index e7877a2..7d98156 100755 --- a/tool/test.dart +++ b/tool/test.dart
@@ -29,7 +29,7 @@ await precompile( executablePath: path.join('bin', 'pub.dart'), outputPath: pubSnapshotFilename, - incrementalDillOutputPath: pubSnapshotIncrementalFilename, + incrementalDillPath: pubSnapshotIncrementalFilename, name: 'bin/pub.dart', packageConfigPath: path.join('.dart_tool', 'package_config.json')); testProcess = await Process.start(