ApplyCommand: convert to git internal format before applying patch

Applying a patch on Windows failed if the patch had the (normal)
single-LF line endings, but the file on disk had the usual Windows
CR-LF line endings.

Git (and JGit) compute diffs on the git-internal blob, i.e., after
CR-LF transformation and clean filtering. Applying patches to files
directly is thus incorrect and may fail if CR-LF settings don't
match, or if clean/smudge filtering is involved.

Change ApplyCommand to run the file content through the check-in
filters before applying the patch, and run the result through the
check-out filters. This makes patch application succeed even if the
patch has single-LFs, but the file has CR-LF and core.autocrlf is
true.

Add tests for various combinations of line endings in the file and in
the patch, and a test to verify the clean/smudge handling.

See also [1].

Running the file though clean/smudge may give strange results with
LFS-managed files. JGit's DiffFormatter has some extra code and
applies the smudge filter again after having run the file through
the check-in filters (CR-LF and clean). So JGit can actually produce
a diff on LFS-managed files using the normal diff machinery. (If it
doesn't run out of memory, that is. After all, LFS is intended for
_large_ files.) How such a diff would be applied with either C git
or JGit is entirely unclear; neither has any code for this special
case. Compare also [2].

Note that C git just doesn't know about LFS and always diffs after
the check-in filter chain, so for LFS files, it'll produce a diff
of the LFS pointers.

[1] https://github.com/git/git/commit/c24f3abac
[2] https://github.com/git-lfs/git-lfs/issues/440

Bug: 571585
Change-Id: I8f71ff26313b5773ff1da612b0938ad2f18751f5
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

15 files changed, 258 insertions, 1 deletions

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf.patch
new file mode 100644
index 0000000000..01eb0b9510
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf.patch
@@ -0,0 +1,9 @@
+diff --git a/crlf b/crlf
+index 9206ee6..95dd193 100644
+--- a/crlf
++++ b/crlf
+@@ -1,3 +1,3 @@
+ foo
+-fie
++bar
+ fum
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2.patch
new file mode 100644
index 0000000000..5a6210489c
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2.patch
@@ -0,0 +1,9 @@
+diff --git a/crlf2 b/crlf2
+index 05c1c78..91e246d 100644
+--- a/crlf2
++++ b/crlf2
+@@ -1,3 +1,3 @@
+ foo

+-fie

++bar

+ fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PostImage
new file mode 100644
index 0000000000..91e246d064
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PostImage
@@ -0,0 +1,3 @@
+foo

+bar

+fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PreImage
new file mode 100644
index 0000000000..05c1c782e5
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PreImage
@@ -0,0 +1,3 @@
+foo

+fie

+fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3.patch
new file mode 100644
index 0000000000..b155148b3e
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3.patch
@@ -0,0 +1,8 @@
+diff --git a/crlf3 b/crlf3
+index e69de29..9206ee6 100644
+--- a/crlf3
++++ b/crlf3
+@@ -0,0 +1,3 @@
++foo
++fie
++fum
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PostImage
new file mode 100644
index 0000000000..05c1c782e5
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PostImage
@@ -0,0 +1,3 @@
+foo

+fie

+fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PreImage
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PreImage
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4.patch
new file mode 100644
index 0000000000..0cf606390a
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4.patch
@@ -0,0 +1,9 @@
+diff --git a/crlf4 b/crlf4
+new file mode 100644
+index 0000000..9206ee6
+--- /dev/null
++++ b/crlf4
+@@ -0,0 +1,3 @@
++foo
++fie
++fum
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4_PostImage
new file mode 100644
index 0000000000..05c1c782e5
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4_PostImage
@@ -0,0 +1,3 @@
+foo

+fie

+fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PostImage
new file mode 100644
index 0000000000..91e246d064
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PostImage
@@ -0,0 +1,3 @@
+foo

+bar

+fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PreImage
new file mode 100644
index 0000000000..05c1c782e5
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PreImage
@@ -0,0 +1,3 @@
+foo

+fie

+fum

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest.patch
new file mode 100644
index 0000000000..ab4d4265a0
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest.patch
@@ -0,0 +1,9 @@
+diff --git a/smudgetest b/smudgetest
+index a24d41e..762c4d0 100644
+--- a/smudgetest
++++ b/smudgetest
+@@ -1,3 +1,3 @@
+ PERLE
+-HEBLE
++sprich
+ speak
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PostImage
new file mode 100644
index 0000000000..ad630893df
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PostImage
@@ -0,0 +1,3 @@
+PARLA
+sprich
+speak
diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PreImage
new file mode 100644
index 0000000000..9bbd8c763e
--- /dev/null
+++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PreImage
@@ -0,0 +1,3 @@
+PARLA
+HABLA
+speak
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ApplyCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ApplyCommandTest.java
index 055eba7184..335a64d70b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ApplyCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/ApplyCommandTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011, 2020 IBM Corporation and others
+ * Copyright (C) 2011, 2021 IBM Corporation and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -18,11 +18,17 @@ import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 
 import org.eclipse.jgit.api.errors.PatchApplyException;
 import org.eclipse.jgit.api.errors.PatchFormatException;
+import org.eclipse.jgit.attributes.FilterCommand;
+import org.eclipse.jgit.attributes.FilterCommandFactory;
+import org.eclipse.jgit.attributes.FilterCommandRegistry;
 import org.eclipse.jgit.diff.RawText;
 import org.eclipse.jgit.junit.RepositoryTestCase;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.junit.Test;
 
 public class ApplyCommandTest extends RepositoryTestCase {
@@ -58,6 +64,189 @@ public class ApplyCommandTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void testCrLf() throws Exception {
+		try {
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+			ApplyResult result = init("crlf", true, true);
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), "crlf"),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), "crlf"),
+					b.getString(0, b.size(), false));
+		} finally {
+			db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF);
+		}
+	}
+
+	@Test
+	public void testCrLfOff() throws Exception {
+		try {
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+			ApplyResult result = init("crlf", true, true);
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), "crlf"),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), "crlf"),
+					b.getString(0, b.size(), false));
+		} finally {
+			db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF);
+		}
+	}
+
+	@Test
+	public void testCrLfEmptyCommitted() throws Exception {
+		try {
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+			ApplyResult result = init("crlf3", true, true);
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), "crlf3"),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), "crlf3"),
+					b.getString(0, b.size(), false));
+		} finally {
+			db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF);
+		}
+	}
+
+	@Test
+	public void testCrLfNewFile() throws Exception {
+		try {
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+			ApplyResult result = init("crlf4", false, true);
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), "crlf4"),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), "crlf4"),
+					b.getString(0, b.size(), false));
+		} finally {
+			db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF);
+		}
+	}
+
+	@Test
+	public void testPatchWithCrLf() throws Exception {
+		try {
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+			ApplyResult result = init("crlf2", true, true);
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), "crlf2"),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), "crlf2"),
+					b.getString(0, b.size(), false));
+		} finally {
+			db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF);
+		}
+	}
+
+	@Test
+	public void testPatchWithCrLf2() throws Exception {
+		String name = "crlf2";
+		try (Git git = new Git(db)) {
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+			a = new RawText(readFile(name + "_PreImage"));
+			write(new File(db.getWorkTree(), name),
+					a.getString(0, a.size(), false));
+
+			git.add().addFilepattern(name).call();
+			git.commit().setMessage("PreImage").call();
+
+			b = new RawText(readFile(name + "_PostImage"));
+
+			db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+			ApplyResult result = git.apply()
+					.setPatch(getTestResource(name + ".patch")).call();
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), name),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), name),
+					b.getString(0, b.size(), false));
+		} finally {
+			db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+					ConfigConstants.CONFIG_KEY_AUTOCRLF);
+		}
+	}
+
+	// Clean/smudge filter for testFiltering. The smudgetest test resources were
+	// created with C git using a clean filter sed -e "s/A/E/g" and the smudge
+	// filter sed -e "s/E/A/g". To keep the test independent of the presence of
+	// sed, implement this with a built-in filter.
+	private static class ReplaceFilter extends FilterCommand {
+
+		private final char toReplace;
+
+		private final char replacement;
+
+		ReplaceFilter(InputStream in, OutputStream out, char toReplace,
+				char replacement) {
+			super(in, out);
+			this.toReplace = toReplace;
+			this.replacement = replacement;
+		}
+
+		@Override
+		public int run() throws IOException {
+			int b = in.read();
+			if (b < 0) {
+				in.close();
+				out.close();
+				return -1;
+			}
+			if ((b & 0xFF) == toReplace) {
+				b = replacement;
+			}
+			out.write(b);
+			return 1;
+		}
+	}
+
+	@Test
+	public void testFiltering() throws Exception {
+		// Set up filter
+		FilterCommandFactory clean = (repo, in, out) -> {
+			return new ReplaceFilter(in, out, 'A', 'E');
+		};
+		FilterCommandFactory smudge = (repo, in, out) -> {
+			return new ReplaceFilter(in, out, 'E', 'A');
+		};
+		FilterCommandRegistry.register("jgit://builtin/a2e/clean", clean);
+		FilterCommandRegistry.register("jgit://builtin/a2e/smudge", smudge);
+		try (Git git = new Git(db)) {
+			Config config = db.getConfig();
+			config.setString(ConfigConstants.CONFIG_FILTER_SECTION, "a2e",
+					"clean", "jgit://builtin/a2e/clean");
+			config.setString(ConfigConstants.CONFIG_FILTER_SECTION, "a2e",
+					"smudge", "jgit://builtin/a2e/smudge");
+			write(new File(db.getWorkTree(), ".gitattributes"),
+					"smudgetest filter=a2e");
+			git.add().addFilepattern(".gitattributes").call();
+			git.commit().setMessage("Attributes").call();
+			ApplyResult result = init("smudgetest", true, true);
+			assertEquals(1, result.getUpdatedFiles().size());
+			assertEquals(new File(db.getWorkTree(), "smudgetest"),
+					result.getUpdatedFiles().get(0));
+			checkFile(new File(db.getWorkTree(), "smudgetest"),
+					b.getString(0, b.size(), false));
+
+		} finally {
+			// Tear down filter
+			FilterCommandRegistry.unregister("jgit://builtin/a2e/clean");
+			FilterCommandRegistry.unregister("jgit://builtin/a2e/smudge");
+		}
+	}
+
+	@Test
 	public void testAddA1() throws Exception {
 		ApplyResult result = init("A1", false, true);
 		assertEquals(1, result.getUpdatedFiles().size());