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(
