Patch the file a symlinked source names (#845)

* Patch the file a symlinked source names

The applier rewrites the whole file through a temporary and swaps it in with
File.Replace, which on Linux and macOS is a rename. A rename replaces the name it is
given, so for a source reached through a symlink - a worktree, a shared checkout, a
vendored copy - the link itself was replaced by a regular file: the link stopped
being one, and the file it named still held the snapshot that had failed. Every
later run reported the same snapshot again, against source the compiler was still
reading from the target.

The link is followed before anything else, so the lock, the mutex, the read and the
swap all name the file that actually holds the source. Two links to one file get the
same lock as a side effect.

The rename also leaves the temporary's permissions on the file. A source that was
executable, or group writable, came back as whatever this process's umask said, so
the destination's mode is carried onto the temporary first. Windows keeps the
destination's ACLs across a Replace and needs none of this.

Both tests are Unix only: a symlink needs elevation on Windows and a file mode is
not a thing it has. The two behaviours behind them were confirmed against rename(2)
directly.

* Restrict the Unix tests with TUnit's own RunOn

Author: SimonCropp

src/DiffEngine.Tests/InlineApplierUnixTests.cs

@@ -0,0 +1,80 @@
+#if NET10_0
+/// <summary>
+/// What the applier does to the file itself, rather than to the text in it. The patch is written
+/// through a temporary and swapped in, and on Linux and macOS that swap is a rename - so what
+/// survives it is the temporary, with the temporary's identity.
+/// </summary>
+public class InlineApplierUnixTests :
+    IDisposable
+{
+    const string source = "class C\n{\n    void M() => Verify(value).Snapshot(\"old\");\n}";
+
+    /// <summary>
+    /// A source file reached through a symlink - a worktree, a vendored copy, a checkout shared
+    /// between two trees. The rename replaced the link with a regular file: the link stopped being
+    /// one, and the file it pointed at still held the old literal, so the next run reported the
+    /// same snapshot again and the patched copy was invisible to the compiler.
+    /// </summary>
+    [Test]
+    // A symlink on Windows needs elevation or developer mode, so this cannot be arranged there.
+    [RunOn(TUnit.Core.Enums.OS.Linux | TUnit.Core.Enums.OS.MacOs)]
+    public async Task A_symlinked_source_is_followed_to_the_file_it_names()
+    {
+        var real = Path.Combine(directory, "Real.cs");
+        File.WriteAllText(real, source);
+        var link = Path.Combine(directory, "Link.cs");
+        File.CreateSymbolicLink(link, real);
+
+        var result = InlineApplier.Apply(Patch(link));
+
+        await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied);
+        await Assert.That(File.ReadAllText(real)).Contains("\"new\"");
+        // Still a link, rather than a regular file holding the patch while the real one holds the
+        // snapshot that failed
+        await Assert.That(new FileInfo(link).LinkTarget).IsNotNull();
+    }
+
+    /// <summary>
+    /// The temporary is created with this process's umask, so without carrying the mode across, a
+    /// source file that was executable - or group writable, or read only to the world - came back
+    /// as whatever the umask said.
+    /// </summary>
+    [Test]
+    // A Unix file mode is not a thing Windows has.
+    [RunOn(TUnit.Core.Enums.OS.Linux | TUnit.Core.Enums.OS.MacOs)]
+    public async Task The_file_keeps_the_permissions_it_had()
+    {
+        var path = Path.Combine(directory, "Sample.cs");
+        File.WriteAllText(path, source);
+        const UnixFileMode mode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute |
+                                  UnixFileMode.GroupRead | UnixFileMode.GroupExecute |
+                                  UnixFileMode.OtherRead | UnixFileMode.OtherExecute;
+        File.SetUnixFileMode(path, mode);
+
+        var result = InlineApplier.Apply(Patch(path));
+
+        await Assert.That(result.Status).IsEqualTo(InlineApplyStatus.Applied);
+        await Assert.That(File.GetUnixFileMode(path)).IsEqualTo(mode);
+    }
+
+    /// <summary>
+    /// Nothing here queues a patch, so none of them has a reviewable identity.
+    /// </summary>
+    static InlinePatch Patch(string sourceFile) =>
+        new(sourceFile, 3, "\"old\"", "new")
+        {
+            TestName = null
+        };
+
+    public InlineApplierUnixTests()
+    {
+        directory = Path.Combine(Path.GetTempPath(), $"InlineApplierUnixTests_{Guid.NewGuid():N}");
+        Directory.CreateDirectory(directory);
+    }
+
+    public void Dispose() =>
+        Directory.Delete(directory, true);
+
+    readonly string directory;
+}
+#endif

src/DiffEngine/Inline/InlineApplier.cs

@@ -1,4 +1,4 @@
-namespace DiffEngine;
+namespace DiffEngine;
 
 /// <summary>
 /// Applies an <see cref="InlinePatch"/> to a source file, preserving the file's
@@ -56,6 +56,10 @@ static InlineApplyResult Run(InlinePatch patch, bool write)
             return InlineApplyResult.Failed($"Invalid InlinePatch.SourceFile: {patch.SourceFile}", exception);
         }
 
+        // Followed before anything else, so the lock, the mutex, the read and the swap all name
+        // the file that actually holds the source
+        fullPath = ResolveLink(fullPath);
+
         var newContent = SourceLanguage.NormalizeNewlines(patch.NewContent);
         var normalizedPath = fullPath.ToLowerInvariant();
         lock (gates.GetOrAdd(normalizedPath, static _ => new()))
@@ -199,6 +203,65 @@ static InlineApplyResult LockedApply(string fullPath, InlinePatch patch, string
     /// framework this targets.
     /// </para>
     /// </summary>
+    /// <summary>
+    /// The file a symlinked source points at, which is the file to patch.
+    /// <para>
+    /// The whole file is rewritten through a temporary and swapped in, and on Linux and macOS that
+    /// swap is a rename: it replaces the link itself with a regular file, leaving the target still
+    /// holding the old literal and the link no longer a link. Following it first puts the patch on
+    /// the real file, and gives two links to one file the same lock into the bargain.
+    /// </para>
+    /// <para>
+    /// The final target rather than one hop, since a chain has the same problem, and the path as
+    /// it stands when nothing resolves: a broken link is a file that cannot be read, which the
+    /// read reports better than this could.
+    /// </para>
+    /// </summary>
+    static string ResolveLink(string path)
+    {
+#if NET6_0_OR_GREATER
+        try
+        {
+            return File.ResolveLinkTarget(path, true)?.FullName ?? path;
+        }
+        catch (Exception exception)
+            when (exception is IOException or UnauthorizedAccessException)
+        {
+            return path;
+        }
+#else
+        return path;
+#endif
+    }
+
+    /// <summary>
+    /// The destination's Unix permissions onto the temporary, because the swap is a rename and the
+    /// file that survives it is the temporary - created with this process's umask. A source file
+    /// that was executable, or group writable, or anything else out of the ordinary, came back as
+    /// whatever the umask happened to say. Windows keeps the destination's ACLs across a Replace,
+    /// so there is nothing to carry there.
+    /// </summary>
+    static void CopyMode(string destination, string temporary)
+    {
+#if NET7_0_OR_GREATER
+        if (OperatingSystem.IsWindows())
+        {
+            return;
+        }
+
+        try
+        {
+            File.SetUnixFileMode(temporary, File.GetUnixFileMode(destination));
+        }
+        catch (Exception exception)
+            when (exception is IOException or UnauthorizedAccessException)
+        {
+            // Best effort. The content is the point, and a mode that could not be read or set is
+            // not worth failing a patch that otherwise applied.
+        }
+#endif
+    }
+
     static void WriteThroughTemporary(string fullPath, byte[] output)
     {
         var directory = Path.GetDirectoryName(fullPath)!;
@@ -208,6 +271,7 @@ static void WriteThroughTemporary(string fullPath, byte[] output)
         try
         {
             File.WriteAllBytes(temporary, output);
+            CopyMode(fullPath, temporary);
             File.Replace(temporary, fullPath, null);
         }
         finally