diff --git a/GitIntegration.Test/Fixtures/patch-unmerged-binary-and-deleted.txt b/GitIntegration.Test/Fixtures/patch-unmerged-binary-and-deleted.txt new file mode 100644 index 0000000..f185393 --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-unmerged-binary-and-deleted.txt @@ -0,0 +1,4 @@ +diff --cc bin.dat +index 1323b0a,012c31b..0000000 +Binary files differ +* Unmerged path d.txt diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs index 466fa71..d1620ff 100644 --- a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -20,6 +20,7 @@ public class GitPatchRoundTripTests { private static readonly GitAuthorName AuthorName = "Fixture Author".As(); private static readonly GitAuthorEmail AuthorEmail = "fixture@example.com".As(); + private static readonly string[] ExpectedUnmergedPaths = ["bin.dat", "d.txt"]; [TestMethod] public async Task StagingOneHunkLeavesTheOtherUnstagedAsync() @@ -377,6 +378,70 @@ await IntegrationGitFixture.ConfigureIdentityAsync( return init.Repository; } + [TestMethod] + public async Task PatchAndDiffAgreeOnUnmergedPathsAfterABinaryAndAModifyDeleteConflictAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + GitRepository seeded = await SeedAsync(client, repository, []).ConfigureAwait(false); + + // Both sides change bin.dat, which git cannot merge as text, and one side deletes d.txt + // while the other modifies it. Neither conflict produces a @@@ hunk. + repository.WriteFile("bin.dat", "\0\u0001base"); + repository.WriteFile("d.txt", "1\n2\n3\n"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + await RunGitAsync(seeded, "checkout", "-q", "-b", "other").ConfigureAwait(false); + repository.WriteFile("bin.dat", "\0\u0001other"); + repository.WriteFile("d.txt", "1\n2\n3\n4\n"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + await RunGitAsync(seeded, "checkout", "-q", "main").ConfigureAwait(false); + repository.WriteFile("bin.dat", "\0\u0001main"); + repository.DeleteFile("d.txt"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + // merge is out of scope for this library, so the fixture runs it directly. It is expected + // to fail, leaving both paths unmerged. + GitProcessResult merged = await seeded.ProcessRunner!.RunAsync( + new GitProcessRequest { Arguments = ["-C", repository.RootPath, "merge", "--no-edit", "other"] }, + TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.IsFalse(merged.Success, "the merge was expected to conflict but succeeded"); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + + IReadOnlyList diff = await opened.Diff().ExecuteAsync().ConfigureAwait(false); + GitPatch patch = await opened.Patch().ExecuteAsync().ConfigureAwait(false); + + string[] diffUnmerged = [.. diff + .Where(entry => entry.Kind == GitChangeKind.Unmerged) + .Select(entry => entry.Path.WeakString) + .Order(StringComparer.Ordinal)]; + string[] patchUnmerged = [.. patch.Files + .Where(file => file.Kind == GitChangeKind.Unmerged && file.IsConflicted) + .Select(file => file.Path.WeakString) + .Order(StringComparer.Ordinal)]; + + CollectionAssert.AreEqual(ExpectedUnmergedPaths, diffUnmerged, "the fixture must leave both conflicts"); + CollectionAssert.AreEqual(diffUnmerged, patchUnmerged); + Assert.IsTrue(patch.Files.All(file => file.Hunks.Count == 0)); + } + + /// Runs a git command this library has no verb for, failing the test if git fails. + /// The repository to run in. + /// The git arguments, after -C <root>. + private async Task RunGitAsync(GitRepository repository, params string[] arguments) + { + GitProcessResult result = await repository.ProcessRunner!.RunAsync( + new GitProcessRequest { Arguments = ["-C", repository.LocalPath!.WeakString, .. arguments] }, + TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.IsTrue(result.Success, $"git {string.Join(' ', arguments)} failed"); + } + /// Stages everything in the working tree and commits it. /// The repository to commit in. private async Task CommitAllAsync(GitRepository repository) diff --git a/GitIntegration.Test/Parsing/GitPatchParserTests.cs b/GitIntegration.Test/Parsing/GitPatchParserTests.cs index 21a3a31..c898146 100644 --- a/GitIntegration.Test/Parsing/GitPatchParserTests.cs +++ b/GitIntegration.Test/Parsing/GitPatchParserTests.cs @@ -206,6 +206,40 @@ public void FlagsAConflictedFileAndGivesItNoHunks() Assert.AreEqual(0, file.Hunks.Count); } + [TestMethod] + public void FlagsAConflictedBinaryFileAndAModifyDeleteConflict() + { + // Captured with git 2.43.0 on Linux after a merge left bin.dat changed on both sides (UU) + // and d.txt deleted on one side and modified on the other (DU). Neither has a @@@ line. + GitPatch patch = GitPatchParser.Parse(Fixture("patch-unmerged-binary-and-deleted.txt")); + + Assert.AreEqual(2, patch.Files.Count); + + GitFilePatch binary = patch.Files[0]; + Assert.AreEqual("bin.dat", binary.Path.WeakString); + Assert.AreEqual(GitChangeKind.Unmerged, binary.Kind); + Assert.IsTrue(binary.IsConflicted); + Assert.AreEqual(0, binary.Hunks.Count); + + GitFilePatch deleted = patch.Files[1]; + Assert.AreEqual("d.txt", deleted.Path.WeakString); + Assert.AreEqual(GitChangeKind.Unmerged, deleted.Kind); + Assert.IsTrue(deleted.IsConflicted); + Assert.AreEqual(0, deleted.Hunks.Count); + } + + [TestMethod] + public void FlagsALongFormCombinedHeaderAsConflicted() + { + const string output = "diff --combined f.txt\nindex 305b879,1ebef9c..0000000\nBinary files differ\n"; + + GitFilePatch file = GitPatchParser.Parse(output).Files.Single(); + + Assert.AreEqual("f.txt", file.Path.WeakString); + Assert.AreEqual(GitChangeKind.Unmerged, file.Kind); + Assert.IsTrue(file.IsConflicted); + } + [TestMethod] public void ParsesCarriageReturnContentWithoutStrippingIt() { diff --git a/GitIntegration/Parsing/GitPatchParser.cs b/GitIntegration/Parsing/GitPatchParser.cs index b674879..4b8721d 100644 --- a/GitIntegration/Parsing/GitPatchParser.cs +++ b/GitIntegration/Parsing/GitPatchParser.cs @@ -22,6 +22,8 @@ internal static class GitPatchParser { private const string GitHeaderPrefix = "diff --git "; private const string CombinedHeaderPrefix = "diff --cc "; + private const string LongCombinedHeaderPrefix = "diff --combined "; + private const string UnmergedPathPrefix = "* Unmerged path "; private const string BSidePathMarker = " b/"; private const string RenameFromPrefix = "rename from "; private const string RenameToPrefix = "rename to "; @@ -36,7 +38,10 @@ internal static class GitPatchParser /// Parses the text a diff-producing command wrote to standard output. /// /// Everything git wrote to standard output. - /// The patch, with one file per diff --git or diff --cc block found. + /// + /// The patch, with one file per diff --git, diff --cc or diff --combined + /// block found, and one per * Unmerged path line. + /// /// A header or a hunk was malformed. public static GitPatch Parse(string output) { @@ -48,7 +53,19 @@ public static GitPatch Parse(string output) int index = 0; while (index < lines.Count) { - if (!IsFileStart(Line(output, lines, index))) + string line = Line(output, lines, index); + + if (line.StartsWith(UnmergedPathPrefix, StringComparison.Ordinal)) + { + // A modify/delete conflict has no content to diff, so git names the path on this + // one line instead of starting a file block for it. Diff() reports the same path + // as Unmerged, and leaving it out here would hide the conflict entirely. + files.Add(UnmergedPath(output, lines, index, line)); + index++; + continue; + } + + if (!IsFileStart(line)) { index++; continue; @@ -105,7 +122,23 @@ private static int RegionEnd(string output, List<(int Start, int End)> lines, in private static bool IsFileStart(string line) => line.StartsWith(GitHeaderPrefix, StringComparison.Ordinal) || - line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal); + IsCombinedStart(line); + + private static bool IsCombinedStart(string line) => + line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal) || + line.StartsWith(LongCombinedHeaderPrefix, StringComparison.Ordinal); + + private static GitFilePatch UnmergedPath(string output, List<(int Start, int End)> lines, int index, string line) => + new() + { + Path = GitParseValues.ToRelativeFilePath(UnquotePath(line[UnmergedPathPrefix.Length..], line)), + OriginalPath = null, + Kind = GitChangeKind.Unmerged, + IsBinary = false, + IsConflicted = true, + Header = output[lines[index].Start..RegionEnd(output, lines, index + 1)], + Hunks = [], + }; private static GitFilePatch ParseFile(string output, List<(int Start, int End)> lines, ref int index) { @@ -116,7 +149,11 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> RelativeFilePath? originalPath = null; GitChangeKind kind = GitChangeKind.Modified; bool isBinary = false; - bool isConflicted = false; + + // Git writes the combined format only for an unmerged path, and only some of those carry + // a @@@ line: a conflicted binary file stops at "Binary files differ". The header alone + // decides, so every such path reports the Unmerged that GitDiffParser gives it. + bool isConflicted = IsCombinedStart(firstLine); index++; @@ -125,6 +162,7 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> string line = Line(output, lines, index); if (IsFileStart(line) || + line.StartsWith(UnmergedPathPrefix, StringComparison.Ordinal) || line.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal) || line.StartsWith(HunkPrefix, StringComparison.Ordinal)) { @@ -135,6 +173,13 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> index++; } + // Set after the header lines, so that a mode line in a combined header cannot report an + // unmerged path as Added or Deleted. + if (isConflicted) + { + kind = GitChangeKind.Unmerged; + } + string header = output[fileStart..RegionEnd(output, lines, index)]; List hunks = []; @@ -142,17 +187,14 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> { string boundary = Line(output, lines, index); - if (boundary.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal)) + if (isConflicted) { // Combined format from an unmerged path is not a patch git apply accepts, so its // body is skipped rather than misread as ordinary hunks. Kind follows the same // enum member GitDiffParser reports for the path, so a caller switching on // GitChangeKind gets one answer from both verbs. - isConflicted = true; - kind = GitChangeKind.Unmerged; - index++; - - while (index < lines.Count && !IsFileStart(Line(output, lines, index))) + while (index < lines.Count && !IsFileStart(Line(output, lines, index)) && + !Line(output, lines, index).StartsWith(UnmergedPathPrefix, StringComparison.Ordinal)) { index++; } @@ -202,6 +244,11 @@ private static string ReadPathFromFileStart(string line) return UnquotePath(line[CombinedHeaderPrefix.Length..], line); } + if (line.StartsWith(LongCombinedHeaderPrefix, StringComparison.Ordinal)) + { + return UnquotePath(line[LongCombinedHeaderPrefix.Length..], line); + } + string remainder = line[GitHeaderPrefix.Length..]; // A path git had to C-quote puts the whole operand, prefix included, inside the quotes.