From e58bf0870e069fc758d1d9c83b83b8ab80929f34 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 29 Apr 2021 12:58:23 +0200 Subject: Add git config for conflict style merge/diff3 Add a constant in ConfigConstants, and a ConflictStyle enum in MergeCommand. Change-Id: Idf8e036b6b6953bec06d6923a39e5ff30c2da562 Signed-off-by: Thomas Wolf --- .../src/org/eclipse/jgit/api/MergeCommand.java | 14 ++++++++++++++ .../src/org/eclipse/jgit/lib/ConfigConstants.java | 8 ++++++++ 2 files changed, 22 insertions(+) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java index c611f915ae..ef56d802c8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java @@ -86,6 +86,20 @@ public class MergeCommand extends GitCommand { private ProgressMonitor monitor = NullProgressMonitor.INSTANCE; + /** + * Values for the "merge.conflictStyle" git config. + * + * @since 5.12 + */ + public enum ConflictStyle { + + /** "merge" style: only ours/theirs. This is the default. */ + MERGE, + + /** "diff3" style: ours/base/theirs. */ + DIFF3 + } + /** * The modes available for fast forward merges corresponding to the * --ff, --no-ff and --ff-only diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 03c1ef904c..3e3d9b5694 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -397,8 +397,16 @@ public final class ConfigConstants { /** The "ff" key */ public static final String CONFIG_KEY_FF = "ff"; + /** + * The "conflictStyle" key. + * + * @since 5.12 + */ + public static final String CONFIG_KEY_CONFLICTSTYLE = "conflictStyle"; + /** * The "checkstat" key + * * @since 3.0 */ public static final String CONFIG_KEY_CHECKSTAT = "checkstat"; -- cgit v1.2.3 From cb65846722e82300ef570bc9e161ac1c9ca97348 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 14 May 2021 09:02:46 +0200 Subject: Fix @since tag for introduction of PUBKEY_ACCEPTED_ALGORITHMS This constant was shipped with 5.11.1. Change-Id: I480dbefab1cccca78cefbc709b79e5405f8bf8cd Signed-off-by: Matthias Sohn --- org.eclipse.jgit/.settings/.api_filters | 11 +++++++++++ .../src/org/eclipse/jgit/transport/SshConstants.java | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jgit/.settings/.api_filters (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters new file mode 100644 index 0000000000..33331fbab7 --- /dev/null +++ b/org.eclipse.jgit/.settings/.api_filters @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshConstants.java index be55cd1b81..5cd5b334ab 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/SshConstants.java @@ -118,7 +118,7 @@ public final class SshConstants { * Key in an ssh config file; defines signature algorithms for public key * authentication as a comma-separated list. * - * @since 5.11 + * @since 5.11.1 */ public static final String PUBKEY_ACCEPTED_ALGORITHMS = "PubkeyAcceptedAlgorithms"; -- cgit v1.2.3 From 87704b773654470508de1dc9570914cad168ccf9 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 15 May 2021 18:13:04 +0200 Subject: SSH config: fix negated patterns Negated patterns were handled wrongly. According to the OpenBSD ssh_config man page,[1] a negated pattern never matches. Negated patterns make only sense if there are positive patterns; the negated pattern then can define exceptions for the positive patterns. OpenSshConfigFile did this wrongly. It handled "!foo" as "matching everything but foo", but actually the semantics is "if the input is "foo", this entry doesn't apply. If the input is anything else, other patterns determine whether the entry may apply.". [1] https://man.openbsd.org/ssh_config Change-Id: I50f6e46581b7ece4c949eddf62f4a265573ec29e Signed-off-by: Thomas Wolf --- .../eclipse/jgit/transport/OpenSshConfigTest.java | 57 ++++++++++- .../internal/transport/ssh/OpenSshConfigFile.java | 108 ++++++++++----------- 2 files changed, 106 insertions(+), 59 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java index 4be2271a8c..82109582f5 100644 --- a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java +++ b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008, 2017 Google Inc. and others + * Copyright (C) 2008, 2021 Google Inc. 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 @@ -517,4 +517,59 @@ public class OpenSshConfigTest extends RepositoryTestCase { assertEquals("/tmp/${TST_VAR/bar", c.getValue(SshConstants.IDENTITY_AGENT)); } + + @Test + public void testNegativeMatch() throws Exception { + config("Host foo.bar !foobar.baz *.baz\n" + "Port 29418\n"); + Host h = osc.lookup("foo.bar"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + h = osc.lookup("foobar.baz"); + assertNotNull(h); + assertEquals(22, h.getPort()); + h = osc.lookup("foo.baz"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + } + + @Test + public void testNegativeMatch2() throws Exception { + // Negative match after the positive match. + config("Host foo.bar *.baz !foobar.baz\n" + "Port 29418\n"); + Host h = osc.lookup("foo.bar"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + h = osc.lookup("foobar.baz"); + assertNotNull(h); + assertEquals(22, h.getPort()); + h = osc.lookup("foo.baz"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + } + + @Test + public void testNoMatch() throws Exception { + config("Host !host1 !host2\n" + "Port 29418\n"); + Host h = osc.lookup("host1"); + assertNotNull(h); + assertEquals(22, h.getPort()); + h = osc.lookup("host2"); + assertNotNull(h); + assertEquals(22, h.getPort()); + h = osc.lookup("host3"); + assertNotNull(h); + assertEquals(22, h.getPort()); + } + + @Test + public void testMultipleMatch() throws Exception { + config("Host foo.bar\nPort 29418\nIdentityFile /foo\n\n" + + "Host *.bar\nPort 22\nIdentityFile /bar\n" + + "Host foo.bar\nPort 47\nIdentityFile /baz\n"); + Host h = osc.lookup("foo.bar"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + assertArrayEquals(new Object[] { "/foo", "/bar", "/baz" }, + h.getConfig().getValues("IdentityFile")); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java index de6a346cb2..e4753df00c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java @@ -1,6 +1,6 @@ /* * Copyright (C) 2008, 2017, Google Inc. - * Copyright (C) 2017, 2018, Thomas Wolf and others + * Copyright (C) 2017, 2021, Thomas Wolf 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 @@ -21,7 +21,8 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashMap; +import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -82,12 +83,6 @@ import org.eclipse.jgit.util.SystemReader; */ public class OpenSshConfigFile implements SshConfigStore { - /** - * "Host" name of the HostEntry for the default options before the first - * host block in a config file. - */ - private static final String DEFAULT_NAME = ""; //$NON-NLS-1$ - /** The user's home directory, as key files may be relative to here. */ private final File home; @@ -105,11 +100,9 @@ public class OpenSshConfigFile implements SshConfigStore { * fully resolved entries created from that. */ private static class State { - // Keyed by pattern; if a "Host" line has multiple patterns, we generate - // duplicate HostEntry objects - Map entries = new LinkedHashMap<>(); + List entries = new LinkedList<>(); - // Keyed by user@hostname:port + // Previous lookups, keyed by user@hostname:port Map hosts = new HashMap<>(); @Override @@ -165,14 +158,16 @@ public class OpenSshConfigFile implements SshConfigStore { return h; } HostEntry fullConfig = new HostEntry(); - // Initialize with default entries at the top of the file, before the - // first Host block. - fullConfig.merge(cache.entries.get(DEFAULT_NAME)); - for (Map.Entry e : cache.entries.entrySet()) { - String pattern = e.getKey(); - if (isHostMatch(pattern, hostName)) { - fullConfig.merge(e.getValue()); - } + Iterator entries = cache.entries.iterator(); + if (entries.hasNext()) { + // Should always have at least the first top entry containing + // key-value pairs before the first Host block + fullConfig.merge(entries.next()); + entries.forEachRemaining(entry -> { + if (entry.matches(hostName)) { + fullConfig.merge(entry); + } + }); } fullConfig.substitute(hostName, port, userName, localUserName, home); cache.hosts.put(cacheKey, fullConfig); @@ -208,20 +203,19 @@ public class OpenSshConfigFile implements SshConfigStore { return state; } - private Map parse(BufferedReader reader) + private List parse(BufferedReader reader) throws IOException { - final Map entries = new LinkedHashMap<>(); - final List current = new ArrayList<>(4); - String line; + final List entries = new LinkedList<>(); // The man page doesn't say so, but the openssh parser (readconf.c) // starts out in active mode and thus always applies any lines that // occur before the first host block. We gather those options in a // HostEntry for DEFAULT_NAME. HostEntry defaults = new HostEntry(); - current.add(defaults); - entries.put(DEFAULT_NAME, defaults); + HostEntry current = defaults; + entries.add(defaults); + String line; while ((line = reader.readLine()) != null) { // OpenSsh ignores trailing comments on a line. Anything after the // first # on a line is trimmed away (yes, even if the hash is @@ -246,38 +240,17 @@ public class OpenSshConfigFile implements SshConfigStore { String argValue = parts.length > 1 ? parts[1].trim() : ""; //$NON-NLS-1$ if (StringUtils.equalsIgnoreCase(SshConstants.HOST, keyword)) { - current.clear(); - for (String name : parseList(argValue)) { - if (name == null || name.isEmpty()) { - // null should not occur, but better be safe than sorry. - continue; - } - HostEntry c = entries.get(name); - if (c == null) { - c = new HostEntry(); - entries.put(name, c); - } - current.add(c); - } - continue; - } - - if (current.isEmpty()) { - // We received an option outside of a Host block. We - // don't know who this should match against, so skip. + current = new HostEntry(parseList(argValue)); + entries.add(current); continue; } if (HostEntry.isListKey(keyword)) { List args = validate(keyword, parseList(argValue)); - for (HostEntry entry : current) { - entry.setValue(keyword, args); - } + current.setValue(keyword, args); } else if (!argValue.isEmpty()) { argValue = validate(keyword, dequote(argValue)); - for (HostEntry entry : current) { - entry.setValue(keyword, argValue); - } + current.setValue(keyword, argValue); } } @@ -358,13 +331,6 @@ public class OpenSshConfigFile implements SshConfigStore { return value; } - private static boolean isHostMatch(String pattern, String name) { - if (pattern.startsWith("!")) { //$NON-NLS-1$ - return !patternMatchesHost(pattern.substring(1), name); - } - return patternMatchesHost(pattern, name); - } - private static boolean patternMatchesHost(String pattern, String name) { if (pattern.indexOf('*') >= 0 || pattern.indexOf('?') >= 0) { final FileNameMatcher fn; @@ -511,6 +477,32 @@ public class OpenSshConfigFile implements SshConfigStore { private Map> listOptions; + private final List patterns; + + // Constructor used to build the merged entry; never matches anything + HostEntry() { + this.patterns = Collections.emptyList(); + } + + HostEntry(List patterns) { + this.patterns = patterns; + } + + boolean matches(String hostName) { + boolean doesMatch = false; + for (String pattern : patterns) { + if (pattern.startsWith("!")) { //$NON-NLS-1$ + if (patternMatchesHost(pattern.substring(1), hostName)) { + return false; + } + } else if (!doesMatch + && patternMatchesHost(pattern, hostName)) { + doesMatch = true; + } + } + return doesMatch; + } + private static String toKey(String key) { String k = ALIASES.get(key); return k != null ? k : key; -- cgit v1.2.3 From c718e6059c0371dcde83d51f83f3031f934c34b7 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 15 May 2021 18:25:52 +0200 Subject: SSH config: fix whitespace handling Use Character.isWhitespace() instead of Character.isSpaceChar() to treat TABs as whitespace, too. Change-Id: Iffc59c13357d981ede6a1e0feb6ea6ff03fb3064 Signed-off-by: Thomas Wolf --- .../org/eclipse/jgit/transport/OpenSshConfigTest.java | 17 +++++++++++++++++ .../jgit/internal/transport/ssh/OpenSshConfigFile.java | 13 ++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java index 82109582f5..93d85e2e90 100644 --- a/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java +++ b/org.eclipse.jgit.ssh.jsch.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java @@ -572,4 +572,21 @@ public class OpenSshConfigTest extends RepositoryTestCase { assertArrayEquals(new Object[] { "/foo", "/bar", "/baz" }, h.getConfig().getValues("IdentityFile")); } + + @Test + public void testWhitespace() throws Exception { + config("Host foo \tbar baz\nPort 29418\n"); + Host h = osc.lookup("foo"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + h = osc.lookup("bar"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + h = osc.lookup("baz"); + assertNotNull(h); + assertEquals(29418, h.getPort()); + h = osc.lookup("\tbar"); + assertNotNull(h); + assertEquals(22, h.getPort()); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java index e4753df00c..bf93d77f7e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java @@ -273,7 +273,7 @@ public class OpenSshConfigFile implements SshConfigStore { int length = argument.length(); while (start < length) { // Skip whitespace - if (Character.isSpaceChar(argument.charAt(start))) { + if (Character.isWhitespace(argument.charAt(start))) { start++; continue; } @@ -288,7 +288,7 @@ public class OpenSshConfigFile implements SshConfigStore { } else { int stop = start + 1; while (stop < length - && !Character.isSpaceChar(argument.charAt(stop))) { + && !Character.isWhitespace(argument.charAt(stop))) { stop++; } result.add(argument.substring(start, stop)); @@ -355,9 +355,12 @@ public class OpenSshConfigFile implements SshConfigStore { private static String stripWhitespace(String value) { final StringBuilder b = new StringBuilder(); - for (int i = 0; i < value.length(); i++) { - if (!Character.isSpaceChar(value.charAt(i))) - b.append(value.charAt(i)); + int length = value.length(); + for (int i = 0; i < length; i++) { + char ch = value.charAt(i); + if (!Character.isWhitespace(ch)) { + b.append(ch); + } } return b.toString(); } -- cgit v1.2.3 From d2846cc8b2a831a089ee768a0475e64ec5b85519 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 2 Mar 2021 09:53:08 +0100 Subject: 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 --- .../tst-rsrc/org/eclipse/jgit/diff/crlf.patch | 9 + .../tst-rsrc/org/eclipse/jgit/diff/crlf2.patch | 9 + .../tst-rsrc/org/eclipse/jgit/diff/crlf2_PostImage | 3 + .../tst-rsrc/org/eclipse/jgit/diff/crlf2_PreImage | 3 + .../tst-rsrc/org/eclipse/jgit/diff/crlf3.patch | 8 + .../tst-rsrc/org/eclipse/jgit/diff/crlf3_PostImage | 3 + .../tst-rsrc/org/eclipse/jgit/diff/crlf3_PreImage | 0 .../tst-rsrc/org/eclipse/jgit/diff/crlf4.patch | 9 + .../tst-rsrc/org/eclipse/jgit/diff/crlf4_PostImage | 3 + .../tst-rsrc/org/eclipse/jgit/diff/crlf_PostImage | 3 + .../tst-rsrc/org/eclipse/jgit/diff/crlf_PreImage | 3 + .../org/eclipse/jgit/diff/smudgetest.patch | 9 + .../org/eclipse/jgit/diff/smudgetest_PostImage | 3 + .../org/eclipse/jgit/diff/smudgetest_PreImage | 3 + .../tst/org/eclipse/jgit/api/ApplyCommandTest.java | 191 +++++++++++++- .../src/org/eclipse/jgit/api/ApplyCommand.java | 279 +++++++++++++++++++-- 16 files changed, 513 insertions(+), 25 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf2_PreImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf3_PreImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf4_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/crlf_PreImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/smudgetest_PreImage (limited to 'org.eclipse.jgit/src/org/eclipse') 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 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 { @@ -57,6 +63,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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java index e228e8276a..5d975ea389 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.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 @@ -9,10 +9,16 @@ */ package org.eclipse.jgit.api; +import java.io.BufferedWriter; import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.Writer; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.text.MessageFormat; @@ -20,18 +26,45 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import org.eclipse.jgit.api.errors.FilterFailedException; import org.eclipse.jgit.api.errors.GitAPIException; 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.FilterCommandRegistry; import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.diff.RawText; +import org.eclipse.jgit.dircache.DirCache; +import org.eclipse.jgit.dircache.DirCacheCheckout; +import org.eclipse.jgit.dircache.DirCacheCheckout.CheckoutMetadata; +import org.eclipse.jgit.dircache.DirCacheIterator; +import org.eclipse.jgit.errors.LargeObjectException; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.CoreConfig.EolStreamType; import org.eclipse.jgit.lib.FileMode; +import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.lib.ObjectStream; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.patch.FileHeader; import org.eclipse.jgit.patch.HunkHeader; import org.eclipse.jgit.patch.Patch; +import org.eclipse.jgit.treewalk.FileTreeIterator; +import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.treewalk.TreeWalk.OperationType; +import org.eclipse.jgit.treewalk.filter.AndTreeFilter; +import org.eclipse.jgit.treewalk.filter.NotIgnoredFilter; +import org.eclipse.jgit.treewalk.filter.PathFilterGroup; +import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; +import org.eclipse.jgit.util.IO; +import org.eclipse.jgit.util.RawParseUtils; +import org.eclipse.jgit.util.StringUtils; +import org.eclipse.jgit.util.TemporaryBuffer; +import org.eclipse.jgit.util.FS.ExecutionResult; +import org.eclipse.jgit.util.TemporaryBuffer.LocalFile; +import org.eclipse.jgit.util.io.EolStreamTypeUtil; /** * Apply a patch to files and/or to the index. @@ -45,7 +78,7 @@ public class ApplyCommand extends GitCommand { private InputStream in; /** - * Constructs the command if the patch is to be applied to the index. + * Constructs the command. * * @param repo */ @@ -79,6 +112,7 @@ public class ApplyCommand extends GitCommand { public ApplyResult call() throws GitAPIException, PatchFormatException, PatchApplyException { checkCallable(); + setCallable(false); ApplyResult r = new ApplyResult(); try { final Patch p = new Patch(); @@ -87,19 +121,22 @@ public class ApplyCommand extends GitCommand { } finally { in.close(); } - if (!p.getErrors().isEmpty()) + if (!p.getErrors().isEmpty()) { throw new PatchFormatException(p.getErrors()); + } + Repository repository = getRepository(); + DirCache cache = repository.readDirCache(); for (FileHeader fh : p.getFiles()) { ChangeType type = fh.getChangeType(); File f = null; switch (type) { case ADD: f = getFile(fh.getNewPath(), true); - apply(f, fh); + apply(repository, fh.getNewPath(), cache, f, fh); break; case MODIFY: f = getFile(fh.getOldPath(), false); - apply(f, fh); + apply(repository, fh.getOldPath(), cache, f, fh); break; case DELETE: f = getFile(fh.getOldPath(), false); @@ -118,14 +155,14 @@ public class ApplyCommand extends GitCommand { throw new PatchApplyException(MessageFormat.format( JGitText.get().renameFileFailed, f, dest), e); } - apply(dest, fh); + apply(repository, fh.getOldPath(), cache, dest, fh); break; case COPY: f = getFile(fh.getOldPath(), false); File target = getFile(fh.getNewPath(), false); FileUtils.mkdirs(target.getParentFile(), true); Files.copy(f.toPath(), target.toPath()); - apply(target, fh); + apply(repository, fh.getOldPath(), cache, target, fh); } r.addUpdatedFile(f); } @@ -133,14 +170,13 @@ public class ApplyCommand extends GitCommand { throw new PatchApplyException(MessageFormat.format( JGitText.get().patchApplyException, e.getMessage()), e); } - setCallable(false); return r; } private File getFile(String path, boolean create) throws PatchApplyException { File f = new File(getRepository().getWorkTree(), path); - if (create) + if (create) { try { File parent = f.getParentFile(); FileUtils.mkdirs(parent, true); @@ -149,21 +185,201 @@ public class ApplyCommand extends GitCommand { throw new PatchApplyException(MessageFormat.format( JGitText.get().createNewFileFailed, f), e); } + } return f; } + private void apply(Repository repository, String path, DirCache cache, + File f, FileHeader fh) throws IOException, PatchApplyException { + boolean convertCrLf = needsCrLfConversion(f, fh); + // Use a TreeWalk with a DirCacheIterator to pick up the correct + // clean/smudge filters. CR-LF handling is completely determined by + // whether the file or the patch have CR-LF line endings. + try (TreeWalk walk = new TreeWalk(repository)) { + walk.setOperationType(OperationType.CHECKIN_OP); + FileTreeIterator files = new FileTreeIterator(repository); + int fileIdx = walk.addTree(files); + int cacheIdx = walk.addTree(new DirCacheIterator(cache)); + files.setDirCacheIterator(walk, cacheIdx); + walk.setFilter(AndTreeFilter.create( + PathFilterGroup.createFromStrings(path), + new NotIgnoredFilter(fileIdx))); + walk.setRecursive(true); + if (walk.next()) { + // If the file on disk has no newline characters, convertCrLf + // will be false. In that case we want to honor the normal git + // settings. + EolStreamType streamType = convertCrLf ? EolStreamType.TEXT_CRLF + : walk.getEolStreamType(OperationType.CHECKOUT_OP); + String command = walk.getFilterCommand( + Constants.ATTR_FILTER_TYPE_SMUDGE); + CheckoutMetadata checkOut = new CheckoutMetadata(streamType, command); + FileTreeIterator file = walk.getTree(fileIdx, + FileTreeIterator.class); + if (file != null) { + command = walk + .getFilterCommand(Constants.ATTR_FILTER_TYPE_CLEAN); + RawText raw; + // Can't use file.openEntryStream() as it would do CR-LF + // conversion as usual, not as wanted by us. + try (InputStream input = filterClean(repository, path, + new FileInputStream(f), convertCrLf, command)) { + raw = new RawText(IO.readWholeStream(input, 0).array()); + } + apply(repository, path, raw, f, fh, checkOut); + return; + } + } + } + // File ignored? + RawText raw; + CheckoutMetadata checkOut; + if (convertCrLf) { + try (InputStream input = EolStreamTypeUtil.wrapInputStream( + new FileInputStream(f), EolStreamType.TEXT_LF)) { + raw = new RawText(IO.readWholeStream(input, 0).array()); + } + checkOut = new CheckoutMetadata(EolStreamType.TEXT_CRLF, null); + } else { + raw = new RawText(f); + checkOut = new CheckoutMetadata(EolStreamType.DIRECT, null); + } + apply(repository, path, raw, f, fh, checkOut); + } + + private boolean needsCrLfConversion(File f, FileHeader fileHeader) + throws IOException { + if (!hasCrLf(fileHeader)) { + try (InputStream input = new FileInputStream(f)) { + return RawText.isCrLfText(input); + } + } + return false; + } + + private static boolean hasCrLf(FileHeader fileHeader) { + if (fileHeader == null) { + return false; + } + for (HunkHeader header : fileHeader.getHunks()) { + byte[] buf = header.getBuffer(); + int hunkEnd = header.getEndOffset(); + int lineStart = header.getStartOffset(); + while (lineStart < hunkEnd) { + int nextLineStart = RawParseUtils.nextLF(buf, lineStart); + if (nextLineStart > hunkEnd) { + nextLineStart = hunkEnd; + } + if (nextLineStart <= lineStart) { + break; + } + if (nextLineStart - lineStart > 1) { + char first = (char) (buf[lineStart] & 0xFF); + if (first == ' ' || first == '-') { + // It's an old line. Does it end in CR-LF? + if (buf[nextLineStart - 2] == '\r') { + return true; + } + } + } + lineStart = nextLineStart; + } + } + return false; + } + + private InputStream filterClean(Repository repository, String path, + InputStream fromFile, boolean convertCrLf, String filterCommand) + throws IOException { + InputStream input = fromFile; + if (convertCrLf) { + input = EolStreamTypeUtil.wrapInputStream(input, + EolStreamType.TEXT_LF); + } + if (StringUtils.isEmptyOrNull(filterCommand)) { + return input; + } + if (FilterCommandRegistry.isRegistered(filterCommand)) { + LocalFile buffer = new TemporaryBuffer.LocalFile(null); + FilterCommand command = FilterCommandRegistry.createFilterCommand( + filterCommand, repository, input, buffer); + while (command.run() != -1) { + // loop as long as command.run() tells there is work to do + } + return buffer.openInputStreamWithAutoDestroy(); + } + FS fs = repository.getFS(); + ProcessBuilder filterProcessBuilder = fs.runInShell(filterCommand, + new String[0]); + filterProcessBuilder.directory(repository.getWorkTree()); + filterProcessBuilder.environment().put(Constants.GIT_DIR_KEY, + repository.getDirectory().getAbsolutePath()); + ExecutionResult result; + try { + result = fs.execute(filterProcessBuilder, in); + } catch (IOException | InterruptedException e) { + throw new IOException( + new FilterFailedException(e, filterCommand, path)); + } + int rc = result.getRc(); + if (rc != 0) { + throw new IOException(new FilterFailedException(rc, filterCommand, + path, result.getStdout().toByteArray(4096), RawParseUtils + .decode(result.getStderr().toByteArray(4096)))); + } + return result.getStdout().openInputStreamWithAutoDestroy(); + } + /** - * @param f - * @param fh - * @throws IOException - * @throws PatchApplyException + * We write the patch result to a {@link TemporaryBuffer} and then use + * {@link DirCacheCheckout}.getContent() to run the result through the CR-LF + * and smudge filters. DirCacheCheckout needs an ObjectLoader, not a + * TemporaryBuffer, so this class bridges between the two, making the + * TemporaryBuffer look like an ordinary git blob to DirCacheCheckout. */ - private void apply(File f, FileHeader fh) + private static class BufferLoader extends ObjectLoader { + + private TemporaryBuffer data; + + BufferLoader(TemporaryBuffer data) { + this.data = data; + } + + @Override + public int getType() { + return Constants.OBJ_BLOB; + } + + @Override + public long getSize() { + return data.length(); + } + + @Override + public boolean isLarge() { + return true; + } + + @Override + public byte[] getCachedBytes() throws LargeObjectException { + throw new LargeObjectException(); + } + + @Override + public ObjectStream openStream() + throws MissingObjectException, IOException { + return new ObjectStream.Filter(getType(), getSize(), + data.openInputStream()); + } + } + + private void apply(Repository repository, String path, RawText rt, File f, + FileHeader fh, CheckoutMetadata checkOut) throws IOException, PatchApplyException { - RawText rt = new RawText(f); List oldLines = new ArrayList<>(rt.size()); - for (int i = 0; i < rt.size(); i++) + for (int i = 0; i < rt.size(); i++) { oldLines.add(rt.getString(i)); + } List newLines = new ArrayList<>(oldLines); int afterLastHunk = 0; int lineNumberShift = 0; @@ -279,17 +495,32 @@ public class ApplyCommand extends GitCommand { if (!isChanged(oldLines, newLines)) { return; // Don't touch the file } - try (Writer fw = Files.newBufferedWriter(f.toPath())) { - for (Iterator l = newLines.iterator(); l.hasNext();) { - fw.write(l.next()); - if (l.hasNext()) { - // Don't bother handling line endings - if it was Windows, - // the \r is still there! - fw.write('\n'); + + // TODO: forcing UTF-8 is a bit strange and may lead to re-coding if the + // input was some other encoding, but it's what previous versions of + // this code used. (Even earlier the code used the default encoding, + // which has the same problem.) Perhaps using bytes instead of Strings + // for the lines would be better. + TemporaryBuffer buffer = new TemporaryBuffer.LocalFile(null); + try { + try (Writer w = new BufferedWriter( + new OutputStreamWriter(buffer, StandardCharsets.UTF_8))) { + for (Iterator l = newLines.iterator(); l.hasNext();) { + w.write(l.next()); + if (l.hasNext()) { + w.write('\n'); + } } } + try (OutputStream output = new FileOutputStream(f)) { + DirCacheCheckout.getContent(repository, path, checkOut, + new BufferLoader(buffer), null, output); + } + } finally { + buffer.destroy(); } - getRepository().getFS().setExecute(f, fh.getNewMode() == FileMode.EXECUTABLE_FILE); + repository.getFS().setExecute(f, + fh.getNewMode() == FileMode.EXECUTABLE_FILE); } private boolean canApplyAt(List hunkLines, List newLines, -- cgit v1.2.3 From 303dd019d160a17fc93d148ca30bf78bfe976274 Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 12 May 2021 14:57:49 -0700 Subject: Optimize RefDirectory.isNameConflicting() Avoid having to scan over ALL loose refs to determine if the name is nested within or is a container of an existing reference. This can get really expensive if there are too many loose refs. Instead use exactRef and getRefsByPrefix which scan based on a prefix. With a simple shell script(like below) using jgit client to create 1k refs in a new repository on NFS, this change brings down the time from 12mins to 7mins. for ref in $(seq 1 1000); do jgit branch "$ref" done Here are few recorded elapsed times to create a new branch on NFS based repositories with varying loose refs count. As we see here, this change improves the name conflicting check from O(n^2) to O(1). loose_refs_count with_change without_change 50 44 ms 164 ms 300 45 ms 1193 ms 1k 38 ms 2610 ms 2k 44 ms 6003 ms 9k 46 ms 27860 ms 20k 45 ms 48591 ms 50k 51 ms 135471 ms 110k 43 ms 294252 ms 160k 52 ms 430976 ms Change-Id: Ie994fc184b8f82811bfb37b111eb9733dbe3e6e0 Signed-off-by: Kaushik Lingarkar --- .../jgit/internal/storage/file/RefDirectory.java | 35 ++-------------------- 1 file changed, 3 insertions(+), 32 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java index de7e4b3f25..f7a52a54b6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java @@ -276,47 +276,18 @@ public class RefDirectory extends RefDatabase { /** {@inheritDoc} */ @Override public boolean isNameConflicting(String name) throws IOException { - RefList packed = getPackedRefs(); - RefList loose = getLooseRefs(); - // Cannot be nested within an existing reference. int lastSlash = name.lastIndexOf('/'); while (0 < lastSlash) { String needle = name.substring(0, lastSlash); - if (loose.contains(needle) || packed.contains(needle)) + if (exactRef(needle) != null) { return true; + } lastSlash = name.lastIndexOf('/', lastSlash - 1); } // Cannot be the container of an existing reference. - String prefix = name + '/'; - int idx; - - idx = -(packed.find(prefix) + 1); - if (idx < packed.size() && packed.get(idx).getName().startsWith(prefix)) - return true; - - idx = -(loose.find(prefix) + 1); - if (idx < loose.size() && loose.get(idx).getName().startsWith(prefix)) - return true; - - return false; - } - - private RefList getLooseRefs() { - final RefList oldLoose = looseRefs.get(); - - LooseScanner scan = new LooseScanner(oldLoose); - scan.scan(ALL); - - RefList loose; - if (scan.newLoose != null) { - loose = scan.newLoose.toRefList(); - if (looseRefs.compareAndSet(oldLoose, loose)) - modCnt.incrementAndGet(); - } else - loose = oldLoose; - return loose; + return !getRefsByPrefix(name + '/').isEmpty(); } /** {@inheritDoc} */ -- cgit v1.2.3 From 8bc166b00da5fc74a659b42be779328a9508866b Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Wed, 12 May 2021 16:12:27 -0700 Subject: BatchRefUpdate: Skip saving conflicting ref names and prefixes in memory Rather than getting all ref names and prefixes and saving them in memory to perform the check for conflicting names, rely on RefDirectory.isNameConflicting as it is no longer an expensive call after it was optimized in Ie994fc. The old optimization to save ref names and prefixes in memory was targeted towards making clones faster. With this change, the clone performance is unaffected when tests were done with repos containing many(~500k) refs. Here are few recorded elapsed times for creating 10 branches using BatchRefUpdate on NFS based repositories with varying loose refs count. As seen here, this change helps improve the BatchRefUpdate performance from O(n^2) to O(1). loose_refs_count with_change without_change 50 241 ms 310 ms 300 263 ms 1502 ms 1k 181 ms 4241 ms 2k 204 ms 6440 ms 9k 158 ms 25930 ms 20k 154 ms 60443 ms 50k 171 ms 135199 ms 110k 157 ms 329450 ms 160k 209 ms 396328 ms This update improves the Gerrit notedb migration performance as it uses BatchRefUpdate to write change meta refs similar to the test performed above. Change-Id: I853ac6c7feb4b39c3156c01876b38cbd182accfe Signed-off-by: Kaushik Lingarkar --- .../src/org/eclipse/jgit/lib/BatchRefUpdate.java | 54 ++++++---------------- 1 file changed, 13 insertions(+), 41 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java index 925b6beadb..46d2ab3397 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java @@ -46,7 +46,6 @@ package org.eclipse.jgit.lib; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON; -import static java.util.stream.Collectors.toCollection; import java.io.IOException; import java.text.MessageFormat; @@ -62,7 +61,6 @@ import java.util.concurrent.TimeoutException; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.PushCertificate; import org.eclipse.jgit.transport.ReceiveCommand; @@ -528,42 +526,24 @@ public class BatchRefUpdate { } } if (!commands2.isEmpty()) { - // What part of the name space is already taken - Collection takenNames = refdb.getRefs().stream() - .map(Ref::getName) - .collect(toCollection(HashSet::new)); - Collection takenPrefixes = getTakenPrefixes(takenNames); - - // Now to the update that may require more room in the name space + // Perform updates that may require more room in the name space for (ReceiveCommand cmd : commands2) { try { if (cmd.getResult() == NOT_ATTEMPTED) { cmd.updateType(walk); RefUpdate ru = newUpdate(cmd); - SWITCH: switch (cmd.getType()) { - case DELETE: - // Performed in the first phase - break; - case UPDATE: - case UPDATE_NONFASTFORWARD: - RefUpdate ruu = newUpdate(cmd); - cmd.setResult(ruu.update(walk)); - break; - case CREATE: - for (String prefix : getPrefixes(cmd.getRefName())) { - if (takenNames.contains(prefix)) { - cmd.setResult(Result.LOCK_FAILURE); - break SWITCH; - } - } - if (takenPrefixes.contains(cmd.getRefName())) { - cmd.setResult(Result.LOCK_FAILURE); - break SWITCH; - } - ru.setCheckConflicting(false); - takenPrefixes.addAll(getPrefixes(cmd.getRefName())); - takenNames.add(cmd.getRefName()); - cmd.setResult(ru.update(walk)); + switch (cmd.getType()) { + case DELETE: + // Performed in the first phase + break; + case UPDATE: + case UPDATE_NONFASTFORWARD: + RefUpdate ruu = newUpdate(cmd); + cmd.setResult(ruu.update(walk)); + break; + case CREATE: + cmd.setResult(ru.update(walk)); + break; } } } catch (IOException err) { @@ -635,14 +615,6 @@ public class BatchRefUpdate { execute(walk, monitor, null); } - private static Collection getTakenPrefixes(Collection names) { - Collection ref = new HashSet<>(); - for (String name : names) { - addPrefixesTo(name, ref); - } - return ref; - } - /** * Get all path prefixes of a ref name. * -- cgit v1.2.3 From 501fc0dadde1b68a6c7bccd870e18cdf03d0e62c Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 5 Mar 2021 23:55:18 +0100 Subject: ApplyCommand: add a base-85 codec Add an implementation for base-85 encoding and decoding [1]. Git binary patches use this format. Base-85 encoding assembles bytes as 32-bit MSB values, then converts these values to base-85 numbers (always 5 bytes) encoded as printable ASCII characters. Decoding base-85 is the reverse operation. Note that decoding may overflow on invalid input as 85^5 > 2^32. Encodings always have a length that is a multiple of 5. If input length is not divisible by 4, padding bytes are (logically) added, which are ignored when decoding. The encoding for n bytes has thus always exactly length (n + 3) / 4 * 5 in integer arithmetic (truncating division). Includes tests. [1] https://datatracker.ietf.org/doc/html/rfc1924 Bug: 371725 Change-Id: Ib5b9a503cd62cf70e080a4fb38c8cd1eeeaebcfe Signed-off-by: Thomas Wolf Signed-off-by: Matthias Sohn --- .../tst/org/eclipse/jgit/util/Base85Test.java | 87 +++++++++ .../org/eclipse/jgit/internal/JGitText.properties | 5 + .../src/org/eclipse/jgit/internal/JGitText.java | 5 + .../src/org/eclipse/jgit/util/Base85.java | 195 +++++++++++++++++++++ 4 files changed, 292 insertions(+) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/util/Base85Test.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/util/Base85.java (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/Base85Test.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/Base85Test.java new file mode 100644 index 0000000000..a49878cc76 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/Base85Test.java @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.nio.charset.StandardCharsets; + +import org.junit.Test; + +/** + * Tests for {@link Base85}. + */ +public class Base85Test { + + private static final String VALID_CHARS = "0123456789" + + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + + "!#$%&()*+-;<=>?@^_`{|}~"; + + @Test + public void testChars() { + for (int i = 0; i < 256; i++) { + byte[] testData = { '1', '2', '3', '4', (byte) i }; + if (VALID_CHARS.indexOf(i) >= 0) { + byte[] decoded = Base85.decode(testData, 4); + assertNotNull(decoded); + } else { + assertThrows(IllegalArgumentException.class, + () -> Base85.decode(testData, 4)); + } + } + } + + private void roundtrip(byte[] data, int expectedLength) { + byte[] encoded = Base85.encode(data); + assertEquals(expectedLength, encoded.length); + assertArrayEquals(data, Base85.decode(encoded, data.length)); + } + + private void roundtrip(String data, int expectedLength) { + roundtrip(data.getBytes(StandardCharsets.US_ASCII), expectedLength); + } + + @Test + public void testPadding() { + roundtrip("", 0); + roundtrip("a", 5); + roundtrip("ab", 5); + roundtrip("abc", 5); + roundtrip("abcd", 5); + roundtrip("abcde", 10); + roundtrip("abcdef", 10); + roundtrip("abcdefg", 10); + roundtrip("abcdefgh", 10); + roundtrip("abcdefghi", 15); + } + + @Test + public void testBinary() { + roundtrip(new byte[] { 1 }, 5); + roundtrip(new byte[] { 1, 2 }, 5); + roundtrip(new byte[] { 1, 2, 3 }, 5); + roundtrip(new byte[] { 1, 2, 3, 4 }, 5); + roundtrip(new byte[] { 1, 2, 3, 4, 5 }, 10); + roundtrip(new byte[] { 1, 2, 3, 4, 5, 0, 0, 0 }, 10); + roundtrip(new byte[] { 1, 2, 3, 4, 0, 0, 0, 5 }, 10); + } + + @Test + public void testOverflow() { + IllegalArgumentException e = assertThrows( + IllegalArgumentException.class, + () -> Base85.decode(new byte[] { '~', '~', '~', '~', '~' }, 4)); + assertTrue(e.getMessage().contains("overflow")); + } +} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 2fa8713daa..6c4ca52ccf 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -37,6 +37,11 @@ badRef=Bad ref: {0}: {1} badSectionEntry=Bad section entry: {0} badShallowLine=Bad shallow line: {0} bareRepositoryNoWorkdirAndIndex=Bare Repository has neither a working tree, nor an index +base85invalidChar=Invalid base-85 character: 0x{0} +base85length=Base-85 encoded data must have a length that is a multiple of 5 +base85overflow=Base-85 value overflow, does not fit into 32 bits: 0x{0} +base85tooLong=Extra base-85 encoded data for output size of {0} bytes +base85tooShort=Base-85 data decoded into less than {0} bytes baseLengthIncorrect=base length incorrect bitmapMissingObject=Bitmap at {0} is missing {1}. bitmapsMustBePrepared=Bitmaps must be prepared before they may be written. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index ab9fc5c9bb..5c194f3413 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -65,6 +65,11 @@ public class JGitText extends TranslationBundle { /***/ public String badSectionEntry; /***/ public String badShallowLine; /***/ public String bareRepositoryNoWorkdirAndIndex; + /***/ public String base85invalidChar; + /***/ public String base85length; + /***/ public String base85overflow; + /***/ public String base85tooLong; + /***/ public String base85tooShort; /***/ public String baseLengthIncorrect; /***/ public String bitmapMissingObject; /***/ public String bitmapsMustBePrepared; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Base85.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Base85.java new file mode 100644 index 0000000000..54b7cfcaa7 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/Base85.java @@ -0,0 +1,195 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util; + +import java.nio.charset.StandardCharsets; +import java.text.MessageFormat; +import java.util.Arrays; + +import org.eclipse.jgit.internal.JGitText; + +/** + * Base-85 encoder/decoder. + * + * @since 5.12 + */ +public final class Base85 { + + private static final byte[] ENCODE = ("0123456789" //$NON-NLS-1$ + + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" //$NON-NLS-1$ + + "abcdefghijklmnopqrstuvwxyz" //$NON-NLS-1$ + + "!#$%&()*+-;<=>?@^_`{|}~") //$NON-NLS-1$ + .getBytes(StandardCharsets.US_ASCII); + + private static final int[] DECODE = new int[256]; + + static { + Arrays.fill(DECODE, -1); + for (int i = 0; i < ENCODE.length; i++) { + DECODE[ENCODE[i]] = i; + } + } + + private Base85() { + // No instantiation + } + + /** + * Determines the length of the base-85 encoding for {@code rawLength} + * bytes. + * + * @param rawLength + * number of bytes to encode + * @return number of bytes needed for the base-85 encoding of + * {@code rawLength} bytes + */ + public static int encodedLength(int rawLength) { + return (rawLength + 3) / 4 * 5; + } + + /** + * Encodes the given {@code data} in Base-85. + * + * @param data + * to encode + * @return encoded data + */ + public static byte[] encode(byte[] data) { + return encode(data, 0, data.length); + } + + /** + * Encodes {@code length} bytes of {@code data} in Base-85, beginning at the + * {@code start} index. + * + * @param data + * to encode + * @param start + * index of the first byte to encode + * @param length + * number of bytes to encode + * @return encoded data + */ + public static byte[] encode(byte[] data, int start, int length) { + byte[] result = new byte[encodedLength(length)]; + int end = start + length; + int in = start; + int out = 0; + while (in < end) { + // Accumulate remaining bytes MSB first as a 32bit value + long accumulator = ((long) (data[in++] & 0xFF)) << 24; + if (in < end) { + accumulator |= (data[in++] & 0xFF) << 16; + if (in < end) { + accumulator |= (data[in++] & 0xFF) << 8; + if (in < end) { + accumulator |= (data[in++] & 0xFF); + } + } + } + // Write the 32bit value in base-85 encoding, also MSB first + for (int i = 4; i >= 0; i--) { + result[out + i] = ENCODE[(int) (accumulator % 85)]; + accumulator /= 85; + } + out += 5; + } + return result; + } + + /** + * Decodes the Base-85 {@code encoded} data into a byte array of + * {@code expectedSize} bytes. + * + * @param encoded + * Base-85 encoded data + * @param expectedSize + * of the result + * @return the decoded bytes + * @throws IllegalArgumentException + * if expectedSize doesn't match, the encoded data has a length + * that is not a multiple of 5, or there are invalid characters + * in the encoded data + */ + public static byte[] decode(byte[] encoded, int expectedSize) { + return decode(encoded, 0, encoded.length, expectedSize); + } + + /** + * Decodes {@code length} bytes of Base-85 {@code encoded} data, beginning + * at the {@code start} index, into a byte array of {@code expectedSize} + * bytes. + * + * @param encoded + * Base-85 encoded data + * @param start + * index at which the data to decode starts in {@code encoded} + * @param length + * of the Base-85 encoded data + * @param expectedSize + * of the result + * @return the decoded bytes + * @throws IllegalArgumentException + * if expectedSize doesn't match, {@code length} is not a + * multiple of 5, or there are invalid characters in the encoded + * data + */ + public static byte[] decode(byte[] encoded, int start, int length, + int expectedSize) { + if (length % 5 != 0) { + throw new IllegalArgumentException(JGitText.get().base85length); + } + byte[] result = new byte[expectedSize]; + int end = start + length; + int in = start; + int out = 0; + while (in < end && out < expectedSize) { + // Accumulate 5 bytes, "MSB" first + long accumulator = 0; + for (int i = 4; i >= 0; i--) { + int val = DECODE[encoded[in++] & 0xFF]; + if (val < 0) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().base85invalidChar, + Integer.toHexString(encoded[in - 1] & 0xFF))); + } + accumulator = accumulator * 85 + val; + } + if (accumulator > 0xFFFF_FFFFL) { + throw new IllegalArgumentException( + MessageFormat.format(JGitText.get().base85overflow, + Long.toHexString(accumulator))); + } + // Write remaining bytes, MSB first + result[out++] = (byte) (accumulator >>> 24); + if (out < expectedSize) { + result[out++] = (byte) (accumulator >>> 16); + if (out < expectedSize) { + result[out++] = (byte) (accumulator >>> 8); + if (out < expectedSize) { + result[out++] = (byte) accumulator; + } + } + } + } + // Should have exhausted 'in' and filled 'out' completely + if (in < end) { + throw new IllegalArgumentException( + MessageFormat.format(JGitText.get().base85tooLong, + Integer.valueOf(expectedSize))); + } + if (out < expectedSize) { + throw new IllegalArgumentException( + MessageFormat.format(JGitText.get().base85tooShort, + Integer.valueOf(expectedSize))); + } + return result; + } +} -- cgit v1.2.3 From 2eb54afe6a5cb5dd2a108285ad1676b7798d5a15 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 6 Mar 2021 00:00:15 +0100 Subject: ApplyCommand: add streams to read/write binary patch hunks Add streams that can encode or decode git binary patch data on the fly. Git writes binary patches base-85 encoded, at most 52 un-encoded bytes, with the unencoded data length prefixed in a one-character encoding, and suffixed with a newline character. Add a test for both the new input and the output stream. The test roundtrips binary data of different lengths in different ways. Bug: 371725 Change-Id: Ic3faebaa4637520f5448b3d1acd78d5aaab3907a Signed-off-by: Thomas Wolf --- .../eclipse/jgit/util/io/BinaryHunkStreamTest.java | 146 +++++++++++++++++++++ .../org/eclipse/jgit/internal/JGitText.properties | 4 + .../src/org/eclipse/jgit/internal/JGitText.java | 4 + .../jgit/util/io/BinaryHunkInputStream.java | 113 ++++++++++++++++ .../jgit/util/io/BinaryHunkOutputStream.java | 116 ++++++++++++++++ 5 files changed, 383 insertions(+) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryHunkStreamTest.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkOutputStream.java (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryHunkStreamTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryHunkStreamTest.java new file mode 100644 index 0000000000..b198c32a78 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryHunkStreamTest.java @@ -0,0 +1,146 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util.io; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Arrays; + +import org.junit.Test; + +/** + * Tests for {@link BinaryHunkInputStream} and {@link BinaryHunkOutputStream}. + */ +public class BinaryHunkStreamTest { + + @Test + public void testRoundtripWholeBuffer() throws IOException { + for (int length = 1; length < 520 + 52; length++) { + byte[] data = new byte[length]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) (255 - (i % 256)); + } + try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); + BinaryHunkOutputStream out = new BinaryHunkOutputStream( + bos)) { + out.write(data); + out.flush(); + byte[] encoded = bos.toByteArray(); + assertFalse(Arrays.equals(data, encoded)); + try (BinaryHunkInputStream in = new BinaryHunkInputStream( + new ByteArrayInputStream(encoded))) { + byte[] decoded = new byte[data.length]; + int newLength = in.read(decoded); + assertEquals(newLength, decoded.length); + assertEquals(-1, in.read()); + assertArrayEquals(data, decoded); + } + } + } + } + + @Test + public void testRoundtripChunks() throws IOException { + for (int length = 1; length < 520 + 52; length++) { + byte[] data = new byte[length]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) (255 - (i % 256)); + } + try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); + BinaryHunkOutputStream out = new BinaryHunkOutputStream( + bos)) { + out.write(data, 0, data.length / 2); + out.write(data, data.length / 2, data.length - data.length / 2); + out.flush(); + byte[] encoded = bos.toByteArray(); + assertFalse(Arrays.equals(data, encoded)); + try (BinaryHunkInputStream in = new BinaryHunkInputStream( + new ByteArrayInputStream(encoded))) { + byte[] decoded = new byte[data.length]; + int p = 0; + int n; + while ((n = in.read(decoded, p, + Math.min(decoded.length - p, 57))) >= 0) { + p += n; + if (p == decoded.length) { + break; + } + } + assertEquals(p, decoded.length); + assertEquals(-1, in.read()); + assertArrayEquals(data, decoded); + } + } + } + } + + @Test + public void testRoundtripBytes() throws IOException { + for (int length = 1; length < 520 + 52; length++) { + byte[] data = new byte[length]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) (255 - (i % 256)); + } + try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); + BinaryHunkOutputStream out = new BinaryHunkOutputStream( + bos)) { + for (int i = 0; i < data.length; i++) { + out.write(data[i]); + } + out.flush(); + byte[] encoded = bos.toByteArray(); + assertFalse(Arrays.equals(data, encoded)); + try (BinaryHunkInputStream in = new BinaryHunkInputStream( + new ByteArrayInputStream(encoded))) { + byte[] decoded = new byte[data.length]; + for (int i = 0; i < decoded.length; i++) { + int val = in.read(); + assertTrue(0 <= val && val <= 255); + decoded[i] = (byte) val; + } + assertEquals(-1, in.read()); + assertArrayEquals(data, decoded); + } + } + } + } + + @Test + public void testRoundtripWithClose() throws IOException { + for (int length = 1; length < 520 + 52; length++) { + byte[] data = new byte[length]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) (255 - (i % 256)); + } + try (ByteArrayOutputStream bos = new ByteArrayOutputStream()) { + try (BinaryHunkOutputStream out = new BinaryHunkOutputStream( + bos)) { + out.write(data); + } + byte[] encoded = bos.toByteArray(); + assertFalse(Arrays.equals(data, encoded)); + try (BinaryHunkInputStream in = new BinaryHunkInputStream( + new ByteArrayInputStream(encoded))) { + byte[] decoded = new byte[data.length]; + int newLength = in.read(decoded); + assertEquals(newLength, decoded.length); + assertEquals(-1, in.read()); + assertArrayEquals(data, decoded); + } + } + } + } +} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 6c4ca52ccf..f8c9ea0dcc 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -43,6 +43,10 @@ base85overflow=Base-85 value overflow, does not fit into 32 bits: 0x{0} base85tooLong=Extra base-85 encoded data for output size of {0} bytes base85tooShort=Base-85 data decoded into less than {0} bytes baseLengthIncorrect=base length incorrect +binaryHunkDecodeError=Binary hunk, line {0}: invalid input +binaryHunkInvalidLength=Binary hunk, line {0}: input corrupt; expected length byte, got 0x{1} +binaryHunkLineTooShort=Binary hunk, line {0}: input ended prematurely +binaryHunkMissingNewline=Binary hunk, line {0}: input line not terminated by newline bitmapMissingObject=Bitmap at {0} is missing {1}. bitmapsMustBePrepared=Bitmaps must be prepared before they may be written. blameNotCommittedYet=Not Committed Yet diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 5c194f3413..85b40cb6dc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -71,6 +71,10 @@ public class JGitText extends TranslationBundle { /***/ public String base85tooLong; /***/ public String base85tooShort; /***/ public String baseLengthIncorrect; + /***/ public String binaryHunkDecodeError; + /***/ public String binaryHunkInvalidLength; + /***/ public String binaryHunkLineTooShort; + /***/ public String binaryHunkMissingNewline; /***/ public String bitmapMissingObject; /***/ public String bitmapsMustBePrepared; /***/ public String blameNotCommittedYet; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java new file mode 100644 index 0000000000..57b2d7a4b2 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java @@ -0,0 +1,113 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util.io; + +import java.io.EOFException; +import java.io.IOException; +import java.io.InputStream; +import java.io.StreamCorruptedException; +import java.text.MessageFormat; + +import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.util.Base85; + +/** + * A stream that decodes git binary patch data on the fly. + * + * @since 5.12 + */ +public class BinaryHunkInputStream extends InputStream { + + private final InputStream in; + + private int lineNumber; + + private byte[] buffer; + + private int pos = 0; + + /** + * Creates a new {@link BinaryHunkInputStream}. + * + * @param in + * {@link InputStream} to read the base-85 encoded patch data + * from + */ + public BinaryHunkInputStream(InputStream in) { + this.in = in; + } + + @Override + public int read() throws IOException { + if (pos < 0) { + return -1; + } + if (buffer == null || pos == buffer.length) { + fillBuffer(); + } + if (pos >= 0) { + return buffer[pos++] & 0xFF; + } + return -1; + } + + @Override + public void close() throws IOException { + in.close(); + buffer = null; + } + + private void fillBuffer() throws IOException { + int length = in.read(); + if (length < 0) { + pos = length; + buffer = null; + return; + } + lineNumber++; + // Length is encoded with characters, A..Z for 1..26 and a..z for 27..52 + if ('A' <= length && length <= 'Z') { + length = length - 'A' + 1; + } else if ('a' <= length && length <= 'z') { + length = length - 'a' + 27; + } else { + throw new StreamCorruptedException(MessageFormat.format( + JGitText.get().binaryHunkInvalidLength, + Integer.valueOf(lineNumber), Integer.toHexString(length))); + } + byte[] encoded = new byte[Base85.encodedLength(length)]; + for (int i = 0; i < encoded.length; i++) { + int b = in.read(); + if (b < 0 || b == '\n') { + throw new EOFException(MessageFormat.format( + JGitText.get().binaryHunkInvalidLength, + Integer.valueOf(lineNumber))); + } + encoded[i] = (byte) b; + } + // Must be followed by a newline; tolerate EOF. + int b = in.read(); + if (b >= 0 && b != '\n') { + throw new StreamCorruptedException(MessageFormat.format( + JGitText.get().binaryHunkMissingNewline, + Integer.valueOf(lineNumber))); + } + try { + buffer = Base85.decode(encoded, length); + } catch (IllegalArgumentException e) { + StreamCorruptedException ex = new StreamCorruptedException( + MessageFormat.format(JGitText.get().binaryHunkDecodeError, + Integer.valueOf(lineNumber))); + ex.initCause(e); + throw ex; + } + pos = 0; + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkOutputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkOutputStream.java new file mode 100644 index 0000000000..30551c09fd --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkOutputStream.java @@ -0,0 +1,116 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util.io; + +import java.io.IOException; +import java.io.OutputStream; + +import org.eclipse.jgit.util.Base85; + +/** + * An {@link OutputStream} that encodes data for a git binary patch. + * + * @since 5.12 + */ +public class BinaryHunkOutputStream extends OutputStream { + + private static final int MAX_BYTES = 52; + + private final OutputStream out; + + private final byte[] buffer = new byte[MAX_BYTES]; + + private int pos; + + /** + * Creates a new {@link BinaryHunkOutputStream}. + * + * @param out + * {@link OutputStream} to write the encoded data to + */ + public BinaryHunkOutputStream(OutputStream out) { + this.out = out; + } + + /** + * Flushes and closes this stream, and closes the underlying + * {@link OutputStream}. + */ + @Override + public void close() throws IOException { + flush(); + out.close(); + } + + /** + * Writes any buffered output as a binary patch line to the underlying + * {@link OutputStream} and flushes that stream, too. + */ + @Override + public void flush() throws IOException { + if (pos > 0) { + encode(buffer, 0, pos); + pos = 0; + } + out.flush(); + } + + @Override + public void write(int b) throws IOException { + buffer[pos++] = (byte) b; + if (pos == buffer.length) { + encode(buffer, 0, pos); + pos = 0; + } + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + if (len == 0) { + return; + } + int toCopy = len; + int in = off; + if (pos > 0) { + // Fill the buffer + int chunk = Math.min(toCopy, buffer.length - pos); + System.arraycopy(b, in, buffer, pos, chunk); + in += chunk; + pos += chunk; + toCopy -= chunk; + if (pos == buffer.length) { + encode(buffer, 0, pos); + pos = 0; + } + if (toCopy == 0) { + return; + } + } + while (toCopy >= MAX_BYTES) { + encode(b, in, MAX_BYTES); + toCopy -= MAX_BYTES; + in += MAX_BYTES; + } + if (toCopy > 0) { + System.arraycopy(b, in, buffer, 0, toCopy); + pos = toCopy; + } + } + + private void encode(byte[] data, int off, int length) throws IOException { + if (length <= 26) { + out.write('A' + length - 1); + } else { + out.write('a' + length - 27); + } + out.write(Base85.encode(data, off, length)); + out.write('\n'); + } +} -- cgit v1.2.3 From 0fe794a433d54504de066ae119b5835ab69c1c54 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 7 Mar 2021 18:49:01 +0100 Subject: ApplyCommand: add a stream to apply a delta patch Add a new BinaryDeltaInputStream that applies a delta provided by another InputStream to a given base. Because delta application needs random access to the base, the base itself cannot be yet another InputStream. But at least this enables streaming of the result. Add a simple test using delta hunks generated by C git. Bug: 371725 Change-Id: Ibd26fa2f49860737ad5c5387f7f4870d3e85e628 Signed-off-by: Thomas Wolf Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/util/io/delta1.forward | 1 + .../org/eclipse/jgit/util/io/delta1.reverse | 1 + .../jgit/util/io/BinaryDeltaInputStreamTest.java | 103 +++++++++++ .../org/eclipse/jgit/internal/JGitText.properties | 3 + .../src/org/eclipse/jgit/internal/JGitText.java | 3 + .../jgit/util/io/BinaryDeltaInputStream.java | 206 +++++++++++++++++++++ 6 files changed, 317 insertions(+) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.forward create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.reverse create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryDeltaInputStreamTest.java create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.forward b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.forward new file mode 100644 index 0000000000..878b167ae9 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.forward @@ -0,0 +1 @@ +ScmZp0Xmwa1z*+$U3j_csN(Dmz diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.reverse b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.reverse new file mode 100644 index 0000000000..7ff7a08ad0 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/util/io/delta1.reverse @@ -0,0 +1 @@ +TcmZp5XmD5{u!xa=5hEi28?FP4 diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryDeltaInputStreamTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryDeltaInputStreamTest.java new file mode 100644 index 0000000000..d9297fcd9c --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/io/BinaryDeltaInputStreamTest.java @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util.io; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.util.zip.InflaterInputStream; + +import org.junit.Test; + +/** + * Crude tests for the {@link BinaryDeltaInputStream} using delta diffs + * generated by C git. + */ +public class BinaryDeltaInputStreamTest { + + private InputStream getBinaryHunk(String name) { + return this.getClass().getResourceAsStream(name); + } + + @Test + public void testBinaryDelta() throws Exception { + // Prepare our test data + byte[] data = new byte[8192]; + for (int i = 0; i < data.length; i++) { + data[i] = (byte) (255 - (i % 256)); + } + // Same, but with five 'x' inserted in the middle. + int middle = data.length / 2; + byte[] newData = new byte[data.length + 5]; + System.arraycopy(data, 0, newData, 0, middle); + for (int i = 0; i < 5; i++) { + newData[middle + i] = 'x'; + } + System.arraycopy(data, middle, newData, middle + 5, middle); + // delta1.forward has the instructions + // @formatter:off + // COPY 0 4096 + // INSERT 5 xxxxx + // COPY 0 4096 + // @formatter:on + // Note that the way we built newData could be expressed as + // @formatter:off + // COPY 0 4096 + // INSERT 5 xxxxx + // COPY 4096 4096 + // @formatter:on + try (ByteArrayOutputStream out = new ByteArrayOutputStream(); + BinaryDeltaInputStream input = new BinaryDeltaInputStream(data, + new InflaterInputStream(new BinaryHunkInputStream( + getBinaryHunk("delta1.forward"))))) { + byte[] buf = new byte[1024]; + int n; + while ((n = input.read(buf)) >= 0) { + out.write(buf, 0, n); + } + assertArrayEquals(newData, out.toByteArray()); + assertTrue(input.isFullyConsumed()); + } + // delta1.reverse has the instructions + // @formatter:off + // COPY 0 4096 + // COPY 256 3840 + // COPY 256 256 + // @formatter:on + // Note that there are alternatives, for instance + // @formatter:off + // COPY 0 4096 + // COPY 4101 4096 + // @formatter:on + // or + // @formatter:off + // COPY 0 4096 + // COPY 0 4096 + // @formatter:on + try (ByteArrayOutputStream out = new ByteArrayOutputStream(); + BinaryDeltaInputStream input = new BinaryDeltaInputStream( + newData, + new InflaterInputStream(new BinaryHunkInputStream( + getBinaryHunk("delta1.reverse"))))) { + long expectedSize = input.getExpectedResultSize(); + assertEquals(data.length, expectedSize); + byte[] buf = new byte[1024]; + int n; + while ((n = input.read(buf)) >= 0) { + out.write(buf, 0, n); + } + assertArrayEquals(data, out.toByteArray()); + assertTrue(input.isFullyConsumed()); + } + } +} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index f8c9ea0dcc..8b6872d337 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -43,6 +43,9 @@ base85overflow=Base-85 value overflow, does not fit into 32 bits: 0x{0} base85tooLong=Extra base-85 encoded data for output size of {0} bytes base85tooShort=Base-85 data decoded into less than {0} bytes baseLengthIncorrect=base length incorrect +binaryDeltaBaseLengthMismatch=Binary delta base length does not match, expected {0}, got {1} +binaryDeltaInvalidOffset=Binary delta offset + length too large: {0} + {1} +binaryDeltaInvalidResultLength=Binary delta expected result length is negative binaryHunkDecodeError=Binary hunk, line {0}: invalid input binaryHunkInvalidLength=Binary hunk, line {0}: input corrupt; expected length byte, got 0x{1} binaryHunkLineTooShort=Binary hunk, line {0}: input ended prematurely diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 85b40cb6dc..3b67b0f628 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -71,6 +71,9 @@ public class JGitText extends TranslationBundle { /***/ public String base85tooLong; /***/ public String base85tooShort; /***/ public String baseLengthIncorrect; + /***/ public String binaryDeltaBaseLengthMismatch; + /***/ public String binaryDeltaInvalidOffset; + /***/ public String binaryDeltaInvalidResultLength; /***/ public String binaryHunkDecodeError; /***/ public String binaryHunkInvalidLength; /***/ public String binaryHunkLineTooShort; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java new file mode 100644 index 0000000000..7f0d87f0da --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java @@ -0,0 +1,206 @@ +/* + * Copyright (C) 2021 Thomas Wolf 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 + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.util.io; + +import java.io.EOFException; +import java.io.IOException; +import java.io.InputStream; +import java.io.StreamCorruptedException; +import java.text.MessageFormat; + +import org.eclipse.jgit.internal.JGitText; + +/** + * An {@link InputStream} that applies a binary delta to a base on the fly. + *

+ * Delta application to a base needs random access to the base data. The delta + * is expressed as a sequence of copy and insert instructions. A copy + * instruction has the form "COPY fromOffset length" and says "copy length bytes + * from the base, starting at offset fromOffset, to the result". An insert + * instruction has the form "INSERT length" followed by length bytes and says + * "copy the next length bytes from the delta to the result". + *

+ *

+ * These instructions are generated using a content-defined chunking algorithm + * (currently C git uses the standard Rabin variant; but there are others that + * could be used) that identifies equal chunks. It is entirely possible that a + * later copy instruction has a fromOffset that is before the fromOffset of an + * earlier copy instruction. + *

+ *

+ * This makes it impossible to stream the base. + *

+ *

+ * JGit is limited to 2GB maximum size for the base since array indices are + * signed 32bit values. + * + * @since 5.12 + */ +public class BinaryDeltaInputStream extends InputStream { + + private final byte[] base; + + private final InputStream delta; + + private long resultLength; + + private long toDeliver = -1; + + private int fromBase; + + private int fromDelta; + + private int baseOffset = -1; + + /** + * Creates a new {@link BinaryDeltaInputStream} that applies {@code delta} + * to {@code base}. + * + * @param base + * data to apply the delta to + * @param delta + * {@link InputStream} delivering the delta to apply + */ + public BinaryDeltaInputStream(byte[] base, InputStream delta) { + this.base = base; + this.delta = delta; + } + + @Override + public int read() throws IOException { + int b = readNext(); + if (b >= 0) { + toDeliver--; + } + return b; + } + + private void initialize() throws IOException { + long baseSize = readVarInt(delta); + if (baseSize > Integer.MAX_VALUE || baseSize < 0 + || (int) baseSize != base.length) { + throw new IOException(MessageFormat.format( + JGitText.get().binaryDeltaBaseLengthMismatch, + Integer.valueOf(base.length), Long.valueOf(baseSize))); + } + resultLength = readVarInt(delta); + if (resultLength < 0) { + throw new StreamCorruptedException( + JGitText.get().binaryDeltaInvalidResultLength); + } + toDeliver = resultLength; + baseOffset = 0; + } + + private int readNext() throws IOException { + if (baseOffset < 0) { + initialize(); + } + if (fromBase > 0) { + fromBase--; + return base[baseOffset++] & 0xFF; + } else if (fromDelta > 0) { + fromDelta--; + return delta.read(); + } + int command = delta.read(); + if (command < 0) { + return -1; + } + if ((command & 0x80) != 0) { + // Decode offset and length to read from base + long copyOffset = 0; + for (int i = 1, shift = 0; i < 0x10; i *= 2, shift += 8) { + if ((command & i) != 0) { + copyOffset |= ((long) next(delta)) << shift; + } + } + int copySize = 0; + for (int i = 0x10, shift = 0; i < 0x80; i *= 2, shift += 8) { + if ((command & i) != 0) { + copySize |= next(delta) << shift; + } + } + if (copySize == 0) { + copySize = 0x10000; + } + if (copyOffset > base.length - copySize) { + throw new StreamCorruptedException(MessageFormat.format( + JGitText.get().binaryDeltaInvalidOffset, + Long.valueOf(copyOffset), Integer.valueOf(copySize))); + } + baseOffset = (int) copyOffset; + fromBase = copySize; + return readNext(); + } else if (command != 0) { + // The next 'command' bytes come from the delta + fromDelta = command - 1; + return delta.read(); + } else { + // Zero is reserved + throw new StreamCorruptedException( + JGitText.get().unsupportedCommand0); + } + } + + private int next(InputStream in) throws IOException { + int b = in.read(); + if (b < 0) { + throw new EOFException(); + } + return b; + } + + private long readVarInt(InputStream in) throws IOException { + long val = 0; + int shift = 0; + int b; + do { + b = next(in); + val |= ((long) (b & 0x7f)) << shift; + shift += 7; + } while ((b & 0x80) != 0); + return val; + } + + /** + * Tells the expected size of the final result. + * + * @return the size + * @throws IOException + * if the size cannot be determined from {@code delta} + */ + public long getExpectedResultSize() throws IOException { + if (baseOffset < 0) { + initialize(); + } + return resultLength; + } + + /** + * Tells whether the delta has been fully consumed, and the expected number + * of bytes for the combined result have been read from this + * {@link BinaryDeltaInputStream}. + * + * @return whether delta application was successful + */ + public boolean isFullyConsumed() { + try { + return toDeliver == 0 && delta.read() < 0; + } catch (IOException e) { + return toDeliver == 0; + } + } + + @Override + public void close() throws IOException { + delta.close(); + } +} -- cgit v1.2.3 From 10ac4499115965ff10e547a0632c89873a06cf91 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 7 Mar 2021 18:50:23 +0100 Subject: ApplyCommand: support binary patches Implement applying binary patches. Handles both literal and delta patches. Note that C git also runs binary files through the clean and smudge filters. Implement the same safeguards against corrupted patches as in C git: require the full OIDs to be present in the patch file, and apply a binary patch only if both pre- and post-image hashes match. Add tests for applying literal and delta patches. Bug: 371725 Change-Id: I71dc214fe4145d7cc8e4769384fb78c7d0d6c220 Signed-off-by: Thomas Wolf --- .../tst-rsrc/org/eclipse/jgit/diff/.gitattributes | 2 + .../tst-rsrc/org/eclipse/jgit/diff/delta.patch | 8 + .../tst-rsrc/org/eclipse/jgit/diff/delta_PostImage | Bin 0 -> 8197 bytes .../tst-rsrc/org/eclipse/jgit/diff/delta_PreImage | Bin 0 -> 8192 bytes .../tst-rsrc/org/eclipse/jgit/diff/literal.patch | 126 ++++++++++ .../org/eclipse/jgit/diff/literal_PostImage | Bin 0 -> 5389 bytes .../org/eclipse/jgit/diff/literal_PreImage | Bin 0 -> 1629 bytes .../org/eclipse/jgit/diff/literal_add.patch | 40 +++ .../org/eclipse/jgit/diff/literal_add_PostImage | Bin 0 -> 1629 bytes .../tst/org/eclipse/jgit/api/ApplyCommandTest.java | 41 +++ .../org/eclipse/jgit/internal/JGitText.properties | 3 + .../src/org/eclipse/jgit/api/ApplyCommand.java | 274 ++++++++++++++++++--- .../src/org/eclipse/jgit/internal/JGitText.java | 3 + 13 files changed, 467 insertions(+), 30 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PreImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PreImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add_PostImage (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/.gitattributes b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/.gitattributes index c5831d9a8b..28caa2f82a 100644 --- a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/.gitattributes +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/.gitattributes @@ -1,3 +1,5 @@ *.patch -crlf *Image -crlf *.out -crlf +delta* -text +literal* -text diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta.patch new file mode 100644 index 0000000000..84855308a5 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta.patch @@ -0,0 +1,8 @@ +diff --git a/delta b/delta +index b4527005bf9e4da2dd1d7185b51bdac623a4218f..8b370bb5f2bc3261b6b62e80d6edd784a61ec225 100644 +GIT binary patch +delta 14 +ScmZp0Xmwa1z*+$U3j_csN(Dmz + +delta 12 +TcmZp5XmD5{u!xa=5hEi28?FP4 diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PostImage new file mode 100644 index 0000000000..8b370bb5f2 Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PostImage differ diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PreImage new file mode 100644 index 0000000000..b4527005bf Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/delta_PreImage differ diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal.patch new file mode 100644 index 0000000000..c8811d5bb0 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal.patch @@ -0,0 +1,126 @@ +diff --git a/literal b/literal +index 799df8578e3cae8a5e979182391b8e9a6a3deded..74e4201af6378ac87bf9e118fbab6553c7fd355c 100644 +GIT binary patch +literal 5389 +zcmc&&X;f25x2=0gFa(g;$fO8C5fl|r2$Mhpf-;XXh+qO@qwRo*3aF3^Peq(ZoX{Y4 +zI{-=?P(d+4RGeCZATuQ301AQ)VKCt)uP*jm>#bhz`}L-^YNgIORkhFFyDIsSoS=YZ +zn$&4j0Dz{qmq!QyC=5|RQGrv{71I}RA|<%_y8&?Pi28Ue8RkfG$TD|u^R3|*xU(zB +zZ@K5P&3+xejCMcUc0Y!I$?Wj6>+rQxa)B}|OzgvWV`Bp=t=@L+unvcheY-FGWVHFi +zl+hUAG;tQLGGOrYEIbZ_wx8)lP#?yGs}Q8a$4&`AtDjvvjQwm{y=_{&ObKM)zwukV +z?ApAEov=%*pABr(4!6O;+fK5~27)yC*}z_K8|2pNXFEZO(FtixfQbDV^TUBe=x-IU#ZN;Z$u`KA0EhnA +zLo!6&-=5&_Z6}_v3(9Hrv4vE{zR(;7T997ReQrr7yVJYLD +z`dVs3lYV+~Opo@m+<1RMi}Zelv(aO{zFtaOw^J2so#)_}Rh^3mTy5Ulm5u0bd1$sd +z%1SaNcGG8UC;ePqht{0v`kKJ63u}K_vov5v_U_lm!{_Bh8qW{?ZT<4o4jb||uFmi^ +z30ZU^a{;@{vf9?de5)Ugd$z>m!pd1kt}&nNxfQ%$Cv5tI#JL~tM!fXdm38~gug64p +zPQHzsxrEPoH~acuuiPqwgeLkX9stl@czd`lPi^UIPuqI(lTP(tdKzaJng-1~7E-it +z^}F4)Uyk$l@U7J3X7b1TZoizoMla|<)LiP!1Nk#4H#fGIR{!zvUg_6cr7kf6jdd66 +zD!Qc#F=d2LiSHOK8+x;I0C(Yj=B4!G8I;6d7oDZ^(^PBbaSI=dUyaxIy}Ywy^7z`I +zRoLoo=^Wk)|L%+9vXYI@|SURQQGX8NqLpNr;n9In~x$Y`9UUu4b +z=jT}O9W^IA>!s^fWqQta2@~XR4QTc%_W6`nbn@Ne3#VgO$O3|`e3XWjkL%mQV8j1RJ?y=7L@4SY`!CkuNp2*<-VN6L3(>ruD2jFae>PqZX-`bsIQ&+JMt{gP +zMGkCfgv$FQnqcs~f~OdiY|DPg!rmxM$kF4{{`Zqm(~$&i +z+QAv2st76{Qlo6S65)=WRxC)yUy +z;w?RL0hdV)Qm>* +zBES}EqfdtubT$WSPJnZ!yI~I$dFb&`ckB_IEzK!M8+oC^5DKk{7mRf$B2MJ7ftooS +z7m`QD_Z3v!u!If;5QIFAc%E+N2j>1FKgg +z7%ZjHi24miQsya7RW(wcks%Keps;zu)b0A +zs25JPhFO?2aFvEEbVje2)&!nBGNI6va`ZjV^<&<;*oAIbv0|%zcTmYw4&4OpddEAL +z6i|AB&Whx=JaY`U$^x-?&ZhXh-V_q@PFA<@aasSI*_5LVydg75>Np#kd?j=AhtCi>#xxE@)4Jm|^ +zs9C!Q5#be(DKW;PJqrzQr-nDtV8hy!9XcqM*>Kc{Hi)Y +zPmTmm2c(`po-3dPH8V+oB5Go4z_^Jy6~>KBLl`$QO+h7U>07{50ljGbu$HFq9!STl +zRQU5*u}TW)qryW}P=qcYIYJU%0T=Mg2!9T1opK-OQxml?Ex>xTe&mRTFc(zdmsR+t +ztR(Ivu${EFe*mQyVqfLUoaL3f?`<5A#U40Ybl`&(Ya7=bOd)MOJ2G-U=Z>H$_T|0Y +z^{W@KQn>cOguJ0?aGWf-7(JT(@|I5iBf%`}C~BFpWn|N#tKb_RgYeZTSsiw02)mW; +zvgwkPnZPcejj(JX5!D~Nk~Vjrw9#|*0yG^^1WJ@@<2`R|zz2lx1OkxK8@F%oTSx+v +zp`5$JW&137yLLy#O#u!FXR4^OH*u1oLwc>28fXL7)Um@g{(VmPZizpTP_aRKYb4TI +z*llsqd3ccr733kJ;G2m^Rr<}i{_2P}cOcP_#NKNavgOg9e~mw%r_cj!h+n+Q(e7dY +zC7;|xx2D1TQPpU?h~b)&6iQnTo2o=U9Xl>ccTK6SsO(t4G6P%VNTRCVMTO(8c(IO9 +ztdEfH%PZ8bO#&%E{E-yYlWb6kB6USjj8Yqt(D$@XA3B!#=%_Dwqv+wirB8 +zL^qgwAuxFziHukE)?`+00Gm~>VH3$dQxU2SgG17fWu(#onhwJB<(sSx0V4UlzdrHG%y;vzb`345ye5k10>)4L5IP; +zI`Paa7l+a12C$+Z@l=KLP~eg`bYR1*#VVLKkmcS@$g~*cC3h|a5><(KK90MXhsr(- +zS*DXf9uTQ(;M&~LdwT{0V;*V4H`CKtJ43iN(f5JSYLWRl6-*t-_9iQ%61lUAJ!kUg +z(L+JGpp!~(Y}EpKiZ`@b=pr(v=o>StUA>||(UbKYhJ5 +z-VuxUVj8GE-%=*eT>9*iKn*b4#VW5Z`H%uP25`v85fP34#l;zORy_|pRt|oo&HcS`K{o|XYD2B +zJ7zeBa7)ogsKao*=)JgoOWdFR*}770hIp{Vxa^gDK(ef_Bn(Rd&92Af^K>|x*b?pq +z^dPeNYe@*U035~{RBN)ZOyG{>0&8ibxsV2O6<*3EtogHERT`5X?k$L43Zz2vG&`zYC@zG9><*Ko|Ln#1{*GL0XV_zF;!)3W>if +zpd#f+`~!h1Qi#Mq5}?RAB(=wijm3dgvIBbgde~LPJMuNMyC(Rxh+U4g +zUUkq6#}v28CoPlCukTBh>Mh`0>y4h}Tg&ESk7r!yE=O>(dSt(ALywPCZytwK9m#d$ +zUPt$*@ckG@_#W!myqe{smjppbnDdfPTRpkQP{j*IeS42pi12jTuZ6ys1T5r_(hW9$ +zdU7+-&x42VYT#sQ({uaa88Y>$n@=VcSoT;2uznU|sX9<|8I +z#a`x8Ju?m%F#NNYeFlr`c&}3+utiFXL*IBnB%_2G1?n1oIEO?&oxik>_V5QV2zH0A7$&pmD5vGXmi?%F>v^^WO~RO=R2-$8j> +z$9#dRvai_64=G}G`Q{gD#mTbl6~b3Hm)jlbG5!s?KJG3&RG_$|YwdjQi9{E#S|Ew} +zkgR3d$ZSKM=4o=NyOt`Csy}G%=@s6PWyc6xp66bC`^7jFIWoQ3akGcgz=9JSjufwm +zP`;}AAj@X6zHDlrem}uB$*zX^8l9|B?|3VpYe?RjSh!xi9$!6|^XyqmvH8ll0z*^LmU^ke9M!>G+BM)T9M +zvbft3*&THeF9!a=qS6Gt{Qt`(rJo+ZM=+B3-uVBe{r`m2>TuR2Gvx|d^>(M0_1^{J +z&OOgkw(AystYHt|E^z)78usIKcv{%8i?zQQxwP+SKCpk(P++5}ym5b3NZWww$fh${ +z+z&czyqYIeTzcz0oSZKBoa(Z#PTn*0gDZ_->&rhr!vd6b34w3Fxi*Jyc-Y>4e@$^4 +cxguN4kni(NU3<6%{;31JJp(*$x^uGr9fGaZv;Y7A + +literal 1629 +zcmV-j2BP_iP)XOp-ouEG7tw0 +zrz1-=F%F0OAZ*Bt%4AWe%cd6G%m`sY6u$yfhB9DOrfkevkPjQ|7bz{Y^t;geeV+Z% +z-b-(9d+9KXPx9t{&pFR|p5ODF^FHT&;Y${|kx3oD%S#{awv*l7K)tMQzWRR$@NLb_ +zw;H}v)GQIf6%3j8H~%fa;jd>h$695QRWckW+^S($6E4nXW;Y+LsHg~iVF2&*`MKhR +zHmmCQ1byyU`lD^F&Xm|TNV +z%8R3a_|Tzmm#*3P@Q-HSUYM1eH!c3jMs;&&F +z8VCRQ^d#Seu9v{M5A2zz4}Ep>na1}yj<$BEqf8?QeYhH<1JqFP0=f?}mZ~v(g +zxoJ3q`BNsdeb*C=&+znTPg(oGZ>TsNm*p6-s^-~ +zo5qiMvi`c3fXjnY +z?nzI>n30qgyK(yTm1Cj+ioF{^3@0CWrSO)>rn{c}{@GoeufLXs+vZGT(^CeYib +z@v#)Z=<7IOmIA1i6V}+JBXJIJp`n@GI}ZTx-oB4$Ywcp$iqa857|UG%CfGW821q7= +zq9+APM+z`d>S`Lf|K3fsw_Ssnls$oQl&LdL&c>{lV@vUe( +zB{i9~>+T+MNs|1@lU%4xMhqz7rXF{kl4D#XI1 +zbX{p6Z2g0st1+xPRr~fQk-nI>L$^#A<5PaL=!6;PSwBuOsthb;@vtaN3H8!fg-Rw7QQgI>Pk?49a|gzM^OzTtUX=( +z<{t+S#TiXq)6|eE-gzSI0rP_+YmYB4y}5I>Ds?yLyvmA{4domX6nRy|Tb{c@gw3Go +zSAy>Q%C;02*syDzNaO$;UaV@p5uVP-xx6jWU($8IpY(BDzOQ7j6qV%)(*@c8YURW; +zzpcp2S4#n^S%{1S+QBwkHC1k7_I_Hk`;+V09uYtc%=Ww#?-e@}G}|!}PU;OD{-Qsp +bU%LDkMDBWX&Ru`<00000NkvXXu0mjfJA@b~ + diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PostImage new file mode 100644 index 0000000000..74e4201af6 Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PostImage differ diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PreImage new file mode 100644 index 0000000000..799df8578e Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_PreImage differ diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add.patch new file mode 100644 index 0000000000..bb6a9e42a3 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add.patch @@ -0,0 +1,40 @@ +diff --git a/literal_add b/literal_add +new file mode 100644 +index 0000000000000000000000000000000000000000..799df8578e3cae8a5e979182391b8e9a6a3deded +GIT binary patch +literal 1629 +zcmV-j2BP_iP)XOp-ouEG7tw0 +zrz1-=F%F0OAZ*Bt%4AWe%cd6G%m`sY6u$yfhB9DOrfkevkPjQ|7bz{Y^t;geeV+Z% +z-b-(9d+9KXPx9t{&pFR|p5ODF^FHT&;Y${|kx3oD%S#{awv*l7K)tMQzWRR$@NLb_ +zw;H}v)GQIf6%3j8H~%fa;jd>h$695QRWckW+^S($6E4nXW;Y+LsHg~iVF2&*`MKhR +zHmmCQ1byyU`lD^F&Xm|TNV +z%8R3a_|Tzmm#*3P@Q-HSUYM1eH!c3jMs;&&F +z8VCRQ^d#Seu9v{M5A2zz4}Ep>na1}yj<$BEqf8?QeYhH<1JqFP0=f?}mZ~v(g +zxoJ3q`BNsdeb*C=&+znTPg(oGZ>TsNm*p6-s^-~ +zo5qiMvi`c3fXjnY +z?nzI>n30qgyK(yTm1Cj+ioF{^3@0CWrSO)>rn{c}{@GoeufLXs+vZGT(^CeYib +z@v#)Z=<7IOmIA1i6V}+JBXJIJp`n@GI}ZTx-oB4$Ywcp$iqa857|UG%CfGW821q7= +zq9+APM+z`d>S`Lf|K3fsw_Ssnls$oQl&LdL&c>{lV@vUe( +zB{i9~>+T+MNs|1@lU%4xMhqz7rXF{kl4D#XI1 +zbX{p6Z2g0st1+xPRr~fQk-nI>L$^#A<5PaL=!6;PSwBuOsthb;@vtaN3H8!fg-Rw7QQgI>Pk?49a|gzM^OzTtUX=( +z<{t+S#TiXq)6|eE-gzSI0rP_+YmYB4y}5I>Ds?yLyvmA{4domX6nRy|Tb{c@gw3Go +zSAy>Q%C;02*syDzNaO$;UaV@p5uVP-xx6jWU($8IpY(BDzOQ7j6qV%)(*@c8YURW; +zzpcp2S4#n^S%{1S+QBwkHC1k7_I_Hk`;+V09uYtc%=Ww#?-e@}G}|!}PU;OD{-Qsp +bU%LDkMDBWX&Ru`<00000NkvXXu0mjfJA@b~ + +literal 0 +HcmV?d00001 diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add_PostImage new file mode 100644 index 0000000000..799df8578e Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/literal_add_PostImage differ 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 335a64d70b..e9b8924e6f 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 @@ -9,6 +9,7 @@ */ package org.eclipse.jgit.api; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -19,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Files; import org.eclipse.jgit.api.errors.PatchApplyException; import org.eclipse.jgit.api.errors.PatchFormatException; @@ -29,6 +31,7 @@ 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.eclipse.jgit.util.IO; import org.junit.Test; public class ApplyCommandTest extends RepositoryTestCase { @@ -246,6 +249,44 @@ public class ApplyCommandTest extends RepositoryTestCase { } } + private void checkBinary(String name, boolean hasPreImage) + throws Exception { + try (Git git = new Git(db)) { + byte[] post = IO + .readWholeStream(getTestResource(name + "_PostImage"), 0) + .array(); + File f = new File(db.getWorkTree(), name); + if (hasPreImage) { + byte[] pre = IO + .readWholeStream(getTestResource(name + "_PreImage"), 0) + .array(); + Files.write(f.toPath(), pre); + git.add().addFilepattern(name).call(); + git.commit().setMessage("PreImage").call(); + } + ApplyResult result = git.apply() + .setPatch(getTestResource(name + ".patch")).call(); + assertEquals(1, result.getUpdatedFiles().size()); + assertEquals(f, result.getUpdatedFiles().get(0)); + assertArrayEquals(post, Files.readAllBytes(f.toPath())); + } + } + + @Test + public void testBinaryDelta() throws Exception { + checkBinary("delta", true); + } + + @Test + public void testBinaryLiteral() throws Exception { + checkBinary("literal", true); + } + + @Test + public void testBinaryLiteralAdd() throws Exception { + checkBinary("literal_add", false); + } + @Test public void testAddA1() throws Exception { ApplyResult result = init("A1", false, true); diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 8b6872d337..962324e0f7 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -13,6 +13,9 @@ ambiguousObjectAbbreviation=Object abbreviation {0} is ambiguous aNewObjectIdIsRequired=A NewObjectId is required. anExceptionOccurredWhileTryingToAddTheIdOfHEAD=An exception occurred while trying to add the Id of HEAD anSSHSessionHasBeenAlreadyCreated=An SSH session has been already created +applyBinaryBaseOidWrong=Cannot apply binary patch; OID for file {0} does not match +applyBinaryOidTooShort=Binary patch for file {0} does not have full IDs +applyBinaryResultOidWrong=Result of binary patch for file {0} has wrong OID. applyingCommit=Applying {0} archiveFormatAlreadyAbsent=Archive format already absent: {0} archiveFormatAlreadyRegistered=Archive format already registered with different implementation: {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java index 5d975ea389..f7eba88acd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java @@ -9,7 +9,9 @@ */ package org.eclipse.jgit.api; +import java.io.BufferedInputStream; import java.io.BufferedWriter; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -25,6 +27,7 @@ import java.text.MessageFormat; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.zip.InflaterInputStream; import org.eclipse.jgit.api.errors.FilterFailedException; import org.eclipse.jgit.api.errors.GitAPIException; @@ -44,10 +47,13 @@ import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.CoreConfig.EolStreamType; import org.eclipse.jgit.lib.FileMode; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ObjectStream; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.patch.BinaryHunk; import org.eclipse.jgit.patch.FileHeader; +import org.eclipse.jgit.patch.FileHeader.PatchType; import org.eclipse.jgit.patch.HunkHeader; import org.eclipse.jgit.patch.Patch; import org.eclipse.jgit.treewalk.FileTreeIterator; @@ -57,14 +63,17 @@ import org.eclipse.jgit.treewalk.filter.AndTreeFilter; import org.eclipse.jgit.treewalk.filter.NotIgnoredFilter; import org.eclipse.jgit.treewalk.filter.PathFilterGroup; import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.util.FS.ExecutionResult; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.TemporaryBuffer; -import org.eclipse.jgit.util.FS.ExecutionResult; import org.eclipse.jgit.util.TemporaryBuffer.LocalFile; +import org.eclipse.jgit.util.io.BinaryDeltaInputStream; +import org.eclipse.jgit.util.io.BinaryHunkInputStream; import org.eclipse.jgit.util.io.EolStreamTypeUtil; +import org.eclipse.jgit.util.sha1.SHA1; /** * Apply a patch to files and/or to the index. @@ -191,6 +200,9 @@ public class ApplyCommand extends GitCommand { private void apply(Repository repository, String path, DirCache cache, File f, FileHeader fh) throws IOException, PatchApplyException { + if (PatchType.BINARY.equals(fh.getPatchType())) { + return; + } boolean convertCrLf = needsCrLfConversion(f, fh); // Use a TreeWalk with a DirCacheIterator to pick up the correct // clean/smudge filters. CR-LF handling is completely determined by @@ -217,16 +229,23 @@ public class ApplyCommand extends GitCommand { FileTreeIterator file = walk.getTree(fileIdx, FileTreeIterator.class); if (file != null) { - command = walk - .getFilterCommand(Constants.ATTR_FILTER_TYPE_CLEAN); - RawText raw; - // Can't use file.openEntryStream() as it would do CR-LF - // conversion as usual, not as wanted by us. - try (InputStream input = filterClean(repository, path, - new FileInputStream(f), convertCrLf, command)) { - raw = new RawText(IO.readWholeStream(input, 0).array()); + if (PatchType.GIT_BINARY.equals(fh.getPatchType())) { + applyBinary(repository, path, f, fh, + file::openEntryStream, file.getEntryObjectId(), + checkOut); + } else { + command = walk.getFilterCommand( + Constants.ATTR_FILTER_TYPE_CLEAN); + RawText raw; + // Can't use file.openEntryStream() as it would do CR-LF + // conversion as usual, not as wanted by us. + try (InputStream input = filterClean(repository, path, + new FileInputStream(f), convertCrLf, command)) { + raw = new RawText( + IO.readWholeStream(input, 0).array()); + } + applyText(repository, path, raw, f, fh, checkOut); } - apply(repository, path, raw, f, fh, checkOut); return; } } @@ -234,21 +253,30 @@ public class ApplyCommand extends GitCommand { // File ignored? RawText raw; CheckoutMetadata checkOut; - if (convertCrLf) { - try (InputStream input = EolStreamTypeUtil.wrapInputStream( - new FileInputStream(f), EolStreamType.TEXT_LF)) { - raw = new RawText(IO.readWholeStream(input, 0).array()); - } - checkOut = new CheckoutMetadata(EolStreamType.TEXT_CRLF, null); - } else { - raw = new RawText(f); + if (PatchType.GIT_BINARY.equals(fh.getPatchType())) { checkOut = new CheckoutMetadata(EolStreamType.DIRECT, null); + applyBinary(repository, path, f, fh, () -> new FileInputStream(f), + null, checkOut); + } else { + if (convertCrLf) { + try (InputStream input = EolStreamTypeUtil.wrapInputStream( + new FileInputStream(f), EolStreamType.TEXT_LF)) { + raw = new RawText(IO.readWholeStream(input, 0).array()); + } + checkOut = new CheckoutMetadata(EolStreamType.TEXT_CRLF, null); + } else { + raw = new RawText(f); + checkOut = new CheckoutMetadata(EolStreamType.DIRECT, null); + } + applyText(repository, path, raw, f, fh, checkOut); } - apply(repository, path, raw, f, fh, checkOut); } private boolean needsCrLfConversion(File f, FileHeader fileHeader) throws IOException { + if (PatchType.GIT_BINARY.equals(fileHeader.getPatchType())) { + return false; + } if (!hasCrLf(fileHeader)) { try (InputStream input = new FileInputStream(f)) { return RawText.isCrLfText(input); @@ -258,7 +286,7 @@ public class ApplyCommand extends GitCommand { } private static boolean hasCrLf(FileHeader fileHeader) { - if (fileHeader == null) { + if (PatchType.GIT_BINARY.equals(fileHeader.getPatchType())) { return false; } for (HunkHeader header : fileHeader.getHunks()) { @@ -330,19 +358,30 @@ public class ApplyCommand extends GitCommand { return result.getStdout().openInputStreamWithAutoDestroy(); } + /** + * Something that can supply an {@link InputStream}. + */ + private interface StreamSupplier { + InputStream load() throws IOException; + } + /** * We write the patch result to a {@link TemporaryBuffer} and then use * {@link DirCacheCheckout}.getContent() to run the result through the CR-LF * and smudge filters. DirCacheCheckout needs an ObjectLoader, not a - * TemporaryBuffer, so this class bridges between the two, making the - * TemporaryBuffer look like an ordinary git blob to DirCacheCheckout. + * TemporaryBuffer, so this class bridges between the two, making any Stream + * provided by a {@link StreamSupplier} look like an ordinary git blob to + * DirCacheCheckout. */ - private static class BufferLoader extends ObjectLoader { + private static class StreamLoader extends ObjectLoader { - private TemporaryBuffer data; + private StreamSupplier data; - BufferLoader(TemporaryBuffer data) { + private long size; + + StreamLoader(StreamSupplier data, long length) { this.data = data; + this.size = length; } @Override @@ -352,7 +391,7 @@ public class ApplyCommand extends GitCommand { @Override public long getSize() { - return data.length(); + return size; } @Override @@ -369,12 +408,146 @@ public class ApplyCommand extends GitCommand { public ObjectStream openStream() throws MissingObjectException, IOException { return new ObjectStream.Filter(getType(), getSize(), - data.openInputStream()); + new BufferedInputStream(data.load())); } } - private void apply(Repository repository, String path, RawText rt, File f, - FileHeader fh, CheckoutMetadata checkOut) + private void initHash(SHA1 hash, long size) { + hash.update(Constants.encodedTypeString(Constants.OBJ_BLOB)); + hash.update((byte) ' '); + hash.update(Constants.encodeASCII(size)); + hash.update((byte) 0); + } + + private ObjectId hash(File f) throws IOException { + SHA1 hash = SHA1.newInstance(); + initHash(hash, f.length()); + try (InputStream input = new FileInputStream(f)) { + byte[] buf = new byte[8192]; + int n; + while ((n = input.read(buf)) >= 0) { + hash.update(buf, 0, n); + } + } + return hash.toObjectId(); + } + + private void checkOid(ObjectId baseId, ObjectId id, ChangeType type, File f, + String path) + throws PatchApplyException, IOException { + boolean hashOk = false; + if (id != null) { + hashOk = baseId.equals(id); + if (!hashOk && ChangeType.ADD.equals(type) + && ObjectId.zeroId().equals(baseId)) { + // We create the file first. The OID of an empty file is not the + // zero id! + hashOk = Constants.EMPTY_BLOB_ID.equals(id); + } + } else { + if (ObjectId.zeroId().equals(baseId)) { + // File empty is OK. + hashOk = !f.exists() || f.length() == 0; + } else { + hashOk = baseId.equals(hash(f)); + } + } + if (!hashOk) { + throw new PatchApplyException(MessageFormat + .format(JGitText.get().applyBinaryBaseOidWrong, path)); + } + } + + private void applyBinary(Repository repository, String path, File f, + FileHeader fh, StreamSupplier loader, ObjectId id, + CheckoutMetadata checkOut) + throws PatchApplyException, IOException { + if (!fh.getOldId().isComplete() || !fh.getNewId().isComplete()) { + throw new PatchApplyException(MessageFormat + .format(JGitText.get().applyBinaryOidTooShort, path)); + } + BinaryHunk hunk = fh.getForwardBinaryHunk(); + // A BinaryHunk has the start at the "literal" or "delta" token. Data + // starts on the next line. + int start = RawParseUtils.nextLF(hunk.getBuffer(), + hunk.getStartOffset()); + int length = hunk.getEndOffset() - start; + SHA1 hash = SHA1.newInstance(); + // Write to a buffer and copy to the file only if everything was fine + TemporaryBuffer buffer = new TemporaryBuffer.LocalFile(null); + try { + switch (hunk.getType()) { + case LITERAL_DEFLATED: + // This just overwrites the file. We need to check the hash of + // the base. + checkOid(fh.getOldId().toObjectId(), id, fh.getChangeType(), f, + path); + initHash(hash, hunk.getSize()); + try (OutputStream out = buffer; + InputStream inflated = new SHA1InputStream(hash, + new InflaterInputStream( + new BinaryHunkInputStream( + new ByteArrayInputStream( + hunk.getBuffer(), start, + length))))) { + DirCacheCheckout.getContent(repository, path, checkOut, + new StreamLoader(() -> inflated, hunk.getSize()), + null, out); + if (!fh.getNewId().toObjectId().equals(hash.toObjectId())) { + throw new PatchApplyException(MessageFormat.format( + JGitText.get().applyBinaryResultOidWrong, + path)); + } + } + try (InputStream bufIn = buffer.openInputStream()) { + Files.copy(bufIn, f.toPath(), + StandardCopyOption.REPLACE_EXISTING); + } + break; + case DELTA_DEFLATED: + // Unfortunately delta application needs random access to the + // base to construct the result. + byte[] base; + try (InputStream input = loader.load()) { + base = IO.readWholeStream(input, 0).array(); + } + // At least stream the result! + try (BinaryDeltaInputStream input = new BinaryDeltaInputStream( + base, + new InflaterInputStream(new BinaryHunkInputStream( + new ByteArrayInputStream(hunk.getBuffer(), + start, length))))) { + long finalSize = input.getExpectedResultSize(); + initHash(hash, finalSize); + try (OutputStream out = buffer; + SHA1InputStream hashed = new SHA1InputStream(hash, + input)) { + DirCacheCheckout.getContent(repository, path, checkOut, + new StreamLoader(() -> hashed, finalSize), null, + out); + if (!fh.getNewId().toObjectId() + .equals(hash.toObjectId())) { + throw new PatchApplyException(MessageFormat.format( + JGitText.get().applyBinaryResultOidWrong, + path)); + } + } + } + try (InputStream bufIn = buffer.openInputStream()) { + Files.copy(bufIn, f.toPath(), + StandardCopyOption.REPLACE_EXISTING); + } + break; + default: + break; + } + } finally { + buffer.destroy(); + } + } + + private void applyText(Repository repository, String path, RawText rt, + File f, FileHeader fh, CheckoutMetadata checkOut) throws IOException, PatchApplyException { List oldLines = new ArrayList<>(rt.size()); for (int i = 0; i < rt.size(); i++) { @@ -514,7 +687,9 @@ public class ApplyCommand extends GitCommand { } try (OutputStream output = new FileOutputStream(f)) { DirCacheCheckout.getContent(repository, path, checkOut, - new BufferLoader(buffer), null, output); + new StreamLoader(buffer::openInputStream, + buffer.length()), + null, output); } } finally { buffer.destroy(); @@ -565,4 +740,43 @@ public class ApplyCommand extends GitCommand { return lhrt.getString(lhrt.size() - 1) .equals("\\ No newline at end of file"); //$NON-NLS-1$ } + + /** + * An {@link InputStream} that updates a {@link SHA1} on every byte read. + * The hash is supposed to have been initialized before reading starts. + */ + private static class SHA1InputStream extends InputStream { + + private final SHA1 hash; + + private final InputStream in; + + SHA1InputStream(SHA1 hash, InputStream in) { + this.hash = hash; + this.in = in; + } + + @Override + public int read() throws IOException { + int b = in.read(); + if (b >= 0) { + hash.update((byte) b); + } + return b; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int n = in.read(b, off, len); + if (n > 0) { + hash.update(b, off, n); + } + return n; + } + + @Override + public void close() throws IOException { + in.close(); + } + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 3b67b0f628..fd54986f94 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -41,6 +41,9 @@ public class JGitText extends TranslationBundle { /***/ public String aNewObjectIdIsRequired; /***/ public String anExceptionOccurredWhileTryingToAddTheIdOfHEAD; /***/ public String anSSHSessionHasBeenAlreadyCreated; + /***/ public String applyBinaryBaseOidWrong; + /***/ public String applyBinaryOidTooShort; + /***/ public String applyBinaryResultOidWrong; /***/ public String applyingCommit; /***/ public String archiveFormatAlreadyAbsent; /***/ public String archiveFormatAlreadyRegistered; -- cgit v1.2.3 From 76b76a6048b9b3dbca965463d4f6f5732ffb784d Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Wed, 10 Mar 2021 14:25:37 +0100 Subject: ApplyCommand: use byte arrays for text patches, not strings Instead of converting the patch bytes to strings apply the patch on byte level, like C git does. Converting the input lines and the hunk lines from bytes to strings and then applying the patch based on strings may give surprising results if a patch converts a text file from one encoding to another. Moreover, in the end we don't know which encoding to use to write the result. Previous code just wrote the result as UTF-8, which forcibly changed the encoding if the original input had some other encoding (even if the patch had the same non-UTF-8 encoding). It was also wrong if the input was UTF-8, and the patch should have changed the encoding to something else. So use ByteBuffers instead of Strings. This has the additional advantage that all these ByteBuffers can share the underlying byte arrays of the input and of the patch, so it also reduces memory consumption. Change-Id: I450975f2ba0e7d0bec8973e3113cc2e7aea187ee Signed-off-by: Thomas Wolf --- .../.settings/org.eclipse.core.resources.prefs | 3 +- .../tst-rsrc/org/eclipse/jgit/diff/umlaut.patch | 7 +++ .../org/eclipse/jgit/diff/umlaut_PostImage | 1 + .../tst-rsrc/org/eclipse/jgit/diff/umlaut_PreImage | 1 + .../tst/org/eclipse/jgit/api/ApplyCommandTest.java | 8 +++ .../src/org/eclipse/jgit/api/ApplyCommand.java | 70 ++++++++++------------ .../src/org/eclipse/jgit/diff/RawText.java | 24 +++++++- 7 files changed, 74 insertions(+), 40 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PreImage (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/.settings/org.eclipse.core.resources.prefs b/org.eclipse.jgit.test/.settings/org.eclipse.core.resources.prefs index 6a9621db1d..cddb99d1d5 100644 --- a/org.eclipse.jgit.test/.settings/org.eclipse.core.resources.prefs +++ b/org.eclipse.jgit.test/.settings/org.eclipse.core.resources.prefs @@ -1,5 +1,6 @@ -#Sat Dec 20 21:21:24 CET 2008 eclipse.preferences.version=1 +encoding//tst-rsrc/org/eclipse/jgit/diff/umlaut.patch=ISO-8859-1 +encoding//tst-rsrc/org/eclipse/jgit/diff/umlaut_PostImage=ISO-8859-1 encoding//tst-rsrc/org/eclipse/jgit/patch/testGetText_BothISO88591.patch=ISO-8859-1 encoding//tst-rsrc/org/eclipse/jgit/patch/testGetText_Convert.patch=ISO-8859-1 encoding//tst-rsrc/org/eclipse/jgit/patch/testGetText_DiffCc.patch=ISO-8859-1 diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut.patch new file mode 100644 index 0000000000..7380dbed82 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut.patch @@ -0,0 +1,7 @@ +diff --git a/umlaut b/umlaut +index 003a054..557f72f 100644 +--- a/umlaut ++++ b/umlaut +@@ -1 +1 @@ +-ÄÖÜ ++ÄÖÜ diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PostImage new file mode 100644 index 0000000000..557f72f513 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PostImage @@ -0,0 +1 @@ +ÄÖÜ diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PreImage new file mode 100644 index 0000000000..003a054114 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/umlaut_PreImage @@ -0,0 +1 @@ +ÄÖÜ 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 e9b8924e6f..b997ac009a 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 @@ -287,6 +287,14 @@ public class ApplyCommandTest extends RepositoryTestCase { checkBinary("literal_add", false); } + @Test + public void testEncodingChange() throws Exception { + // This is a text patch that changes a file containing ÄÖÜ in UTF-8 to + // the same characters in ISO-8859-1. The patch file itself uses mixed + // encoding. Since checkFile() works with strings use the binary check. + checkBinary("umlaut", true); + } + @Test public void testAddA1() throws Exception { ApplyResult result = init("A1", false, true); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java index f7eba88acd..ec53412fc1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java @@ -10,7 +10,6 @@ package org.eclipse.jgit.api; import java.io.BufferedInputStream; -import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; @@ -18,9 +17,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.Writer; -import java.nio.charset.StandardCharsets; +import java.nio.ByteBuffer; import java.nio.file.Files; import java.nio.file.StandardCopyOption; import java.text.MessageFormat; @@ -549,11 +546,11 @@ public class ApplyCommand extends GitCommand { private void applyText(Repository repository, String path, RawText rt, File f, FileHeader fh, CheckoutMetadata checkOut) throws IOException, PatchApplyException { - List oldLines = new ArrayList<>(rt.size()); + List oldLines = new ArrayList<>(rt.size()); for (int i = 0; i < rt.size(); i++) { - oldLines.add(rt.getString(i)); + oldLines.add(rt.getRawString(i)); } - List newLines = new ArrayList<>(oldLines); + List newLines = new ArrayList<>(oldLines); int afterLastHunk = 0; int lineNumberShift = 0; int lastHunkNewLine = -1; @@ -571,9 +568,9 @@ public class ApplyCommand extends GitCommand { b.length); RawText hrt = new RawText(b); - List hunkLines = new ArrayList<>(hrt.size()); + List hunkLines = new ArrayList<>(hrt.size()); for (int i = 0; i < hrt.size(); i++) { - hunkLines.add(hrt.getString(i)); + hunkLines.add(hrt.getRawString(i)); } if (hh.getNewStartLine() == 0) { @@ -642,8 +639,8 @@ public class ApplyCommand extends GitCommand { lineNumberShift = applyAt - hh.getNewStartLine() + 1; int sz = hunkLines.size(); for (int j = 1; j < sz; j++) { - String hunkLine = hunkLines.get(j); - switch (hunkLine.charAt(0)) { + ByteBuffer hunkLine = hunkLines.get(j); + switch (hunkLine.array()[hunkLine.position()]) { case ' ': applyAt++; break; @@ -651,7 +648,7 @@ public class ApplyCommand extends GitCommand { newLines.remove(applyAt); break; case '+': - newLines.add(applyAt++, hunkLine.substring(1)); + newLines.add(applyAt++, slice(hunkLine, 1)); break; default: break; @@ -660,28 +657,29 @@ public class ApplyCommand extends GitCommand { afterLastHunk = applyAt; } if (!isNoNewlineAtEndOfFile(fh)) { - newLines.add(""); //$NON-NLS-1$ + newLines.add(null); } if (!rt.isMissingNewlineAtEnd()) { - oldLines.add(""); //$NON-NLS-1$ + oldLines.add(null); } - if (!isChanged(oldLines, newLines)) { - return; // Don't touch the file + if (oldLines.equals(newLines)) { + return; // Unchanged; don't touch the file } - // TODO: forcing UTF-8 is a bit strange and may lead to re-coding if the - // input was some other encoding, but it's what previous versions of - // this code used. (Even earlier the code used the default encoding, - // which has the same problem.) Perhaps using bytes instead of Strings - // for the lines would be better. TemporaryBuffer buffer = new TemporaryBuffer.LocalFile(null); try { - try (Writer w = new BufferedWriter( - new OutputStreamWriter(buffer, StandardCharsets.UTF_8))) { - for (Iterator l = newLines.iterator(); l.hasNext();) { - w.write(l.next()); + try (OutputStream out = buffer) { + for (Iterator l = newLines.iterator(); l + .hasNext();) { + ByteBuffer line = l.next(); + if (line == null) { + // Must be the marker for the final newline + break; + } + out.write(line.array(), line.position(), + line.limit() - line.position()); if (l.hasNext()) { - w.write('\n'); + out.write('\n'); } } } @@ -698,18 +696,18 @@ public class ApplyCommand extends GitCommand { fh.getNewMode() == FileMode.EXECUTABLE_FILE); } - private boolean canApplyAt(List hunkLines, List newLines, - int line) { + private boolean canApplyAt(List hunkLines, + List newLines, int line) { int sz = hunkLines.size(); int limit = newLines.size(); int pos = line; for (int j = 1; j < sz; j++) { - String hunkLine = hunkLines.get(j); - switch (hunkLine.charAt(0)) { + ByteBuffer hunkLine = hunkLines.get(j); + switch (hunkLine.array()[hunkLine.position()]) { case ' ': case '-': if (pos >= limit - || !newLines.get(pos).equals(hunkLine.substring(1))) { + || !newLines.get(pos).equals(slice(hunkLine, 1))) { return false; } pos++; @@ -721,13 +719,9 @@ public class ApplyCommand extends GitCommand { return true; } - private static boolean isChanged(List ol, List nl) { - if (ol.size() != nl.size()) - return true; - for (int i = 0; i < ol.size(); i++) - if (!ol.get(i).equals(nl.get(i))) - return true; - return false; + private ByteBuffer slice(ByteBuffer b, int off) { + int newOffset = b.position() + off; + return ByteBuffer.wrap(b.array(), newOffset, b.limit() - newOffset); } private boolean isNoNewlineAtEndOfFile(FileHeader fh) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java index 9f4b1fa493..d09da019dd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RawText.java @@ -1,6 +1,6 @@ /* * Copyright (C) 2009, Google Inc. - * Copyright (C) 2008-2009, Johannes E. Schindelin and others + * Copyright (C) 2008-2021, Johannes E. Schindelin 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 @@ -16,6 +16,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.ByteBuffer; import org.eclipse.jgit.errors.BinaryBlobException; import org.eclipse.jgit.errors.LargeObjectException; @@ -164,6 +165,27 @@ public class RawText extends Sequence { return getString(i, i + 1, true); } + /** + * Get the raw text for a single line. + * + * @param i + * index of the line to extract. Note this is 0-based, so line + * number 1 is actually index 0. + * @return the text for the line, without a trailing LF, as a + * {@link ByteBuffer} that is backed by a slice of the + * {@link #getRawContent() raw content}, with the buffer's position + * on the start of the line and the limit at the end. + * @since 5.12 + */ + public ByteBuffer getRawString(int i) { + int s = getStart(i); + int e = getEnd(i); + if (e > 0 && content[e - 1] == '\n') { + e--; + } + return ByteBuffer.wrap(content, s, e - s); + } + /** * Get the text for a region of lines. * -- cgit v1.2.3 From 2a0295ccfd1cf8dcb1067575f38c86f430285cda Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Wed, 10 Mar 2021 18:04:25 +0100 Subject: ApplyCommand: handle completely empty context lines in text patches C git treats completely empty lines as empty context lines (which traditionally have a single blank). Apparently newer GNU diff may produce such lines; see [1]. ("Newer" meaning "since 2006"...) [1] https://github.com/git/git/commit/b507b465f7831 Change-Id: I80c1f030edb17a46289b1dabf11a2648d2660d38 Signed-off-by: Thomas Wolf --- .../tst-rsrc/org/eclipse/jgit/diff/emptyLine.patch | 10 ++++++++++ .../tst-rsrc/org/eclipse/jgit/diff/emptyLine_PostImage | 4 ++++ .../tst-rsrc/org/eclipse/jgit/diff/emptyLine_PreImage | 4 ++++ .../tst/org/eclipse/jgit/api/ApplyCommandTest.java | 8 ++++++++ .../src/org/eclipse/jgit/api/ApplyCommand.java | 16 ++++++++++++++-- 5 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PreImage (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine.patch new file mode 100644 index 0000000000..18c80c4feb --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine.patch @@ -0,0 +1,10 @@ +diff --git a/emptyLine b/emptyLine +index 1fd3fa2..45c2c9b 100644 +--- a/emptyLine ++++ b/emptyLine +@@ -1,4 +1,4 @@ + foo + +-fie ++bar + fum diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PostImage new file mode 100644 index 0000000000..45c2c9ba5b --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PostImage @@ -0,0 +1,4 @@ +foo + +bar +fum diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PreImage new file mode 100644 index 0000000000..1fd3fa23b0 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/emptyLine_PreImage @@ -0,0 +1,4 @@ +foo + +fie +fum 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 b997ac009a..807b961300 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 @@ -295,6 +295,14 @@ public class ApplyCommandTest extends RepositoryTestCase { checkBinary("umlaut", true); } + @Test + public void testEmptyLine() throws Exception { + // C git accepts completely empty lines as empty context lines. + // According to comments in the C git sources (apply.c), newer GNU diff + // may produce such diffs. + checkBinary("emptyLine", true); + } + @Test public void testAddA1() throws Exception { ApplyResult result = init("A1", false, true); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java index ec53412fc1..f649c5fdeb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java @@ -640,6 +640,11 @@ public class ApplyCommand extends GitCommand { int sz = hunkLines.size(); for (int j = 1; j < sz; j++) { ByteBuffer hunkLine = hunkLines.get(j); + if (!hunkLine.hasRemaining()) { + // Completely empty line; accept as empty context line + applyAt++; + continue; + } switch (hunkLine.array()[hunkLine.position()]) { case ' ': applyAt++; @@ -676,8 +681,7 @@ public class ApplyCommand extends GitCommand { // Must be the marker for the final newline break; } - out.write(line.array(), line.position(), - line.limit() - line.position()); + out.write(line.array(), line.position(), line.remaining()); if (l.hasNext()) { out.write('\n'); } @@ -703,6 +707,14 @@ public class ApplyCommand extends GitCommand { int pos = line; for (int j = 1; j < sz; j++) { ByteBuffer hunkLine = hunkLines.get(j); + if (!hunkLine.hasRemaining()) { + // Empty line. Accept as empty context line. + if (pos >= limit || newLines.get(pos).hasRemaining()) { + return false; + } + pos++; + continue; + } switch (hunkLine.array()[hunkLine.position()]) { case ' ': case '-': -- cgit v1.2.3 From 1126f26d21b3aadf364af09c79d27d39de5e9bb9 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Wed, 10 Mar 2021 19:26:39 +0100 Subject: ApplyCommand: fix "no newline at end" detection Check the last line of the last hunk of a file, not the last line of the whole patch. Note that C git only checks that this line starts with "\ " and is at least 12 characters long because of possible different texts when non- English messages are used. Change-Id: I0db81699eb3e99ed7b536a3e2b8dc97df1f58a89 Signed-off-by: Thomas Wolf --- .../tst-rsrc/org/eclipse/jgit/diff/hello.patch | 16 ++++++++++++++++ .../tst-rsrc/org/eclipse/jgit/diff/hello_PostImage | 1 + .../tst-rsrc/org/eclipse/jgit/diff/hello_PreImage | 1 + .../tst/org/eclipse/jgit/api/ApplyCommandTest.java | 20 +++++++++++++++++++- .../src/org/eclipse/jgit/api/ApplyCommand.java | 6 +++++- 5 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello.patch create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PostImage create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PreImage (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello.patch b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello.patch new file mode 100644 index 0000000000..f015a38062 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello.patch @@ -0,0 +1,16 @@ +diff --git a/hello b/hello +index b6fc4c6..0abaeaa 100644 +--- a/hello ++++ b/hello +@@ -1 +1 @@ +-hello +\ No newline at end of file ++bye +\ No newline at end of file +diff --git a/yello b/yello +index 391a8cb..d1ed081 100644 +--- a/yello ++++ b/yello +@@ -1 +1 @@ +-yello ++yellow diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PostImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PostImage new file mode 100644 index 0000000000..0abaeaa993 --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PostImage @@ -0,0 +1 @@ +bye \ No newline at end of file diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PreImage b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PreImage new file mode 100644 index 0000000000..b6fc4c620b --- /dev/null +++ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/diff/hello_PreImage @@ -0,0 +1 @@ +hello \ No newline at end of file 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 807b961300..867310b60c 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 @@ -20,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import org.eclipse.jgit.api.errors.PatchApplyException; @@ -251,6 +252,11 @@ public class ApplyCommandTest extends RepositoryTestCase { private void checkBinary(String name, boolean hasPreImage) throws Exception { + checkBinary(name, hasPreImage, 1); + } + + private void checkBinary(String name, boolean hasPreImage, + int numberOfFiles) throws Exception { try (Git git = new Git(db)) { byte[] post = IO .readWholeStream(getTestResource(name + "_PostImage"), 0) @@ -266,7 +272,7 @@ public class ApplyCommandTest extends RepositoryTestCase { } ApplyResult result = git.apply() .setPatch(getTestResource(name + ".patch")).call(); - assertEquals(1, result.getUpdatedFiles().size()); + assertEquals(numberOfFiles, result.getUpdatedFiles().size()); assertEquals(f, result.getUpdatedFiles().get(0)); assertArrayEquals(post, Files.readAllBytes(f.toPath())); } @@ -303,6 +309,18 @@ public class ApplyCommandTest extends RepositoryTestCase { checkBinary("emptyLine", true); } + @Test + public void testMultiFileNoNewline() throws Exception { + // This test needs two files. One is in the test resources. + try (Git git = new Git(db)) { + Files.write(db.getWorkTree().toPath().resolve("yello"), + "yello".getBytes(StandardCharsets.US_ASCII)); + git.add().addFilepattern("yello").call(); + git.commit().setMessage("yello").call(); + } + checkBinary("hello", true, 2); + } + @Test public void testAddA1() throws Exception { ApplyResult result = init("A1", false, true); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java index f649c5fdeb..583767af3f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ApplyCommand.java @@ -742,7 +742,11 @@ public class ApplyCommand extends GitCommand { return false; } HunkHeader lastHunk = hunks.get(hunks.size() - 1); - RawText lhrt = new RawText(lastHunk.getBuffer()); + byte[] buf = new byte[lastHunk.getEndOffset() + - lastHunk.getStartOffset()]; + System.arraycopy(lastHunk.getBuffer(), lastHunk.getStartOffset(), buf, + 0, buf.length); + RawText lhrt = new RawText(buf); return lhrt.getString(lhrt.size() - 1) .equals("\\ No newline at end of file"); //$NON-NLS-1$ } -- cgit v1.2.3 From 0667b8ec4d8b10ebc57271642953de6852c3331b Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 24 May 2021 17:31:25 -0700 Subject: RepoCommand: Do not set 'branch' if the revision is a tag The "branch" field in the .gitmodules is the signal for gerrit to keep the superproject autoupdated. Tags are immutable and there is no need to track them, plus the cgit client requires the field to be a "remote branch name" but not a tag. Do not set the "branch" field if the revision is a tag. Keep those tags in another field ("ref") as they help other tools to find the commit in the destination repository. We can still have false negatives when a refname is not fully qualified, but this check covers e.g. the most common case in android. Note that the javadoc of #setRecordRemoteBranch already mentions that "submodules that request a tag will not have branch name recorded". Change-Id: Ib1c321a4d3b7f8d51ca2ea204f72dc0cfed50c37 Signed-off-by: Ivan Frade --- .../org/eclipse/jgit/gitrepo/RepoCommandTest.java | 48 ++++++++++++++++++++++ .../src/org/eclipse/jgit/gitrepo/RepoCommand.java | 8 +++- 2 files changed, 54 insertions(+), 2 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java index ae4db0b7d8..509adc2cb2 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java @@ -46,6 +46,8 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.util.IO; +import org.eclipse.jgit.util.RawParseUtils; import org.junit.Test; public class RepoCommandTest extends RepositoryTestCase { @@ -749,7 +751,53 @@ public class RepoCommandTest extends RepositoryTestCase { String gitlink = localDb.resolve(Constants.HEAD + ":foo").name(); assertEquals("The gitlink is same as remote head", oldCommitId.name(), gitlink); + + File dotmodules = new File(localDb.getWorkTree(), + Constants.DOT_GIT_MODULES); + assertTrue(dotmodules.exists()); + // The .gitmodules file should have "branch" lines + String gitModulesContents = RawParseUtils + .decode(IO.readFully(dotmodules)); + assertTrue(gitModulesContents.contains("branch = branch")); + } + } + + @Test + public void testRevisionBare_ignoreTags() throws Exception { + Repository remoteDb = createBareRepository(); + Repository tempDb = createWorkRepository(); + + StringBuilder xmlContent = new StringBuilder(); + xmlContent.append("\n") + .append("") + .append("") + .append("") + .append("").append(""); + JGitTestUtil.writeTrashFile(tempDb, "manifest.xml", + xmlContent.toString()); + RepoCommand command = new RepoCommand(remoteDb); + command.setPath( + tempDb.getWorkTree().getAbsolutePath() + "/manifest.xml") + .setURI(rootUri).call(); + // Clone it + File directory = createTempDirectory("testReplaceManifestBare"); + File dotmodules; + try (Repository localDb = Git.cloneRepository().setDirectory(directory) + .setURI(remoteDb.getDirectory().toURI().toString()).call() + .getRepository()) { + dotmodules = new File(localDb.getWorkTree(), + Constants.DOT_GIT_MODULES); + assertTrue(dotmodules.exists()); } + + // The .gitmodules file should not have "branch" lines + String gitModulesContents = RawParseUtils + .decode(IO.readFully(dotmodules)); + assertFalse(gitModulesContents.contains("branch")); + assertTrue(gitModulesContents.contains("ref = refs/tags/" + TAG)); } @Test diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java index c039aaffa9..5bf95def95 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java @@ -12,6 +12,7 @@ package org.eclipse.jgit.gitrepo; import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jgit.lib.Constants.DEFAULT_REMOTE_NAME; import static org.eclipse.jgit.lib.Constants.R_REMOTES; +import static org.eclipse.jgit.lib.Constants.R_TAGS; import java.io.File; import java.io.FileInputStream; @@ -587,8 +588,11 @@ public class RepoCommand extends GitCommand { throw new RemoteUnavailableException(url); } if (recordRemoteBranch) { - // can be branch or tag - cfg.setString("submodule", name, "branch", //$NON-NLS-1$ //$NON-NLS-2$ + // "branch" field is only for non-tag references. + // Keep tags in "ref" field as hint for other tools. + String field = proj.getRevision().startsWith( + R_TAGS) ? "ref" : "branch"; //$NON-NLS-1$ //$NON-NLS-2$ + cfg.setString("submodule", name, field, //$NON-NLS-1$ proj.getRevision()); } -- cgit v1.2.3 From c59626ad7a06d47dcf6c223d0c8ceaec2e63e276 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 24 May 2021 09:53:08 -0700 Subject: RepoCommand: Retry commit on LockFailure When the target repository is receiving commits from other sources, the repo command commit can fail with a LOCK_FAILURE. We could let callers retry, but then the command needs to redo all the work (opening all subrepos to recreate the tree). Retry the commit in LOCK_FAILURE inside the command. The commit rewrites the whole tree, so it shouldn't have merge errors. Use an exponential delay with jitter for the retries. Change-Id: I517b6f2afd16a4b695e6cf471b5d6cf492024ec4 Signed-off-by: Ivan Frade --- .../src/org/eclipse/jgit/gitrepo/RepoCommand.java | 110 +++++++++++++-------- 1 file changed, 67 insertions(+), 43 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java index 5bf95def95..552315d43a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java @@ -80,6 +80,13 @@ import org.eclipse.jgit.util.FileUtils; * @since 3.4 */ public class RepoCommand extends GitCommand { + private static final int LOCK_FAILURE_MAX_RETRIES = 5; + + // Retry exponentially with delays in this range + private static final int LOCK_FAILURE_MIN_RETRY_DELAY_MILLIS = 50; + + private static final int LOCK_FAILURE_MAX_RETRY_DELAY_MILLIS = 5000; + private String manifestPath; private String baseUri; private URI targetUri; @@ -686,50 +693,22 @@ public class RepoCommand extends GitCommand { builder.finish(); ObjectId treeId = index.writeTree(inserter); - // Create a Commit object, populate it and write it - ObjectId headId = repo.resolve(targetBranch + "^{commit}"); //$NON-NLS-1$ - if (headId != null && rw.parseCommit(headId).getTree().getId().equals(treeId)) { - // No change. Do nothing. - return rw.parseCommit(headId); - } - - CommitBuilder commit = new CommitBuilder(); - commit.setTreeId(treeId); - if (headId != null) - commit.setParentIds(headId); - commit.setAuthor(author); - commit.setCommitter(author); - commit.setMessage(RepoText.get().repoCommitMessage); - - ObjectId commitId = inserter.insert(commit); - inserter.flush(); - - RefUpdate ru = repo.updateRef(targetBranch); - ru.setNewObjectId(commitId); - ru.setExpectedOldObjectId(headId != null ? headId : ObjectId.zeroId()); - Result rc = ru.update(rw); - - switch (rc) { - case NEW: - case FORCED: - case FAST_FORWARD: - // Successful. Do nothing. - break; - case REJECTED: - case LOCK_FAILURE: - throw new ConcurrentRefUpdateException( - MessageFormat.format( - JGitText.get().cannotLock, targetBranch), - ru.getRef(), - rc); - default: - throw new JGitInternalException(MessageFormat.format( - JGitText.get().updatingRefFailed, - targetBranch, commitId.name(), rc)); + long prevDelay = 0; + for (int i = 0; i < LOCK_FAILURE_MAX_RETRIES - 1; i++) { + try { + return commitTreeOnCurrentTip( + inserter, rw, treeId); + } catch (ConcurrentRefUpdateException e) { + prevDelay = FileUtils.delay(prevDelay, + LOCK_FAILURE_MIN_RETRY_DELAY_MILLIS, + LOCK_FAILURE_MAX_RETRY_DELAY_MILLIS); + Thread.sleep(prevDelay); + repo.getRefDatabase().refresh(); + } } - - return rw.parseCommit(commitId); - } catch (GitAPIException | IOException e) { + // In the last try, just propagate the exceptions + return commitTreeOnCurrentTip(inserter, rw, treeId); + } catch (GitAPIException | IOException | InterruptedException e) { throw new ManifestErrorException(e); } } @@ -746,6 +725,51 @@ public class RepoCommand extends GitCommand { } } + + private RevCommit commitTreeOnCurrentTip(ObjectInserter inserter, + RevWalk rw, ObjectId treeId) + throws IOException, ConcurrentRefUpdateException { + ObjectId headId = repo.resolve(targetBranch + "^{commit}"); //$NON-NLS-1$ + if (headId != null && rw.parseCommit(headId).getTree().getId().equals(treeId)) { + // No change. Do nothing. + return rw.parseCommit(headId); + } + + CommitBuilder commit = new CommitBuilder(); + commit.setTreeId(treeId); + if (headId != null) + commit.setParentIds(headId); + commit.setAuthor(author); + commit.setCommitter(author); + commit.setMessage(RepoText.get().repoCommitMessage); + + ObjectId commitId = inserter.insert(commit); + inserter.flush(); + + RefUpdate ru = repo.updateRef(targetBranch); + ru.setNewObjectId(commitId); + ru.setExpectedOldObjectId(headId != null ? headId : ObjectId.zeroId()); + Result rc = ru.update(rw); + switch (rc) { + case NEW: + case FORCED: + case FAST_FORWARD: + // Successful. Do nothing. + break; + case REJECTED: + case LOCK_FAILURE: + throw new ConcurrentRefUpdateException(MessageFormat + .format(JGitText.get().cannotLock, targetBranch), + ru.getRef(), rc); + default: + throw new JGitInternalException(MessageFormat.format( + JGitText.get().updatingRefFailed, + targetBranch, commitId.name(), rc)); + } + + return rw.parseCommit(commitId); + } + private void addSubmodule(String name, String url, String path, String revision, List copyfiles, List linkfiles, Git git) throws GitAPIException, IOException { -- cgit v1.2.3 From 1788b72d1af819dee6f371af9e1b0667f0ed8a64 Mon Sep 17 00:00:00 2001 From: Youssef Elghareeb Date: Wed, 12 May 2021 15:35:45 +0200 Subject: Skip detecting content renames for binary files This is similar to change Idbc2c29bd that skipped detecting content renames for large files. With this change, we added a new option in RenameDetector called "skipContentRenamesForBinaryFiles", that when set, causes binary files with any slight modification to be identified as added/deleted. The default for this boolean is false, so preserving current behaviour. Change-Id: I4770b1f69c60b1037025ddd0940ba86df6047299 --- .../org/eclipse/jgit/diff/RenameDetectorTest.java | 51 ++++++++++++++++++++++ .../src/org/eclipse/jgit/diff/RenameDetector.java | 28 ++++++++++++ .../src/org/eclipse/jgit/diff/SimilarityIndex.java | 13 ++++-- .../jgit/diff/SimilarityRenameDetector.java | 26 +++++++++-- 4 files changed, 111 insertions(+), 7 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java index 2ea3cd7ebe..5edb60ce37 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/RenameDetectorTest.java @@ -579,6 +579,57 @@ public class RenameDetectorTest extends RepositoryTestCase { assertDelete(PATH_Q, bId, FileMode.REGULAR_FILE, entries.get(1)); } + @Test + public void testExactRenameForBinaryFile_isIdentified() throws Exception { + ObjectId aId = blob("a\nb\nc\n\0\0\0\0d\n"); + + DiffEntry a = DiffEntry.add(PATH_A, aId); + DiffEntry b = DiffEntry.delete(PATH_Q, aId); + + rd.add(a); + rd.add(b); + + List entries = rd.compute(); + assertEquals(1, entries.size()); + assertRename(b, a, 100, entries.get(0)); + } + + @Test + public void testInexactRenameForBinaryFile_identifiedByDefault() throws Exception { + ObjectId aId = blob("a\nb\nc\n\0\0\0\0d\n"); + ObjectId bId = blob("a\nb\nc\n\0\0\0d\n"); + + DiffEntry a = DiffEntry.add(PATH_A, aId); + DiffEntry b = DiffEntry.delete(PATH_Q, bId); + + rd.add(a); + rd.add(b); + rd.setRenameScore(40); + + List entries = rd.compute(); + assertEquals(1, entries.size()); + assertRename(b, a, 50, entries.get(0)); + } + + @Test + public void testInexactRenameForBinaryFile_notIdentifiedIfSkipParameterSet() throws Exception { + ObjectId aId = blob("a\nb\nc\n\0\0\0\0d\n"); + ObjectId bId = blob("a\nb\nc\n\0\0\0d\n"); + + DiffEntry a = DiffEntry.add(PATH_A, aId); + DiffEntry b = DiffEntry.delete(PATH_Q, bId); + + rd.add(a); + rd.add(b); + rd.setRenameScore(40); + rd.setSkipContentRenamesForBinaryFiles(true); + + List entries = rd.compute(); + assertEquals(2, entries.size()); + assertAdd(PATH_A, aId, FileMode.REGULAR_FILE, entries.get(0)); + assertDelete(PATH_Q, bId, FileMode.REGULAR_FILE, entries.get(1)); + } + @Test public void testSetRenameScore_IllegalArgs() throws Exception { try { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java index 75784c2556..ba1f63b680 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java @@ -104,6 +104,13 @@ public class RenameDetector { */ private int bigFileThreshold = DEFAULT_BIG_FILE_THRESHOLD; + /** + * Skip detecting content renames for binary files. Content renames are + * those that are not exact, that is with a slight content modification + * between the two files. + */ + private boolean skipContentRenamesForBinaryFiles = false; + /** Set if the number of adds or deletes was over the limit. */ private boolean overRenameLimit; @@ -235,6 +242,26 @@ public class RenameDetector { this.bigFileThreshold = threshold; } + /** + * Get skipping detecting content renames for binary files. + * + * @return true if content renames should be skipped for binary files, false otherwise. + * @since 5.12 + */ + public boolean getSkipContentRenamesForBinaryFiles() { + return skipContentRenamesForBinaryFiles; + } + + /** + * Sets skipping detecting content renames for binary files. + * + * @param value true if content renames should be skipped for binary files, false otherwise. + * @since 5.12 + */ + public void setSkipContentRenamesForBinaryFiles(boolean value) { + this.skipContentRenamesForBinaryFiles = value; + } + /** * Check if the detector is over the rename limit. *

@@ -521,6 +548,7 @@ public class RenameDetector { d = new SimilarityRenameDetector(reader, deleted, added); d.setRenameScore(getRenameScore()); d.setBigFileThreshold(getBigFileThreshold()); + d.setSkipBinaryFiles(getSkipContentRenamesForBinaryFiles()); d.compute(pm); overRenameLimit |= d.isTableOverflow(); deleted = d.getLeftOverSources(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java index fb6e5df589..661369b86a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java @@ -102,6 +102,15 @@ public class SimilarityIndex { idGrowAt = growAt(idHashBits); } + static boolean isBinary(ObjectLoader obj) throws IOException { + if (obj.isLarge()) { + try (ObjectStream in1 = obj.openStream()) { + return RawText.isBinary(in1); + } + } + return RawText.isBinary(obj.getCachedBytes()); + } + void hash(ObjectLoader obj) throws MissingObjectException, IOException, TableFullException { if (obj.isLarge()) { @@ -115,9 +124,7 @@ public class SimilarityIndex { private void hashLargeObject(ObjectLoader obj) throws IOException, TableFullException { boolean text; - try (ObjectStream in1 = obj.openStream()) { - text = !RawText.isBinary(in1); - } + text = !isBinary(obj); try (ObjectStream in2 = obj.openStream()) { hash(in2, in2.getSize(), text); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java index 082f31d178..5871b4aeea 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java @@ -26,6 +26,7 @@ import org.eclipse.jgit.errors.CancelledException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ProgressMonitor; class SimilarityRenameDetector { @@ -87,6 +88,9 @@ class SimilarityRenameDetector { */ private int bigFileThreshold = DEFAULT_BIG_FILE_THRESHOLD; + /** Skip content renames for binary files. */ + private boolean skipBinaryFiles = false; + /** Set if any {@link SimilarityIndex.TableFullException} occurs. */ private boolean tableOverflow; @@ -107,6 +111,10 @@ class SimilarityRenameDetector { bigFileThreshold = threshold; } + void setSkipBinaryFiles(boolean value) { + skipBinaryFiles = value; + } + void compute(ProgressMonitor pm) throws IOException, CancelledException { if (pm == null) pm = NullProgressMonitor.INSTANCE; @@ -271,7 +279,12 @@ class SimilarityRenameDetector { if (s == null) { try { - s = hash(OLD, srcEnt); + ObjectLoader loader = reader.open(OLD, srcEnt); + if (skipBinaryFiles && SimilarityIndex.isBinary(loader)) { + pm.update(1); + continue SRC; + } + s = hash(loader); } catch (TableFullException tableFull) { tableOverflow = true; continue SRC; @@ -280,7 +293,12 @@ class SimilarityRenameDetector { SimilarityIndex d; try { - d = hash(NEW, dstEnt); + ObjectLoader loader = reader.open(NEW, dstEnt); + if (skipBinaryFiles && SimilarityIndex.isBinary(loader)) { + pm.update(1); + continue; + } + d = hash(loader); } catch (TableFullException tableFull) { if (dstTooLarge == null) dstTooLarge = new BitSet(dsts.size()); @@ -364,10 +382,10 @@ class SimilarityRenameDetector { return (((dirScoreLtr + dirScoreRtl) * 25) + (fileScore * 50)) / 100; } - private SimilarityIndex hash(DiffEntry.Side side, DiffEntry ent) + private SimilarityIndex hash(ObjectLoader objectLoader) throws IOException, TableFullException { SimilarityIndex r = new SimilarityIndex(); - r.hash(reader.open(side, ent)); + r.hash(objectLoader); r.sort(); return r; } -- cgit v1.2.3 From a14bc9bb6935dc0784ba2277751b64fb735a2a23 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 23 Apr 2021 16:52:54 +0200 Subject: Clarify operator precedence to fix errorprone error Errorprone raised error OperatorPrecedence in bazel build. Change-Id: Ibab601e67d4d5cafe9a7d900c78b0d432181a073 Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java index bf93d77f7e..6fbb4c5a07 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java @@ -881,8 +881,8 @@ public class OpenSshConfigFile implements SshConfigStore { public String substitute(String input, String allowed, boolean withEnv) { if (input == null || input.length() <= 1 - || input.indexOf('%') < 0 - && (!withEnv || input.indexOf("${") < 0)) { //$NON-NLS-1$ + || (input.indexOf('%') < 0 + && (!withEnv || input.indexOf("${") < 0))) { //$NON-NLS-1$ return input; } StringBuilder builder = new StringBuilder(); -- cgit v1.2.3 From f6b9b392e7c31cee6954aedc12001de3c17731a3 Mon Sep 17 00:00:00 2001 From: Demetr Starshov Date: Wed, 2 Jun 2021 16:19:39 -0700 Subject: Fixing visibility for HostEntry constructors. HostEntry class was public with empty constructor, so adding constructors with default visibility actually reduced visibility of constructor. Change-Id: I4c996c0559102084946ba49a71afe10dda5e0f95 --- .../jgit/internal/transport/ssh/OpenSshConfigFile.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java index 6fbb4c5a07..228c25f0a5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java @@ -482,12 +482,18 @@ public class OpenSshConfigFile implements SshConfigStore { private final List patterns; - // Constructor used to build the merged entry; never matches anything - HostEntry() { + /** + * Constructor used to build the merged entry; never matches anything + */ + public HostEntry() { this.patterns = Collections.emptyList(); } - HostEntry(List patterns) { + /** + * @param patterns + * to be used in matching against host name. + */ + public HostEntry(List patterns) { this.patterns = patterns; } -- cgit v1.2.3 From c2f9acdc32a5c675a1a307dc5e4e5ba93be5a7f9 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 7 Jun 2021 00:25:32 +0200 Subject: [errorprone] Make operator precedence explicit in OpenSshConfigFile This fixes the errorprone warning OperatorPrecedence. Change-Id: I4c7dafa5ac8e1d58fa15cf91fe1b3cf3f182d536 --- .../org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java index bf93d77f7e..6fbb4c5a07 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/ssh/OpenSshConfigFile.java @@ -881,8 +881,8 @@ public class OpenSshConfigFile implements SshConfigStore { public String substitute(String input, String allowed, boolean withEnv) { if (input == null || input.length() <= 1 - || input.indexOf('%') < 0 - && (!withEnv || input.indexOf("${") < 0)) { //$NON-NLS-1$ + || (input.indexOf('%') < 0 + && (!withEnv || input.indexOf("${") < 0))) { //$NON-NLS-1$ return input; } StringBuilder builder = new StringBuilder(); -- cgit v1.2.3 From ae692779ce69155c970bcedaabe2edc3366fe6a2 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 7 Jun 2021 00:28:48 +0200 Subject: [errorprone] Fix warning InputStreamSlowMultibyteRead Change-Id: I50dace6e310016c04f524eb0cfcce0da05fadd47 --- .../src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java | 5 +++++ .../src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java | 5 +++++ 2 files changed, 10 insertions(+) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java index 7f0d87f0da..9eceeb8117 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryDeltaInputStream.java @@ -82,6 +82,11 @@ public class BinaryDeltaInputStream extends InputStream { return b; } + @Override + public int read(byte[] b, int off, int len) throws IOException { + return super.read(b, off, len); + } + private void initialize() throws IOException { long baseSize = readVarInt(delta); if (baseSize > Integer.MAX_VALUE || baseSize < 0 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java index 57b2d7a4b2..4f940d77a0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/BinaryHunkInputStream.java @@ -58,6 +58,11 @@ public class BinaryHunkInputStream extends InputStream { return -1; } + @Override + public int read(byte[] b, int off, int len) throws IOException { + return super.read(b, off, len); + } + @Override public void close() throws IOException { in.close(); -- cgit v1.2.3 From 64d0aaa2b624e9c011390d817a6d1fef335bf7b6 Mon Sep 17 00:00:00 2001 From: Ronald Bhuleskar Date: Wed, 9 Jun 2021 15:41:09 -0700 Subject: Teach independent negotiation (no pack file) using an option "wait-for-done" From Git commit 9c1e657a8f: Currently, the packfile negotiation step within a Git fetch cannot be done independent of sending the packfile, even though there is at least one application wherein this is useful - push negotiation. Therefore, make it possible for this negotiation step to be done independently. This feature is for protocol v2 only. In the protocol, the main hindrance towards independent negotiation is that the server can unilaterally decide to send the packfile. This is solved by a "wait-for-done" argument: the server will then wait for the client to say "done". In practice, the client will never say it; instead it will cease requests once it is satisfied. Advertising the server capability option "wait-for-done" is behind the transport config: uploadpack.advertisewaitfordone, which by default is false. Change-Id: I5ebd3e99ad76b8943597216e23ced2ed38eb5224 --- .../org/eclipse/jgit/transport/UploadPackTest.java | 102 +++++++++++++++++++++ .../org/eclipse/jgit/transport/FetchV2Request.java | 29 +++++- .../jgit/transport/GitProtocolConstants.java | 7 ++ .../eclipse/jgit/transport/ProtocolV2Parser.java | 3 + .../org/eclipse/jgit/transport/TransferConfig.java | 13 +++ .../src/org/eclipse/jgit/transport/UploadPack.java | 52 ++++++++--- 6 files changed, 189 insertions(+), 17 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java index 5045e9464e..b0b5f68efa 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java @@ -998,6 +998,108 @@ public class UploadPackTest { assertTrue(client.getObjectDatabase().has(barChild.toObjectId())); } + @Test + public void testV2FetchWithoutWaitForDoneReceivesPackfile() + throws Exception { + String commonInBlob = "abcdefghijklmnopqrstuvwxyz"; + + RevBlob parentBlob = remote.blob(commonInBlob + "a"); + RevCommit parent = remote + .commit(remote.tree(remote.file("foo", parentBlob))); + remote.update("branch1", parent); + + RevCommit localParent = null; + RevCommit localChild = null; + try (TestRepository local = new TestRepository<>( + client)) { + RevBlob localParentBlob = local.blob(commonInBlob + "a"); + localParent = local + .commit(local.tree(local.file("foo", localParentBlob))); + RevBlob localChildBlob = local.blob(commonInBlob + "b"); + localChild = local.commit( + local.tree(local.file("foo", localChildBlob)), localParent); + local.update("branch1", localChild); + } + + ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n", + PacketLineIn.delimiter(), + "have " + localParent.toObjectId().getName() + "\n", + "have " + localChild.toObjectId().getName() + "\n", + PacketLineIn.end()); + PacketLineIn pckIn = new PacketLineIn(recvStream); + assertThat(pckIn.readString(), is("acknowledgments")); + assertThat(Arrays.asList(pckIn.readString()), + hasItems("ACK " + parent.toObjectId().getName())); + assertThat(pckIn.readString(), is("ready")); + assertTrue(PacketLineIn.isDelimiter(pckIn.readString())); + assertThat(pckIn.readString(), is("packfile")); + parsePack(recvStream); + } + + @Test + public void testV2FetchWithWaitForDoneOnlyDoesNegotiation() + throws Exception { + String commonInBlob = "abcdefghijklmnopqrstuvwxyz"; + + RevBlob parentBlob = remote.blob(commonInBlob + "a"); + RevCommit parent = remote + .commit(remote.tree(remote.file("foo", parentBlob))); + remote.update("branch1", parent); + + RevCommit localParent = null; + RevCommit localChild = null; + try (TestRepository local = new TestRepository<>( + client)) { + RevBlob localParentBlob = local.blob(commonInBlob + "a"); + localParent = local + .commit(local.tree(local.file("foo", localParentBlob))); + RevBlob localChildBlob = local.blob(commonInBlob + "b"); + localChild = local.commit( + local.tree(local.file("foo", localChildBlob)), localParent); + local.update("branch1", localChild); + } + + ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n", + PacketLineIn.delimiter(), "wait-for-done\n", + "have " + localParent.toObjectId().getName() + "\n", + "have " + localChild.toObjectId().getName() + "\n", + PacketLineIn.end()); + PacketLineIn pckIn = new PacketLineIn(recvStream); + assertThat(pckIn.readString(), is("acknowledgments")); + assertThat(Arrays.asList(pckIn.readString()), + hasItems("ACK " + parent.toObjectId().getName())); + assertTrue(PacketLineIn.isEnd(pckIn.readString())); + } + + @Test + public void testV2FetchWithWaitForDoneOnlyDoesNegotiationAndNothingToAck() + throws Exception { + String commonInBlob = "abcdefghijklmnopqrstuvwxyz"; + + RevCommit localParent = null; + RevCommit localChild = null; + try (TestRepository local = new TestRepository<>( + client)) { + RevBlob localParentBlob = local.blob(commonInBlob + "a"); + localParent = local + .commit(local.tree(local.file("foo", localParentBlob))); + RevBlob localChildBlob = local.blob(commonInBlob + "b"); + localChild = local.commit( + local.tree(local.file("foo", localChildBlob)), localParent); + local.update("branch1", localChild); + } + + ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n", + PacketLineIn.delimiter(), "wait-for-done\n", + "have " + localParent.toObjectId().getName() + "\n", + "have " + localChild.toObjectId().getName() + "\n", + PacketLineIn.end()); + PacketLineIn pckIn = new PacketLineIn(recvStream); + assertThat(pckIn.readString(), is("acknowledgments")); + assertThat(pckIn.readString(), is("NAK")); + assertTrue(PacketLineIn.isEnd(pckIn.readString())); + } + @Test public void testV2FetchThinPack() throws Exception { String commonInBlob = "abcdefghijklmnopqrstuvwxyz"; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java index ea639332ea..50fb9d2262 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV2Request.java @@ -36,6 +36,8 @@ public final class FetchV2Request extends FetchRequest { private final boolean doneReceived; + private final boolean waitForDone; + @NonNull private final List serverOptions; @@ -50,7 +52,8 @@ public final class FetchV2Request extends FetchRequest { @NonNull Set clientShallowCommits, int deepenSince, @NonNull List deepenNotRefs, int depth, @NonNull FilterSpec filterSpec, - boolean doneReceived, @NonNull Set clientCapabilities, + boolean doneReceived, boolean waitForDone, + @NonNull Set clientCapabilities, @Nullable String agent, @NonNull List serverOptions, boolean sidebandAll, @NonNull List packfileUriProtocols) { super(wantIds, depth, clientShallowCommits, filterSpec, @@ -59,6 +62,7 @@ public final class FetchV2Request extends FetchRequest { this.peerHas = requireNonNull(peerHas); this.wantedRefs = requireNonNull(wantedRefs); this.doneReceived = doneReceived; + this.waitForDone = waitForDone; this.serverOptions = requireNonNull(serverOptions); this.sidebandAll = sidebandAll; this.packfileUriProtocols = packfileUriProtocols; @@ -90,7 +94,14 @@ public final class FetchV2Request extends FetchRequest { } /** - * Options received in server-option lines. The caller can choose to act on + * @return true if the request had a "wait-for-done" line + */ + boolean wasWaitForDoneReceived() { + return waitForDone; + } + + /** + * Options received in server-option lines. The caller can choose to act on * these in an application-specific way * * @return Immutable list of server options received in the request @@ -141,6 +152,8 @@ public final class FetchV2Request extends FetchRequest { boolean doneReceived; + boolean waitForDone; + @Nullable String agent; @@ -279,6 +292,16 @@ public final class FetchV2Request extends FetchRequest { return this; } + /** + * Mark that the "wait-for-done" line has been received. + * + * @return this builder + */ + Builder setWaitForDone() { + waitForDone = true; + return this; + } + /** * Value of an agent line received after the command and before the * arguments. E.g. "agent=a.b.c/1.0" should set "a.b.c/1.0". @@ -328,7 +351,7 @@ public final class FetchV2Request extends FetchRequest { FetchV2Request build() { return new FetchV2Request(peerHas, wantedRefs, wantIds, clientShallowCommits, deepenSince, deepenNotRefs, - depth, filterSpec, doneReceived, clientCapabilities, + depth, filterSpec, doneReceived, waitForDone, clientCapabilities, agent, Collections.unmodifiableList(serverOptions), sidebandAll, Collections.unmodifiableList(packfileUriProtocols)); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java index 36fce7a3f5..048097163e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java @@ -148,6 +148,13 @@ public final class GitProtocolConstants { */ public static final String OPTION_SIDEBAND_ALL = "sideband-all"; //$NON-NLS-1$ + /** + * The server waits for client to send "done" before sending any packs back. + * + * @since 5.12 + */ + public static final String OPTION_WAIT_FOR_DONE = "wait-for-done"; //$NON-NLS-1$ + /** * The client supports atomic pushes. If this option is used, the server * will update all refs within one atomic transaction. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java index faccc25185..92f0133f5a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV2Parser.java @@ -19,6 +19,7 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SERVER_OPTI import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDEBAND_ALL; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK; +import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WAIT_FOR_DONE; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WANT_REF; import java.io.IOException; @@ -123,6 +124,8 @@ final class ProtocolV2Parser { reqBuilder.addPeerHas(ObjectId.fromString(line2.substring(5))); } else if (line2.equals("done")) { //$NON-NLS-1$ reqBuilder.setDoneReceived(); + } else if (line2.equals(OPTION_WAIT_FOR_DONE)) { + reqBuilder.setWaitForDone(); } else if (line2.equals(OPTION_THIN_PACK)) { reqBuilder.addClientCapability(OPTION_THIN_PACK); } else if (line2.equals(OPTION_NO_PROGRESS)) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java index 83ffd4123a..c1d630c5ce 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java @@ -120,7 +120,10 @@ public class TransferConfig { private final boolean allowReachableSha1InWant; private final boolean allowFilter; private final boolean allowSidebandAll; + private final boolean advertiseSidebandAll; + private final boolean advertiseWaitForDone; + final @Nullable ProtocolVersion protocolVersion; final String[] hideRefs; @@ -206,6 +209,8 @@ public class TransferConfig { "uploadpack", "allowsidebandall", false); advertiseSidebandAll = rc.getBoolean("uploadpack", "advertisesidebandall", false); + advertiseWaitForDone = rc.getBoolean("uploadpack", + "advertisewaitfordone", false); } /** @@ -304,6 +309,14 @@ public class TransferConfig { return advertiseSidebandAll && allowSidebandAll; } + /** + * @return true to advertise wait-for-done all to the clients + * @since 5.12 + */ + public boolean isAdvertiseWaitForDone() { + return advertiseWaitForDone; + } + /** * Get {@link org.eclipse.jgit.transport.RefFilter} respecting configured * hidden refs. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 7f1ddaab2e..ecf1751932 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -33,6 +33,7 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDEBAND_AL import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K; import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_THIN_PACK; +import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_WAIT_FOR_DONE; import static org.eclipse.jgit.transport.GitProtocolConstants.VERSION_2_REQUEST; import static org.eclipse.jgit.util.RefMap.toRefMap; @@ -1192,9 +1193,10 @@ public class UploadPack { walk.assumeShallow(req.getClientShallowCommits()); if (req.wasDoneReceived()) { - processHaveLines(req.getPeerHas(), ObjectId.zeroId(), + processHaveLines( + req.getPeerHas(), ObjectId.zeroId(), new PacketLineOut(NullOutputStream.INSTANCE, false), - accumulator); + accumulator, req.wasWaitForDoneReceived() ? Option.WAIT_FOR_DONE : Option.NONE); } else { pckOut.writeString( GitProtocolConstants.SECTION_ACKNOWLEDGMENTS + '\n'); @@ -1205,8 +1207,8 @@ public class UploadPack { } processHaveLines(req.getPeerHas(), ObjectId.zeroId(), new PacketLineOut(NullOutputStream.INSTANCE, false), - accumulator); - if (okToGiveUp()) { + accumulator, Option.NONE); + if (!req.wasWaitForDoneReceived() && okToGiveUp()) { pckOut.writeString("ready\n"); //$NON-NLS-1$ } else if (commonBase.isEmpty()) { pckOut.writeString("NAK\n"); //$NON-NLS-1$ @@ -1214,7 +1216,7 @@ public class UploadPack { sectionSent = true; } - if (req.wasDoneReceived() || okToGiveUp()) { + if (req.wasDoneReceived() || (!req.wasWaitForDoneReceived() && okToGiveUp())) { if (mayHaveShallow) { if (sectionSent) pckOut.writeDelim(); @@ -1312,6 +1314,9 @@ public class UploadPack { ? OPTION_SIDEBAND_ALL + ' ' : "") + (cachedPackUriProvider != null ? "packfile-uris " : "") + + (transferConfig.isAdvertiseWaitForDone() + ? OPTION_WAIT_FOR_DONE + ' ' + : "") + OPTION_SHALLOW); caps.add(CAPABILITY_SERVER_OPTION); return caps; @@ -1656,7 +1661,7 @@ public class UploadPack { } if (PacketLineIn.isEnd(line)) { - last = processHaveLines(peerHas, last, pckOut, accumulator); + last = processHaveLines(peerHas, last, pckOut, accumulator, Option.NONE); if (commonBase.isEmpty() || multiAck != MultiAck.OFF) pckOut.writeString("NAK\n"); //$NON-NLS-1$ if (noDone && sentReady) { @@ -1671,7 +1676,7 @@ public class UploadPack { peerHas.add(ObjectId.fromString(line.substring(5))); accumulator.haves++; } else if (line.equals("done")) { //$NON-NLS-1$ - last = processHaveLines(peerHas, last, pckOut, accumulator); + last = processHaveLines(peerHas, last, pckOut, accumulator, Option.NONE); if (commonBase.isEmpty()) pckOut.writeString("NAK\n"); //$NON-NLS-1$ @@ -1687,8 +1692,14 @@ public class UploadPack { } } + private enum Option { + WAIT_FOR_DONE, + NONE; + } + private ObjectId processHaveLines(List peerHas, ObjectId last, - PacketLineOut out, PackStatistics.Accumulator accumulator) + PacketLineOut out, PackStatistics.Accumulator accumulator, + Option option) throws IOException { preUploadHook.onBeginNegotiateRound(this, wantIds, peerHas.size()); if (wantAll.isEmpty() && !wantIds.isEmpty()) @@ -1754,6 +1765,18 @@ public class UploadPack { // create a pack at this point, let the client know so it stops // telling us about its history. // + if (option != Option.WAIT_FOR_DONE) { + sentReady = shouldGiveUp(peerHas, out, missCnt); + } + + preUploadHook.onEndNegotiateRound(this, wantAll, haveCnt, missCnt, sentReady); + peerHas.clear(); + return last; + } + + private boolean shouldGiveUp(List peerHas, PacketLineOut out, int missCnt) + throws IOException { + boolean sentReady = false; boolean didOkToGiveUp = false; if (0 < missCnt) { for (int i = peerHas.size() - 1; i >= 0; i--) { @@ -1765,10 +1788,12 @@ public class UploadPack { case OFF: break; case CONTINUE: - out.writeString("ACK " + id.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString( + "ACK " + id.name() + " continue\n"); //$NON-NLS-1$ //$NON-NLS-2$ break; case DETAILED: - out.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ + out.writeString( + "ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ sentReady = true; break; } @@ -1778,15 +1803,14 @@ public class UploadPack { } } - if (multiAck == MultiAck.DETAILED && !didOkToGiveUp && okToGiveUp()) { + if (multiAck == MultiAck.DETAILED && !didOkToGiveUp + && okToGiveUp()) { ObjectId id = peerHas.get(peerHas.size() - 1); out.writeString("ACK " + id.name() + " ready\n"); //$NON-NLS-1$ //$NON-NLS-2$ sentReady = true; } - preUploadHook.onEndNegotiateRound(this, wantAll, haveCnt, missCnt, sentReady); - peerHas.clear(); - return last; + return sentReady; } private void parseWants(PackStatistics.Accumulator accumulator) throws IOException { -- cgit v1.2.3 From 01b2c4fc90849f43042ffb0bb40fa3ab69541245 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Mon, 14 Jun 2021 21:22:13 +0200 Subject: Fix @since from commit 64d0aaa2 That commit was submitted on master between the 5.12.0 release and the 5.13.0 version bump. Change-Id: I679e818bfc5a4695b66548add9a83a22c89a4ffc Signed-off-by: Thomas Wolf --- .../src/org/eclipse/jgit/transport/GitProtocolConstants.java | 2 +- org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'org.eclipse.jgit/src/org/eclipse') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java index 048097163e..c5e52bef98 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/GitProtocolConstants.java @@ -151,7 +151,7 @@ public final class GitProtocolConstants { /** * The server waits for client to send "done" before sending any packs back. * - * @since 5.12 + * @since 5.13 */ public static final String OPTION_WAIT_FOR_DONE = "wait-for-done"; //$NON-NLS-1$ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java index c1d630c5ce..da97f1e580 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransferConfig.java @@ -311,7 +311,7 @@ public class TransferConfig { /** * @return true to advertise wait-for-done all to the clients - * @since 5.12 + * @since 5.13 */ public boolean isAdvertiseWaitForDone() { return advertiseWaitForDone; -- cgit v1.2.3