Skip to content

Commit 9e24bc7

Browse files
authored
fix: repair no-op resolution timestamps (#4822)
1 parent 8e36831 commit 9e24bc7

6 files changed

Lines changed: 129 additions & 15 deletions

File tree

lib/src/command/dependency_services.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,11 @@ class DependencyServicesApplyCommand extends PubCommand {
575575
overriddenDependencies: entrypoint.lockFile.overriddenDependencies,
576576
);
577577

578-
newLockFile.writeToFile(entrypoint.lockFilePath, cache);
578+
newLockFile.writeToFile(
579+
entrypoint.lockFilePath,
580+
cache,
581+
dependencies: const [],
582+
);
579583
}
580584
});
581585
// Dummy message.

lib/src/entrypoint.dart

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ See $workspacesDocUrl for more information.''',
435435
.sdkConstraints[sdk.identifier]
436436
?.effectiveConstraint,
437437
),
438+
dependencies: [lockFilePath],
438439
);
439440
writeTextFileIfDifferent(packageGraphPath, await _packageGraphFile(cache));
440441

@@ -458,6 +459,13 @@ See $workspacesDocUrl for more information.''',
458459
}
459460
}
460461

462+
Iterable<String> _resolutionFileDependencies() sync* {
463+
for (final package in workspaceRoot.transitiveWorkspace) {
464+
yield package.pubspecPath;
465+
yield package.pubspecOverridesPath;
466+
}
467+
}
468+
461469
Future<String> _packageGraphFile(SystemCache cache) async {
462470
return const JsonEncoder.withIndent(' ').convert({
463471
'roots':
@@ -649,18 +657,23 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
649657
`--enforce-lockfile`.''');
650658
}
651659

660+
_lockFile = newLockFile;
661+
late final packageGraph = PackageGraph.fromSolveResult(this, result);
662+
652663
if (!(dryRun || enforceLockfile)) {
653-
newLockFile.writeToFile(lockFilePath, cache);
664+
newLockFile.writeToFile(
665+
lockFilePath,
666+
cache,
667+
dependencies: _resolutionFileDependencies(),
668+
);
654669
}
655670

656-
_lockFile = newLockFile;
657-
658671
if (!dryRun) {
659672
_removeStrayLockAndConfigFiles();
660673

661674
/// Build a package graph from the version solver results so we don't
662675
/// have to reload and reparse all the pubspecs.
663-
_packageGraph = Future.value(PackageGraph.fromSolveResult(this, result));
676+
_packageGraph = Future.value(packageGraph);
664677

665678
await writePackageConfigFiles();
666679

lib/src/global_packages.dart

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,10 +233,11 @@ Follow progress in https://github.com/dart-lang/sdk/issues/60889.
233233
final tempDir = cache.createTempDir();
234234
// TODO(rnystrom): Look in "bin" and display list of binaries that
235235
// user can run.
236-
LockFile(
237-
[id],
238-
mainDependencies: {id.name},
239-
).writeToFile(p.join(tempDir, 'pubspec.lock'), cache);
236+
LockFile([id], mainDependencies: {id.name}).writeToFile(
237+
p.join(tempDir, 'pubspec.lock'),
238+
cache,
239+
dependencies: const [],
240+
);
240241

241242
tryDeleteEntry(_packageDir(name));
242243
tryRenameDir(tempDir, _packageDir(name));
@@ -333,7 +334,11 @@ To recompile executables, first run `$topLevelProgram pub global deactivate $nam
333334
).show(summary: false);
334335
}
335336

336-
lockFile.writeToFile(p.join(tempDir, 'pubspec.lock'), cache);
337+
lockFile.writeToFile(
338+
p.join(tempDir, 'pubspec.lock'),
339+
cache,
340+
dependencies: const [],
341+
);
337342

338343
final packageDir = _packageDir(name);
339344
tryDeleteEntry(packageDir);

lib/src/io.dart

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,16 +265,35 @@ void writeTextFile(
265265
}
266266

267267
/// Reads the file at [path] and writes [newContent] to it, if it is different
268-
/// from [newContent].
268+
/// from the existing content.
269269
///
270270
/// If the file doesn't exist it is always written.
271-
void writeTextFileIfDifferent(String path, String newContent) {
272-
// Compare to the present package_config.json
273-
// For purposes of equality we don't care about the `generated` timestamp.
271+
///
272+
/// If the content is unchanged but any [dependencies] are newer than [path],
273+
/// [path] is touched.
274+
void writeTextFileIfDifferent(
275+
String path,
276+
String newContent, {
277+
Iterable<String> dependencies = const [],
278+
}) {
274279
final originalText = tryReadTextFile(path);
275280
if (originalText != newContent) {
276281
writeTextFile(path, newContent);
277282
} else {
283+
final modified = tryStatFile(path)?.modified;
284+
if (modified != null) {
285+
for (final dependency in dependencies) {
286+
final dependencyModified = tryStatFile(dependency)?.modified;
287+
if (dependencyModified != null &&
288+
dependencyModified.isAfter(modified)) {
289+
log.fine(
290+
'`$path` is unchanged. Touching because `$dependency` is newer.',
291+
);
292+
touch(path);
293+
return;
294+
}
295+
}
296+
}
278297
log.fine('`$path` is unchanged. Not rewriting.');
279298
}
280299
}

lib/src/lock_file.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,14 @@ ${yamlToString(data)}
389389
/// uses that.
390390
///
391391
/// Relative paths will be resolved relative to [lockFilePath]
392-
void writeToFile(String lockFilePath, SystemCache cache) {
392+
///
393+
/// If the serialized lockfile is unchanged, but any [dependencies] are newer
394+
/// than [lockFilePath], the lockfile is touched.
395+
void writeToFile(
396+
String lockFilePath,
397+
SystemCache cache, {
398+
required Iterable<String> dependencies,
399+
}) {
393400
final windowsLineEndings =
394401
fileExists(lockFilePath) &&
395402
detectWindowsLineEndings(readTextFile(lockFilePath));
@@ -398,6 +405,7 @@ ${yamlToString(data)}
398405
writeTextFileIfDifferent(
399406
lockFilePath,
400407
windowsLineEndings ? serialized.replaceAll('\n', '\r\n') : serialized,
408+
dependencies: dependencies,
401409
);
402410
}
403411

test/package_config_file_test.dart

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,4 +379,69 @@ void main() {
379379
expect(lockFile.lastModifiedSync(), lockfileTimestamp);
380380
expect(workspaceRefFile.lastModifiedSync(), workspaceRefTimestamp);
381381
});
382+
383+
test(
384+
'pub get touches unchanged resolution files after pubspec edits',
385+
() async {
386+
final server = await servePackages();
387+
server.serve('foo', '1.0.0');
388+
389+
await d.dir(appPath, [
390+
d.appPubspec(
391+
dependencies: {'foo': 'any'},
392+
extras: {
393+
'workspace': ['foo'],
394+
'environment': {'sdk': '^3.5.0'},
395+
},
396+
),
397+
d.dir('foo', [d.libPubspec('foo', '1.0.0', resolutionWorkspace: true)]),
398+
]).create();
399+
400+
await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'});
401+
final rootPubspec = File(p.join(sandbox, appPath, 'pubspec.yaml'));
402+
final workspacePubspec = File(
403+
p.join(sandbox, appPath, 'foo', 'pubspec.yaml'),
404+
);
405+
final lockFile = File(p.join(sandbox, appPath, 'pubspec.lock'));
406+
final packageConfigFile = File(
407+
p.join(sandbox, appPath, '.dart_tool', 'package_config.json'),
408+
);
409+
final lockfileContents = lockFile.readAsStringSync();
410+
final packageConfig = jsonDecode(packageConfigFile.readAsStringSync());
411+
412+
// timestamp resolution is rather poor especially on windows.
413+
await Future<void>.delayed(const Duration(seconds: 1));
414+
rootPubspec.writeAsStringSync(
415+
'${rootPubspec.readAsStringSync()}\n# hi\n',
416+
);
417+
workspacePubspec.writeAsStringSync(
418+
'${workspacePubspec.readAsStringSync()}\n# hi\n',
419+
);
420+
421+
expect(
422+
rootPubspec.lastModifiedSync().isAfter(lockFile.lastModifiedSync()),
423+
isTrue,
424+
);
425+
expect(
426+
workspacePubspec.lastModifiedSync().isAfter(
427+
lockFile.lastModifiedSync(),
428+
),
429+
isTrue,
430+
);
431+
432+
await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'});
433+
434+
expect(lockFile.readAsStringSync(), lockfileContents);
435+
expect(jsonDecode(packageConfigFile.readAsStringSync()), packageConfig);
436+
437+
final lockFileModified = lockFile.lastModifiedSync();
438+
final packageConfigModified = packageConfigFile.lastModifiedSync();
439+
expect(rootPubspec.lastModifiedSync().isAfter(lockFileModified), isFalse);
440+
expect(
441+
workspacePubspec.lastModifiedSync().isAfter(lockFileModified),
442+
isFalse,
443+
);
444+
expect(lockFileModified.isAfter(packageConfigModified), isFalse);
445+
},
446+
);
382447
}

0 commit comments

Comments
 (0)