Consider equal timestamps to be outdated (#4833)
diff --git a/lib/src/command/check_resolution_up_to_date.dart b/lib/src/command/check_resolution_up_to_date.dart
index 66fff46..99d7fb8 100644
--- a/lib/src/command/check_resolution_up_to_date.dart
+++ b/lib/src/command/check_resolution_up_to_date.dart
@@ -34,7 +34,11 @@
@override
Future<void> runProtected() async {
- final result = Entrypoint.isResolutionUpToDate(directory, cache);
+ final result = Entrypoint.isResolutionUpToDate(
+ directory,
+ cache,
+ updateOutOfDateTimestamps: false,
+ );
if (result == null) {
fail('Resolution needs updating. Run `$topLevelProgram pub get`');
} else {
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart
index 0583b29..75adf4e 100644
--- a/lib/src/entrypoint.dart
+++ b/lib/src/entrypoint.dart
@@ -833,9 +833,9 @@
/// pubspec.lock. We do this extra round of checking to accommodate for cases
/// where version control or other processes mess up the timestamp order.
///
- /// If the resolution is still valid, the timestamps are updated and this
- /// returns the package configuration and the root dir. Otherwise this
- /// returns `null`.
+ /// If the resolution is still valid, the timestamps are updated (unless
+ /// [updateOutOfDateTimestamps] is false) and this returns the package
+ /// configuration and the root dir. Otherwise this returns `null`.
///
/// This check is on the fast-path of `dart run` and should do as little
/// work as possible. Specifically we avoid parsing any yaml when the
@@ -852,12 +852,24 @@
/// `.dart_tool/package_config.json` is not checked into version control.
static (PackageConfig, String)? isResolutionUpToDate(
String dir,
- SystemCache cache,
- ) {
+ SystemCache cache, {
+ bool updateOutOfDateTimestamps = true,
+ }) {
late final wasRelative = p.isRelative(dir);
String relativeIfNeeded(String path) =>
wasRelative ? p.relative(path) : path;
+ late final rootPackageDir =
+ parentDirs(dir).firstWhereOrNull(
+ (parent) => tryStatFile(p.join(parent, 'pubspec.yaml')) != null,
+ ) ??
+ dir;
+ late final root = Package.load(
+ rootPackageDir,
+ loadPubspec: Pubspec.loadRootWithSources(cache.sources),
+ );
+ late final Package workspaceRoot;
+
/// Whether the lockfile is out of date with respect to the dependencies'
/// pubspecs.
///
@@ -871,6 +883,12 @@
/// Returns whether the locked version of [dep] matches the dependency.
bool isDependencyUpToDate(PackageRange dep) {
if (dep.name == root.name) return true;
+ // Workspace packages are local source packages and are never listed in
+ // the `packages` section of `pubspec.lock`. They are always considered
+ // up-to-date here.
+ if (workspaceRoot.transitiveWorkspace.any((p) => p.name == dep.name)) {
+ return true;
+ }
final locked = lockFile.packages[dep.name];
return locked != null && dep.allows(locked);
@@ -968,7 +986,9 @@
// they are not supposed to work.
final hasExtraMappings =
!packagePathsMapping.keys.every((packageName) {
- return packageName == root.name ||
+ return workspaceRoot.transitiveWorkspace.any(
+ (p) => p.name == packageName,
+ ) ||
lockFile.packages.containsKey(packageName);
});
if (hasExtraMappings) {
@@ -990,8 +1010,8 @@
}
final source = lockFileId.source;
- final lockFilePackagePath = root.path(
- cache.getDirectory(lockFileId, relativeFrom: root.dir),
+ final lockFilePackagePath = workspaceRoot.path(
+ cache.getDirectory(lockFileId, relativeFrom: workspaceRoot.dir),
);
// Make sure that the packagePath agrees with the lock file about the
@@ -1024,7 +1044,7 @@
);
return false;
}
- packagePathsMapping[pkg.name] = root.path(
+ packagePathsMapping[pkg.name] = workspaceRoot.path(
'.dart_tool',
p.fromUri(pkg.rootUri),
);
@@ -1046,6 +1066,20 @@
// correct. This is important for path dependencies as these can mutate.
for (final pkg in packageConfig.packages) {
if (pkg.name == root.name) continue;
+ final workspacePkg = workspaceRoot.transitiveWorkspace.firstWhereOrNull(
+ (p) => p.name == pkg.name,
+ );
+ if (workspacePkg != null) {
+ if (pkg.languageVersion != workspacePkg.pubspec.languageVersion) {
+ log.fine(
+ '${workspacePkg.pubspecPath} has '
+ 'changed since the $lockFilePath file was generated.',
+ );
+ return false;
+ }
+ continue;
+ }
+
final id = lockFile.packages[pkg.name];
if (id == null) {
assert(
@@ -1175,12 +1209,19 @@
);
return null;
}
- final lockFilePath = p.normalize(p.join(rootDir, 'pubspec.lock'));
final packageConfig = _loadPackageConfig(packageConfigPath);
if (p.isWithin(cache.rootDir, packageConfigPath)) {
// We always consider a global package (inside the cache) up-to-date.
return (packageConfig, rootDir);
}
+ workspaceRoot =
+ rootDir == rootPackageDir
+ ? root
+ : Package.load(
+ rootDir,
+ loadPubspec: Pubspec.loadRootWithSources(cache.sources),
+ );
+ final lockFilePath = p.normalize(p.join(rootDir, 'pubspec.lock'));
/// Whether or not the `.dart_tool/package_config.json` file was
/// generated by a different sdk down to changes in minor versions.
@@ -1236,6 +1277,11 @@
final lockFileModified = lockFileStat.modified;
var lockfileNewerThanPubspecs = true;
+ // Whether any pubspec is strictly newer than the lockfile.
+ // We only touch the lockfile to make it newer if this is true, avoiding
+ // touching it on equal timestamps to prevent cascading invalidation of
+ // .dart_tool/package_config.json and downstream developer tools.
+ var pubspecStrictlyNewer = false;
// Check that all packages in packageConfig exist and their pubspecs have
// not been updated since the lockfile was written.
@@ -1261,10 +1307,13 @@
return null;
}
- if (pubspecStat.modified.isAfter(lockFileModified)) {
+ if (!lockFileModified.isAfter(pubspecStat.modified)) {
log.fine('`$pubspecPath` is newer than `$lockFilePath`');
lockfileNewerThanPubspecs = false;
- break;
+ if (pubspecStat.modified.isAfter(lockFileModified)) {
+ pubspecStrictlyNewer = true;
+ break;
+ }
}
final pubspecOverridesPath = p.join(
package.rootUri.path,
@@ -1275,30 +1324,47 @@
// This will wrongly require you to reresolve if a
// `pubspec_overrides.yaml` in a path-dependency is updated. That
// seems acceptable.
- if (pubspecOverridesStat.modified.isAfter(lockFileModified)) {
+ if (!lockFileModified.isAfter(pubspecOverridesStat.modified)) {
log.fine('`$pubspecOverridesPath` is newer than `$lockFilePath`');
lockfileNewerThanPubspecs = false;
+ if (pubspecOverridesStat.modified.isAfter(lockFileModified)) {
+ pubspecStrictlyNewer = true;
+ break;
+ }
}
}
}
+ if (!updateOutOfDateTimestamps && !lockfileNewerThanPubspecs) {
+ log.fine(
+ 'Timestamps are out of order (updateOutOfDateTimestamps: false)',
+ );
+ return null;
+ }
var touchedLockFile = false;
late final lockFile = _loadLockFile(lockFilePath, cache);
- late final root = Package.load(
- dir,
- loadPubspec: Pubspec.loadRootWithSources(cache.sources),
- );
if (!lockfileNewerThanPubspecs) {
if (isLockFileUpToDate(lockFile, root, lockFilePath: lockFilePath)) {
- touch(lockFilePath);
- touchedLockFile = true;
+ if (pubspecStrictlyNewer) {
+ touch(lockFilePath);
+ touchedLockFile = true;
+ }
} else {
return null;
}
}
+ if (!updateOutOfDateTimestamps &&
+ packageConfigStat.modified.isBefore(lockFileModified)) {
+ log.fine(
+ 'Timestamps are out of order (updateOutOfDateTimestamps: false)',
+ );
+ return null;
+ }
+
if (touchedLockFile ||
- lockFileModified.isAfter(packageConfigStat.modified)) {
+ !lockfileNewerThanPubspecs ||
+ packageConfigStat.modified.isBefore(lockFileModified)) {
log.fine('`$lockFilePath` is newer than `$packageConfigPath`');
if (isPackageConfigUpToDate(
packageConfig,
@@ -1307,7 +1373,10 @@
packageConfigPath: packageConfigPath,
lockFilePath: lockFilePath,
)) {
- touch(packageConfigPath);
+ if (touchedLockFile ||
+ lockFileModified.isAfter(packageConfigStat.modified)) {
+ touch(packageConfigPath);
+ }
} else {
return null;
}
diff --git a/test/check_resolution_up_to_date_test.dart b/test/check_resolution_up_to_date_test.dart
index 316a11c..b24c315 100644
--- a/test/check_resolution_up_to_date_test.dart
+++ b/test/check_resolution_up_to_date_test.dart
@@ -38,9 +38,6 @@
exitCode: 0,
);
- // Timestamp resolution is rather poor especially on windows.
- await Future<Null>.delayed(const Duration(seconds: 1));
-
await d.appDir(dependencies: {'foo': '2.0.0'}).create();
await runPub(
@@ -89,9 +86,6 @@
exitCode: 0,
);
- // Timestamp resolution is rather poor especially on windows.
- await Future<Null>.delayed(const Duration(seconds: 1));
-
await d.dir(appPath, [
d.libPubspec(
'myapp',
diff --git a/test/embedding/ensure_pubspec_resolved.dart b/test/embedding/ensure_pubspec_resolved.dart
index 4808368..d7b5050 100644
--- a/test/embedding/ensure_pubspec_resolved.dart
+++ b/test/embedding/ensure_pubspec_resolved.dart
@@ -379,11 +379,10 @@
await d.dir(appPath, [
d.appPubspec(dependencies: {'foo': '1.0.0'}),
]).create();
- // Ensure we get a new mtime (mtime is only reported with 1s precision)
- await _touch('pubspec.yaml');
-
- await _touch('pubspec.lock');
- await _touch('.dart_tool/package_config.json');
+ // Ensure we get a new mtime across files on all platforms
+ await _touchWithDelay('pubspec.yaml');
+ await _touchWithDelay('pubspec.lock');
+ await _touchWithDelay('.dart_tool/package_config.json');
await _noImplicitPubGet();
});
@@ -535,10 +534,13 @@
/// Schedules a non-semantic modification to [path].
Future _touch(String path) async {
- // Delay a bit to make sure the modification times are noticeably different.
- // 1s seems to be the finest granularity that dart:io reports.
- await Future<void>.delayed(const Duration(seconds: 1));
+ path = p.join(d.sandbox, 'myapp', path);
+ touch(path);
+}
+/// Schedules a non-semantic modification to [path] with an artificial delay.
+Future _touchWithDelay(String path) async {
+ await Future<void>.delayed(const Duration(milliseconds: 10));
path = p.join(d.sandbox, 'myapp', path);
touch(path);
}
