diff --git a/lib/src/command/check_resolution_up_to_date.dart b/lib/src/command/check_resolution_up_to_date.dart index 66fff46d7b2..99d7fb89281 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 @@ Otherwise exit non-zero. @override Future 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 39e5b5ecac9..cc0fb6b6669 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -826,9 +826,9 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without /// 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 @@ -845,12 +845,27 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without /// `.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 = () { + for (final parent in parentDirs(dir)) { + if (tryStatFile(p.join(parent, 'pubspec.yaml')) != null) { + return parent; + } + } + return 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. /// @@ -864,6 +879,12 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without /// 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); @@ -961,7 +982,9 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without // 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) { @@ -983,8 +1006,8 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without } 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 @@ -1017,7 +1040,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without ); return false; } - packagePathsMapping[pkg.name] = root.path( + packagePathsMapping[pkg.name] = workspaceRoot.path( '.dart_tool', p.fromUri(pkg.rootUri), ); @@ -1039,6 +1062,20 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without // 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( @@ -1168,12 +1205,19 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without ); 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. @@ -1229,6 +1273,11 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without 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. @@ -1254,10 +1303,13 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without return null; } - if (pubspecStat.modified.isAfter(lockFileModified)) { + if (!lockFileModified.isAfterWithPrecision(pubspecStat.modified)) { log.fine('`$pubspecPath` is newer than `$lockFilePath`'); lockfileNewerThanPubspecs = false; - break; + if (pubspecStat.modified.isAfterWithPrecision(lockFileModified)) { + pubspecStrictlyNewer = true; + break; + } } final pubspecOverridesPath = p.join( package.rootUri.path, @@ -1268,30 +1320,51 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without // 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.isAfterWithPrecision( + pubspecOverridesStat.modified, + )) { log.fine('`$pubspecOverridesPath` is newer than `$lockFilePath`'); lockfileNewerThanPubspecs = false; + if (pubspecOverridesStat.modified.isAfterWithPrecision( + 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.isBeforeWithPrecision(lockFileModified)) { + log.fine( + 'Timestamps are out of order (updateOutOfDateTimestamps: false)', + ); + return null; + } + if (touchedLockFile || - lockFileModified.isAfter(packageConfigStat.modified)) { + !lockfileNewerThanPubspecs || + packageConfigStat.modified.isBeforeWithPrecision(lockFileModified)) { log.fine('`$lockFilePath` is newer than `$packageConfigPath`'); if (isPackageConfigUpToDate( packageConfig, @@ -1300,7 +1373,10 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without packageConfigPath: packageConfigPath, lockFilePath: lockFilePath, )) { - touch(packageConfigPath); + if (touchedLockFile || + lockFileModified.isAfterWithPrecision(packageConfigStat.modified)) { + touch(packageConfigPath); + } } else { return null; } @@ -1628,3 +1704,25 @@ See https://dart.dev/go/sdk-constraint /// For each package in a workspace, a set of changes to dependencies. typedef ChangeSet = Map>; + +extension on DateTime { + /// Whether this [DateTime] is strictly after [other], ignoring + /// sub-second precision on Windows due to `setLastModifiedSync` truncation. + bool isAfterWithPrecision(DateTime other) { + if (platform.isWindows) { + return millisecondsSinceEpoch ~/ 1000 > + other.millisecondsSinceEpoch ~/ 1000; + } + return isAfter(other); + } + + /// Whether this [DateTime] is strictly before [other], ignoring + /// sub-second precision on Windows due to `setLastModifiedSync` truncation. + bool isBeforeWithPrecision(DateTime other) { + if (platform.isWindows) { + return millisecondsSinceEpoch ~/ 1000 < + other.millisecondsSinceEpoch ~/ 1000; + } + return isBefore(other); + } +} diff --git a/lib/src/io.dart b/lib/src/io.dart index f23eac810e9..db81832aa3b 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -1118,6 +1118,15 @@ class PubProcess { /// Updates [path]'s modification time. void touch(String path) { log.fine('Touching `$path`'); + if (platform.isLinux || platform.isMacOS) { + try { + // setLastModifiedSync has poor resolution. We call out to touch instead. + Process.runSync('touch', [path]); + return; + } on Exception catch (_) { + // Fallback if touch command is somehow not found. + } + } File(path).setLastModifiedSync(DateTime.now()); } diff --git a/test/check_resolution_up_to_date_test.dart b/test/check_resolution_up_to_date_test.dart index 316a11cd1a0..b24c315ef3e 100644 --- a/test/check_resolution_up_to_date_test.dart +++ b/test/check_resolution_up_to_date_test.dart @@ -38,9 +38,6 @@ void main() { exitCode: 0, ); - // Timestamp resolution is rather poor especially on windows. - await Future.delayed(const Duration(seconds: 1)); - await d.appDir(dependencies: {'foo': '2.0.0'}).create(); await runPub( @@ -89,9 +86,6 @@ void main() { exitCode: 0, ); - // Timestamp resolution is rather poor especially on windows. - await Future.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 4808368b4a8..b9eaaa4243e 100644 --- a/test/embedding/ensure_pubspec_resolved.dart +++ b/test/embedding/ensure_pubspec_resolved.dart @@ -380,10 +380,10 @@ void testEnsurePubspecResolved() { 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 _touchWithDelay('pubspec.yaml'); - await _touch('pubspec.lock'); - await _touch('.dart_tool/package_config.json'); + await _touchWithDelay('pubspec.lock'); + await _touchWithDelay('.dart_tool/package_config.json'); await _noImplicitPubGet(); }); @@ -535,10 +535,13 @@ Future _noImplicitPubGet({Map? environment}) async { /// 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.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.delayed(const Duration(seconds: 1)); path = p.join(d.sandbox, 'myapp', path); touch(path); }