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

Author: CodaFi

Sources/SwiftDriver/Utilities/VirtualPath.swift

@@ -715,18 +715,33 @@ extension TSCBasic.FileSystem {
     try resolvingVirtualPath(path, apply: exists)
   }
 
+  /// Retrieves the last modification time of the file referenced at the given path.
+  ///
+  /// If the given file path references a symbolic link, the modification time for the *linked file*
+  /// - not the symlink itself - is returned.
+  ///
+  /// - Parameter file: The path to a file.
+  /// - Throws: `SystemError` if the underlying `stat` operation fails.
+  /// - Returns: A `Date` value containing the last modification time.
   public func lastModificationTime(for file: VirtualPath) throws -> Date {
     try resolvingVirtualPath(file) { path in
       #if os(macOS)
       var s = Darwin.stat()
-      let err = lstat(path.pathString, &s)
+      let err = stat(path.pathString, &s)
       guard err == 0 else {
         throw SystemError.stat(errno, path.pathString)
       }
       let ti = (TimeInterval(s.st_mtimespec.tv_sec) - kCFAbsoluteTimeIntervalSince1970) + (1.0e-9 * TimeInterval(s.st_mtimespec.tv_nsec))
       return Date(timeIntervalSinceReferenceDate: ti)
       #else
-      return try localFileSystem.getFileInfo(file).modTime
+      // `getFileInfo` is going to ask Foundation to stat this path, and
+      // Foundation is always going to use `lstat` to do so. This is going to
+      // do the wrong thing for symbolic links, for which we always want to
+      // retrieve the mod time of the underlying file. This makes build systems
+      // that regenerate lots of symlinks but do not otherwise alter the
+      // contents of files - like Bazel - quite happy.
+      let path = resolveSymlinks(path)
+      return try localFileSystem.getFileInfo(path).modTime
       #endif
     }
   }

Tests/SwiftDriverTests/IncrementalCompilationTests.swift

@@ -280,6 +280,22 @@ extension IncrementalCompilationTests {
       try checkNullBuild(checkDiagnostics: true)
     }
   }
+
+  func testSymlinkModification() throws {
+    // Remap
+    // main.swift -> main.swift~link-target.swift
+    // other.swift -> other.swift~link-target.swift
+    for (file, _) in self.inputPathsAndContents {
+      try localFileSystem.createDirectory(tempDir.appending(component: "links"))
+      let linkTarget = tempDir.appending(component: "links").appending(component: file.basename)
+      try localFileSystem.move(from: file, to: linkTarget)
+      try localFileSystem.removeFileTree(file)
+      try localFileSystem.createSymbolicLink(file, pointingAt: linkTarget, relative: false)
+    }
+    try buildInitialState()
+    try checkReactionToTouchingSymlinks()
+    try checkReactionToTouchingSymlinkTargets()
+  }
 }
 
 // MARK: - Test adding an input
@@ -622,6 +638,69 @@ extension IncrementalCompilationTests {
     XCTAssert(graph.contains(sourceBasenameWithoutExt: newInput))
     XCTAssert(graph.contains(name: topLevelName))
   }
+
+  /// Check reaction to touching all symlinks
+  private func checkReactionToTouchingSymlinks(
+    checkDiagnostics: Bool = false,
+    extraArguments: [String] = []
+ ) throws {
+    for (file, _) in self.inputPathsAndContents {
+      try localFileSystem.removeFileTree(file)
+      let linkTarget = tempDir.appending(component: "links").appending(component: file.basename)
+      try localFileSystem.createSymbolicLink(file, pointingAt: linkTarget, relative: false)
+    }
+    try doABuild(
+      "touch both symlinks; non-propagating",
+      checkDiagnostics: checkDiagnostics,
+      extraArguments: extraArguments,
+      expectingRemarks: [
+        "Enabling incremental cross-module building",
+        "Incremental compilation: Read dependency graph",
+        "Incremental compilation: May skip current input  {compile: main.o <= main.swift}",
+        "Incremental compilation: May skip current input  {compile: other.o <= other.swift}",
+        "Incremental compilation: Skipping input:  {compile: main.o <= main.swift}",
+        "Incremental compilation: Skipping input:  {compile: other.o <= other.swift}",
+        "Incremental compilation: Skipping job: Linking theModule",
+        "Skipped Compiling main.swift",
+        "Skipped Compiling other.swift",
+    ],
+    whenAutolinking: autolinkLifecycleExpectations)
+  }
+
+  private func checkReactionToTouchingSymlinkTargets(
+    checkDiagnostics: Bool = false,
+    extraArguments: [String] = []
+ ) throws {
+    for (file, contents) in self.inputPathsAndContents {
+      let linkTarget = tempDir.appending(component: "links").appending(component: file.basename)
+      try! localFileSystem.writeFileContents(linkTarget) { $0 <<< contents }
+    }
+    try doABuild(
+      "touch both symlink targets; non-propagating",
+      checkDiagnostics: checkDiagnostics,
+      extraArguments: extraArguments,
+      expectingRemarks: [
+        "Enabling incremental cross-module building",
+        "Incremental compilation: Read dependency graph",
+        "Incremental compilation: Scheduing changed input  {compile: main.o <= main.swift}",
+        "Incremental compilation: Scheduing changed input  {compile: other.o <= other.swift}",
+        "Incremental compilation: Queuing (initial):  {compile: main.o <= main.swift}",
+        "Incremental compilation: Queuing (initial):  {compile: other.o <= other.swift}",
+        "Incremental compilation: not scheduling dependents of main.swift; unknown changes",
+        "Incremental compilation: not scheduling dependents of other.swift; unknown changes",
+        "Found 2 batchable jobs",
+        "Forming into 1 batch",
+        "Adding {compile: main.swift} to batch 0",
+        "Adding {compile: other.swift} to batch 0",
+        "Forming batch job from 2 constituents: main.swift, other.swift",
+        "Starting Compiling main.swift, other.swift",
+        "Finished Compiling main.swift, other.swift",
+        "Incremental compilation: Scheduling all post-compile jobs because something was compiled",
+        "Starting Linking theModule",
+        "Finished Linking theModule",
+    ],
+    whenAutolinking: autolinkLifecycleExpectations)
+  }
 }
 
 // MARK: - Incremental test perturbation helpers
@@ -776,8 +855,7 @@ extension IncrementalCompilationTests {
     }
   }
 
-  private func doABuildWithoutExpectations( arguments: [String]
-  ) throws -> Driver {
+  private func doABuildWithoutExpectations(arguments: [String]) throws -> Driver {
     // If not checking, print out the diagnostics
     let diagnosticEngine = DiagnosticsEngine(handlers: [
       {print($0, to: &stderrStream); stderrStream.flush()}