Skip to content

Commit 10e75ff

Browse files
committed
Use stat Instead of lstat
Using `lstat` here will do the wrong thing when the input path is a symlink since it's going to give us mod time info on the symlink instead of the file it references. Since mod time updates are intentionally not propagated across links, this has the potential to cause us to miscompile if a file's contents change on disk but the symlink passed as input hasn't actually changed. It also has the potential to defeat the incremental build when build systems that generate symlinks on every build interact with the driver. On Darwin (and, technically, other UNIX-likes) the fix is simple: use stat instead of lstat which will resolve symlinks for us. On Windows the answer is... complicated. Instead, on the fallback path, resolve symlinks before retrieving the modification time of the file. rdar://78828871
1 parent 00ecf8a commit 10e75ff

File tree

3 files changed

+97
-4
lines changed

3 files changed

+97
-4
lines changed

Package.resolved

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Sources/SwiftDriver/Utilities/VirtualPath.swift

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -715,18 +715,33 @@ extension TSCBasic.FileSystem {
715715
try resolvingVirtualPath(path, apply: exists)
716716
}
717717

718+
/// Retrieves the last modification time of the file referenced at the given path.
719+
///
720+
/// If the given file path references a symbolic link, the modification time for the *linked file*
721+
/// - not the symlink itself - is returned.
722+
///
723+
/// - Parameter file: The path to a file.
724+
/// - Throws: `SystemError` if the underlying `stat` operation fails.
725+
/// - Returns: A `Date` value containing the last modification time.
718726
public func lastModificationTime(for file: VirtualPath) throws -> Date {
719727
try resolvingVirtualPath(file) { path in
720728
#if os(macOS)
721729
var s = Darwin.stat()
722-
let err = lstat(path.pathString, &s)
730+
let err = stat(path.pathString, &s)
723731
guard err == 0 else {
724732
throw SystemError.stat(errno, path.pathString)
725733
}
726734
let ti = (TimeInterval(s.st_mtimespec.tv_sec) - kCFAbsoluteTimeIntervalSince1970) + (1.0e-9 * TimeInterval(s.st_mtimespec.tv_nsec))
727735
return Date(timeIntervalSinceReferenceDate: ti)
728736
#else
729-
return try localFileSystem.getFileInfo(file).modTime
737+
// `getFileInfo` is going to ask Foundation to stat this path, and
738+
// Foundation is always going to use `lstat` to do so. This is going to
739+
// do the wrong thing for symbolic links, for which we always want to
740+
// retrieve the mod time of the underlying file. This makes build systems
741+
// that regenerate lots of symlinks but do not otherwise alter the
742+
// contents of files - like Bazel - quite happy.
743+
let path = resolveSymlinks(path)
744+
return try localFileSystem.getFileInfo(path).modTime
730745
#endif
731746
}
732747
}

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,22 @@ extension IncrementalCompilationTests {
287287
tryNoChange(checkDiagnostics: true)
288288
}
289289
}
290+
291+
func testSymlinkModification() throws {
292+
// Remap
293+
// main.swift -> links/main.swift
294+
// other.swift -> links/other.swift
295+
for (file, _) in self.inputPathsAndContents {
296+
try localFileSystem.createDirectory(tempDir.appending(component: "links"))
297+
let linkTarget = tempDir.appending(component: "links").appending(component: file.basename)
298+
try localFileSystem.move(from: file, to: linkTarget)
299+
try localFileSystem.removeFileTree(file)
300+
try localFileSystem.createSymbolicLink(file, pointingAt: linkTarget, relative: false)
301+
}
302+
try tryInitial()
303+
try checkReactionToTouchingSymlinks(checkDiagnostics: true)
304+
try checkReactionToTouchingSymlinkTargets(checkDiagnostics: true)
305+
}
290306
}
291307

292308
// MARK: - Incremental test stages
@@ -535,6 +551,68 @@ extension IncrementalCompilationTests {
535551
],
536552
whenAutolinking: autolinkLifecycleExpectations)
537553
}
554+
555+
private func checkReactionToTouchingSymlinks(
556+
checkDiagnostics: Bool = false,
557+
extraArguments: [String] = []
558+
) throws {
559+
for (file, _) in self.inputPathsAndContents {
560+
try localFileSystem.removeFileTree(file)
561+
let linkTarget = tempDir.appending(component: "links").appending(component: file.basename)
562+
try localFileSystem.createSymbolicLink(file, pointingAt: linkTarget, relative: false)
563+
}
564+
try doABuild(
565+
"touch both symlinks; non-propagating",
566+
checkDiagnostics: checkDiagnostics,
567+
extraArguments: extraArguments,
568+
expectingRemarks: [
569+
"Enabling incremental cross-module building",
570+
"Incremental compilation: Read dependency graph",
571+
"Incremental compilation: May skip current input: {compile: main.o <= main.swift}",
572+
"Incremental compilation: May skip current input: {compile: other.o <= other.swift}",
573+
"Incremental compilation: Skipping input: {compile: main.o <= main.swift}",
574+
"Incremental compilation: Skipping input: {compile: other.o <= other.swift}",
575+
"Incremental compilation: Skipping job: Linking theModule",
576+
"Skipped Compiling main.swift",
577+
"Skipped Compiling other.swift",
578+
],
579+
whenAutolinking: autolinkLifecycleExpectations)
580+
}
581+
582+
private func checkReactionToTouchingSymlinkTargets(
583+
checkDiagnostics: Bool = false,
584+
extraArguments: [String] = []
585+
) throws {
586+
for (file, contents) in self.inputPathsAndContents {
587+
let linkTarget = tempDir.appending(component: "links").appending(component: file.basename)
588+
try! localFileSystem.writeFileContents(linkTarget) { $0 <<< contents }
589+
}
590+
try doABuild(
591+
"touch both symlink targets; non-propagating",
592+
checkDiagnostics: checkDiagnostics,
593+
extraArguments: extraArguments,
594+
expectingRemarks: [
595+
"Enabling incremental cross-module building",
596+
"Incremental compilation: Read dependency graph",
597+
"Incremental compilation: Scheduing changed input {compile: main.o <= main.swift}",
598+
"Incremental compilation: Scheduing changed input {compile: other.o <= other.swift}",
599+
"Incremental compilation: Queuing (initial): {compile: main.o <= main.swift}",
600+
"Incremental compilation: Queuing (initial): {compile: other.o <= other.swift}",
601+
"Incremental compilation: not scheduling dependents of main.swift; unknown changes",
602+
"Incremental compilation: not scheduling dependents of other.swift; unknown changes",
603+
"Found 2 batchable jobs",
604+
"Forming into 1 batch",
605+
"Adding {compile: main.swift} to batch 0",
606+
"Adding {compile: other.swift} to batch 0",
607+
"Forming batch job from 2 constituents: main.swift, other.swift",
608+
"Starting Compiling main.swift, other.swift",
609+
"Finished Compiling main.swift, other.swift",
610+
"Incremental compilation: Scheduling all post-compile jobs because something was compiled",
611+
"Starting Linking theModule",
612+
"Finished Linking theModule",
613+
],
614+
whenAutolinking: autolinkLifecycleExpectations)
615+
}
538616
}
539617

540618
// MARK: - Incremental test perturbation helpers

0 commit comments

Comments
 (0)