diff --git a/src/DiffEngine.Tests/InlineApplierUnixTests.cs b/src/DiffEngine.Tests/InlineApplierUnixTests.cs new file mode 100644 index 00000000..62d3dd19 --- /dev/null +++ b/src/DiffEngine.Tests/InlineApplierUnixTests.cs @@ -0,0 +1,80 @@ +#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] + // 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(); + } + + /// + /// 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] + // 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); + } + + /// + /// 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/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