From 7c2d318a6673f727aab667bd9b13bc87dd18e0a7 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sun, 23 Aug 2026 17:57:39 +1000 Subject: [PATCH 1/2] 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. --- .../InlineApplierUnixTests.cs | 78 +++++++++++++++++++ .../SkipOnWindowsAttribute.cs | 10 +++ src/DiffEngine/Inline/InlineApplier.cs | 66 +++++++++++++++- 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 src/DiffEngine.Tests/InlineApplierUnixTests.cs create mode 100644 src/DiffEngine.Tests/SkipOnWindowsAttribute.cs diff --git a/src/DiffEngine.Tests/InlineApplierUnixTests.cs b/src/DiffEngine.Tests/InlineApplierUnixTests.cs new file mode 100644 index 00000000..ab69978b --- /dev/null +++ b/src/DiffEngine.Tests/InlineApplierUnixTests.cs @@ -0,0 +1,78 @@ +#if NET10_0 +/// +/// 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. +/// +public class InlineApplierUnixTests : + IDisposable +{ + const string source = "class C\n{\n void M() => Verify(value).Snapshot(\"old\");\n}"; + + /// + /// 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. + /// + [Test] + [SkipOnWindows("Making a symlink on Windows needs elevation or developer mode, so this cannot be arranged there.")] + 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(); + } + + /// + /// 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. + /// + [Test] + [SkipOnWindows("A Unix file mode is not a thing Windows has.")] + 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); + } + + /// + /// Nothing here queues a patch, so none of them has a reviewable identity. + /// + 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 diff --git a/src/DiffEngine.Tests/SkipOnWindowsAttribute.cs b/src/DiffEngine.Tests/SkipOnWindowsAttribute.cs new file mode 100644 index 00000000..445364d5 --- /dev/null +++ b/src/DiffEngine.Tests/SkipOnWindowsAttribute.cs @@ -0,0 +1,10 @@ +/// +/// Skips a test on Windows, with the reason given at the use site. For the cases whose subject is +/// a Unix file property - a symlink that can be made without elevation, a permission bit - which +/// Windows either does not have or reports differently. +/// +public sealed class SkipOnWindowsAttribute(string reason) : SkipAttribute(reason) +{ + public override Task ShouldSkip(TestRegisteredContext context) => + Task.FromResult(OperatingSystem.IsWindows()); +} diff --git a/src/DiffEngine/Inline/InlineApplier.cs b/src/DiffEngine/Inline/InlineApplier.cs index 2b5a54e3..ed5d0de3 100644 --- a/src/DiffEngine/Inline/InlineApplier.cs +++ b/src/DiffEngine/Inline/InlineApplier.cs @@ -1,4 +1,4 @@ -namespace DiffEngine; +namespace DiffEngine; /// /// Applies an 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. /// /// + /// + /// The file a symlinked source points at, which is the file to patch. + /// + /// 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. + /// + /// + /// 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. + /// + /// + 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 + } + + /// + /// 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. + /// + 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 From 0f68c1d6b0fdfb48489694bac2a2b128eabf7099 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sun, 23 Aug 2026 18:13:19 +1000 Subject: [PATCH 2/2] Restrict the Unix tests with TUnit's own RunOn --- src/DiffEngine.Tests/InlineApplierUnixTests.cs | 6 ++++-- src/DiffEngine.Tests/SkipOnWindowsAttribute.cs | 10 ---------- 2 files changed, 4 insertions(+), 12 deletions(-) delete mode 100644 src/DiffEngine.Tests/SkipOnWindowsAttribute.cs diff --git a/src/DiffEngine.Tests/InlineApplierUnixTests.cs b/src/DiffEngine.Tests/InlineApplierUnixTests.cs index ab69978b..62d3dd19 100644 --- a/src/DiffEngine.Tests/InlineApplierUnixTests.cs +++ b/src/DiffEngine.Tests/InlineApplierUnixTests.cs @@ -16,7 +16,8 @@ public class InlineApplierUnixTests : /// same snapshot again and the patched copy was invisible to the compiler. /// [Test] - [SkipOnWindows("Making a symlink on Windows needs elevation or developer mode, so this cannot be arranged there.")] + // 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"); @@ -39,7 +40,8 @@ public async Task A_symlinked_source_is_followed_to_the_file_it_names() /// as whatever the umask said. /// [Test] - [SkipOnWindows("A Unix file mode is not a thing Windows has.")] + // 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"); diff --git a/src/DiffEngine.Tests/SkipOnWindowsAttribute.cs b/src/DiffEngine.Tests/SkipOnWindowsAttribute.cs deleted file mode 100644 index 445364d5..00000000 --- a/src/DiffEngine.Tests/SkipOnWindowsAttribute.cs +++ /dev/null @@ -1,10 +0,0 @@ -/// -/// Skips a test on Windows, with the reason given at the use site. For the cases whose subject is -/// a Unix file property - a symlink that can be made without elevation, a permission bit - which -/// Windows either does not have or reports differently. -/// -public sealed class SkipOnWindowsAttribute(string reason) : SkipAttribute(reason) -{ - public override Task ShouldSkip(TestRegisteredContext context) => - Task.FromResult(OperatingSystem.IsWindows()); -}