From 017032c465ff20ef255914eff9b58da67c5e46b4 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 25 Mar 2015 01:31:12 +0100 Subject: [PATCH 01/26] Fix IllegalArgumentException in AmazonS3 JGit hit IllegalArgumentException: invalid content length when pushing large packs to S3. Bug: 463015 Change-Id: Iddf50d90c7e3ccb15b9ff71233338c6b204b3648 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java index 705a84613..f43ea637a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java @@ -489,16 +489,14 @@ private void putImpl(final String bucket, final String key, final String md5str = Base64.encodeBytes(csum); final long len = buf.length(); - final String lenstr = String.valueOf(len); for (int curAttempt = 0; curAttempt < maxAttempts; curAttempt++) { final HttpURLConnection c = open("PUT", bucket, key); //$NON-NLS-1$ - c.setRequestProperty("Content-Length", lenstr); //$NON-NLS-1$ + c.setFixedLengthStreamingMode(len); c.setRequestProperty("Content-MD5", md5str); //$NON-NLS-1$ c.setRequestProperty(X_AMZ_ACL, acl); encryption.request(c, X_AMZ_META); authorize(c); c.setDoOutput(true); - c.setFixedLengthStreamingMode((int) len); monitor.beginTask(monitorTask, (int) (len / 1024)); final OutputStream os = c.getOutputStream(); try { From 5362e1d41c8e3ce6e63fbab9882dfbc4429903e0 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 25 Apr 2015 00:22:59 +0200 Subject: [PATCH 02/26] Delete deprecated checkoutEntry() methods in DirCacheCheckout Change-Id: I7fcf6534e6092ba87360ccd68a7dd7466c5c8911 Signed-off-by: Matthias Sohn --- .../jgit/dircache/DirCacheCheckout.java | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index 99e022bfe..f1241652b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -1129,73 +1129,6 @@ private boolean isModifiedSubtree_IndexTree(String path, ObjectId tree) } } - /** - * Updates the file in the working tree with content and mode from an entry - * in the index. The new content is first written to a new temporary file in - * the same directory as the real file. Then that new file is renamed to the - * final filename. Use this method only for checkout of a single entry. - * Otherwise use - * {@code checkoutEntry(Repository, File f, DirCacheEntry, ObjectReader)} - * instead which allows to reuse one {@code ObjectReader} for multiple - * entries. - * - *

- * TODO: this method works directly on File IO, we may need another - * abstraction (like WorkingTreeIterator). This way we could tell e.g. - * Eclipse that Files in the workspace got changed - *

- * - * @param repository - * @param f - * this parameter is ignored. - * @param entry - * the entry containing new mode and content - * @throws IOException - * @deprecated Use the overloaded form that accepts {@link ObjectReader}. - */ - @Deprecated - public static void checkoutEntry(final Repository repository, File f, - DirCacheEntry entry) throws IOException { - ObjectReader or = repository.newObjectReader(); - try { - checkoutEntry(repository, f, entry, or); - } finally { - or.release(); - } - } - - /** - * Updates the file in the working tree with content and mode from an entry - * in the index. The new content is first written to a new temporary file in - * the same directory as the real file. Then that new file is renamed to the - * final filename. - * - *

- * TODO: this method works directly on File IO, we may need another - * abstraction (like WorkingTreeIterator). This way we could tell e.g. - * Eclipse that Files in the workspace got changed - *

- * - * @param repo - * @param f - * this parameter is ignored. - * @param entry - * the entry containing new mode and content - * @param or - * object reader to use for checkout - * @throws IOException - * @deprecated Do not pass File object. - */ - @Deprecated - public static void checkoutEntry(final Repository repo, File f, - DirCacheEntry entry, ObjectReader or) throws IOException { - if (f == null || repo.getWorkTree() == null) - throw new IllegalArgumentException(); - if (!f.equals(new File(repo.getWorkTree(), entry.getPathString()))) - throw new IllegalArgumentException(); - checkoutEntry(repo, entry, or); - } - /** * Updates the file in the working tree with content and mode from an entry * in the index. The new content is first written to a new temporary file in From ffdacc2bbbcbcdcc0cfe0074f8e5b33817eca4e5 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 25 Apr 2015 00:43:20 +0200 Subject: [PATCH 03/26] Delete deprecated class IgnoreRule Was replaced by FastIgnoreRule in 3.6 Change-Id: Ia9c3d1231a5d97f3f5bddc81113954c9f9d8ee1e Signed-off-by: Matthias Sohn --- .../attributes/AttributesMatcherTest.java | 3 - .../jgit/ignore/FastIgnoreRuleTest.java | 143 ++----- .../ignore/IgnoreMatcherParametrizedTest.java | 55 +-- .../jgit/ignore/IgnoreMatcherTest.java | 400 ------------------ .../ignore/IgnoreRuleSpecialCasesTest.java | 61 +-- .../org/eclipse/jgit/ignore/IgnoreRule.java | 278 ------------ 6 files changed, 66 insertions(+), 874 deletions(-) delete mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherTest.java delete mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/ignore/IgnoreRule.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesMatcherTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesMatcherTest.java index 686540692..9f82b8a1e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesMatcherTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesMatcherTest.java @@ -51,9 +51,6 @@ /** * Tests git attributes pattern matches - *

- * Inspired by {@link org.eclipse.jgit.ignore.IgnoreMatcherTest} - *

*/ public class AttributesMatcherTest { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java index 007bdcc5f..a4b799a72 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java @@ -47,30 +47,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeFalse; -import static org.junit.Assume.assumeTrue; - -import java.util.Arrays; import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; -@SuppressWarnings("deprecation") -@RunWith(Parameterized.class) public class FastIgnoreRuleTest { - @Parameters(name = "OldRule? {0}") - public static Iterable data() { - return Arrays.asList(new Boolean[][] { { Boolean.FALSE }, - { Boolean.TRUE } }); - } - - @Parameter - public Boolean useOldRule; - @Test public void testSimpleCharClass() { assertMatched("[a]", "a"); @@ -385,58 +366,46 @@ public void testSegmentsDoNotMatch() { assertNotMatched("a/b/", "c/a/b/c"); } - @SuppressWarnings("boxing") @Test public void testWildmatch() { - if (useOldRule) - System.err - .println("IgnoreRule can't understand wildmatch rules, skipping testWildmatch!"); + assertMatched("**/a/b", "a/b"); + assertMatched("**/a/b", "c/a/b"); + assertMatched("**/a/b", "c/d/a/b"); + assertMatched("**/**/a/b", "c/d/a/b"); - Boolean assume = useOldRule; - assertMatched("**/a/b", "a/b", assume); - assertMatched("**/a/b", "c/a/b", assume); - assertMatched("**/a/b", "c/d/a/b", assume); - assertMatched("**/**/a/b", "c/d/a/b", assume); + assertMatched("/**/a/b", "a/b"); + assertMatched("/**/a/b", "c/a/b"); + assertMatched("/**/a/b", "c/d/a/b"); + assertMatched("/**/**/a/b", "c/d/a/b"); - assertMatched("/**/a/b", "a/b", assume); - assertMatched("/**/a/b", "c/a/b", assume); - assertMatched("/**/a/b", "c/d/a/b", assume); - assertMatched("/**/**/a/b", "c/d/a/b", assume); + assertMatched("a/b/**", "a/b"); + assertMatched("a/b/**", "a/b/c"); + assertMatched("a/b/**", "a/b/c/d/"); + assertMatched("a/b/**/**", "a/b/c/d"); - assertMatched("a/b/**", "a/b", assume); - assertMatched("a/b/**", "a/b/c", assume); - assertMatched("a/b/**", "a/b/c/d/", assume); - assertMatched("a/b/**/**", "a/b/c/d", assume); + assertMatched("**/a/**/b", "c/d/a/b"); + assertMatched("**/a/**/b", "c/d/a/e/b"); + assertMatched("**/**/a/**/**/b", "c/d/a/e/b"); - assertMatched("**/a/**/b", "c/d/a/b", assume); - assertMatched("**/a/**/b", "c/d/a/e/b", assume); - assertMatched("**/**/a/**/**/b", "c/d/a/e/b", assume); + assertMatched("/**/a/**/b", "c/d/a/b"); + assertMatched("/**/a/**/b", "c/d/a/e/b"); + assertMatched("/**/**/a/**/**/b", "c/d/a/e/b"); - assertMatched("/**/a/**/b", "c/d/a/b", assume); - assertMatched("/**/a/**/b", "c/d/a/e/b", assume); - assertMatched("/**/**/a/**/**/b", "c/d/a/e/b", assume); + assertMatched("a/**/b", "a/b"); + assertMatched("a/**/b", "a/c/b"); + assertMatched("a/**/b", "a/c/d/b"); + assertMatched("a/**/**/b", "a/c/d/b"); - assertMatched("a/**/b", "a/b", assume); - assertMatched("a/**/b", "a/c/b", assume); - assertMatched("a/**/b", "a/c/d/b", assume); - assertMatched("a/**/**/b", "a/c/d/b", assume); - - assertMatched("a/**/b/**/c", "a/c/b/d/c", assume); - assertMatched("a/**/**/b/**/**/c", "a/c/b/d/c", assume); + assertMatched("a/**/b/**/c", "a/c/b/d/c"); + assertMatched("a/**/**/b/**/**/c", "a/c/b/d/c"); } - @SuppressWarnings("boxing") @Test public void testWildmatchDoNotMatch() { - if (useOldRule) - System.err - .println("IgnoreRule can't understand wildmatch rules, skipping testWildmatchDoNotMatch!"); - - Boolean assume = useOldRule; - assertNotMatched("**/a/b", "a/c/b", assume); - assertNotMatched("!/**/*.zip", "c/a/b.zip", assume); - assertNotMatched("!**/*.zip", "c/a/b.zip", assume); - assertNotMatched("a/**/b", "a/c/bb", assume); + assertNotMatched("**/a/b", "a/c/b"); + assertNotMatched("!/**/*.zip", "c/a/b.zip"); + assertNotMatched("!**/*.zip", "c/a/b.zip"); + assertNotMatched("a/**/b", "a/c/bb"); } @SuppressWarnings("unused") @@ -478,55 +447,43 @@ public void testSplit() { split("/a/b/c/", '/').toArray()); } - public void assertMatched(String pattern, String path, Boolean... assume) { + public void assertMatched(String pattern, String path) { boolean match = match(pattern, path); String result = path + " is " + (match ? "ignored" : "not ignored") + " via '" + pattern + "' rule"; - if (!match) + if (!match) { System.err.println(result); - if (assume.length == 0 || !assume[0].booleanValue()) - assertTrue("Expected a match for: " + pattern + " with: " + path, - match); - else - assumeTrue("Expected a match for: " + pattern + " with: " + path, + } + assertTrue("Expected a match for: " + pattern + " with: " + path, match); - if (pattern.startsWith("!")) + if (pattern.startsWith("!")) { pattern = pattern.substring(1); - else + } else { pattern = "!" + pattern; + } match = match(pattern, path); - if (assume.length == 0 || !assume[0].booleanValue()) - assertFalse("Expected no match for: " + pattern + " with: " + path, - match); - else - assumeFalse("Expected no match for: " + pattern + " with: " + path, - match); + assertFalse("Expected no match for: " + pattern + " with: " + path, + match); } - public void assertNotMatched(String pattern, String path, Boolean... assume) { + public void assertNotMatched(String pattern, String path) { boolean match = match(pattern, path); String result = path + " is " + (match ? "ignored" : "not ignored") + " via '" + pattern + "' rule"; - if (match) + if (match) { System.err.println(result); - if (assume.length == 0 || !assume[0].booleanValue()) - assertFalse("Expected no match for: " + pattern + " with: " + path, - match); - else - assumeFalse("Expected no match for: " + pattern + " with: " + path, + } + assertFalse("Expected no match for: " + pattern + " with: " + path, match); - if (pattern.startsWith("!")) + if (pattern.startsWith("!")) { pattern = pattern.substring(1); - else + } else { pattern = "!" + pattern; + } match = match(pattern, path); - if (assume.length == 0 || !assume[0].booleanValue()) - assertTrue("Expected a match for: " + pattern + " with: " + path, - match); - else - assumeTrue("Expected a match for: " + pattern + " with: " + path, + assertTrue("Expected a match for: " + pattern + " with: " + path, match); } @@ -542,16 +499,6 @@ public void assertNotMatched(String pattern, String path, Boolean... assume) { */ private boolean match(String pattern, String target) { boolean isDirectory = target.endsWith("/"); - if (useOldRule.booleanValue()) { - IgnoreRule r = new IgnoreRule(pattern); - // If speed of this test is ever an issue, we can use a presetRule - // field - // to avoid recompiling a pattern each time. - boolean match = r.isMatch(target, isDirectory); - if (r.getNegation()) - match = !match; - return match; - } FastIgnoreRule r = new FastIgnoreRule(pattern); // If speed of this test is ever an issue, we can use a presetRule field // to avoid recompiling a pattern each time. diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java index 699542c57..529c75ff3 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java @@ -47,31 +47,14 @@ import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; -import java.util.Arrays; - import org.eclipse.jgit.junit.Assert; import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; /** * Tests ignore pattern matches */ -@SuppressWarnings("deprecation") -@RunWith(Parameterized.class) public class IgnoreMatcherParametrizedTest { - @Parameters(name = "OldRule? {0}") - public static Iterable data() { - return Arrays.asList(new Boolean[][] { { Boolean.FALSE }, - { Boolean.TRUE } }); - } - - @Parameter - public Boolean useOldRule; - @Test public void testBasic() { String pattern = "/test.stp"; @@ -252,47 +235,38 @@ public void testDirModeAndNoRegex() { @Test public void testDirModeAndRegex1() { - // IgnoreRule was buggy for some cases below, therefore using "Assume" - Boolean assume = useOldRule; - String pattern = "a/*/src/"; assertMatched(pattern, "a/b/src/"); assertMatched(pattern, "a/b/src/new"); assertMatched(pattern, "a/b/src/new/a.c"); assertMatched(pattern, "a/b/src/a.c"); // no match as a "file" pattern, because rule is for directories only - assertNotMatched(pattern, "a/b/src", assume); + assertNotMatched(pattern, "a/b/src"); assertNotMatched(pattern, "a/b/srcA/"); } @Test public void testDirModeAndRegex2() { - // IgnoreRule was buggy for some cases below, therefore using "Assume" - Boolean assume = useOldRule; - String pattern = "a/[a-b]/src/"; assertMatched(pattern, "a/b/src/"); assertMatched(pattern, "a/b/src/new"); assertMatched(pattern, "a/b/src/new/a.c"); assertMatched(pattern, "a/b/src/a.c"); // no match as a "file" pattern, because rule is for directories only - assertNotMatched(pattern, "a/b/src", assume); + assertNotMatched(pattern, "a/b/src"); assertNotMatched(pattern, "a/b/srcA/"); } @Test public void testDirModeAndRegex3() { - // IgnoreRule was buggy for some cases below, therefore using "Assume" - Boolean assume = useOldRule; - String pattern = "**/src/"; - assertMatched(pattern, "a/b/src/", assume); - assertMatched(pattern, "a/b/src/new", assume); - assertMatched(pattern, "a/b/src/new/a.c", assume); - assertMatched(pattern, "a/b/src/a.c", assume); + assertMatched(pattern, "a/b/src/"); + assertMatched(pattern, "a/b/src/new"); + assertMatched(pattern, "a/b/src/new/a.c"); + assertMatched(pattern, "a/b/src/a.c"); // no match as a "file" pattern, because rule is for directories only - assertNotMatched(pattern, "a/b/src", assume); - assertNotMatched(pattern, "a/b/srcA/", assume); + assertNotMatched(pattern, "a/b/src"); + assertNotMatched(pattern, "a/b/srcA/"); } @Test @@ -416,13 +390,8 @@ public void assertNotMatched(String pattern, String target, private boolean match(String pattern, String target) { boolean isDirectory = target.endsWith("/"); boolean match; - if (useOldRule.booleanValue()) { - IgnoreRule r = new IgnoreRule(pattern); - match = r.isMatch(target, isDirectory); - } else { - FastIgnoreRule r = new FastIgnoreRule(pattern); - match = r.isMatch(target, isDirectory); - } + FastIgnoreRule r = new FastIgnoreRule(pattern); + match = r.isMatch(target, isDirectory); if (isDirectory) { boolean noTrailingSlash = matchAsDir(pattern, @@ -447,10 +416,6 @@ private boolean match(String pattern, String target) { */ private boolean matchAsDir(String pattern, String target) { assertFalse(target.endsWith("/")); - if (useOldRule.booleanValue()) { - IgnoreRule r = new IgnoreRule(pattern); - return r.isMatch(target, true); - } FastIgnoreRule r = new FastIgnoreRule(pattern); return r.isMatch(target, true); } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherTest.java deleted file mode 100644 index 0713b1ad1..000000000 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherTest.java +++ /dev/null @@ -1,400 +0,0 @@ -/* - * Copyright (C) 2010, Red Hat Inc. - * and other copyright owners as documented in the project's IP log. - * - * This program and the accompanying materials are made available - * under the terms of the Eclipse Distribution License v1.0 which - * accompanies this distribution, is reproduced below, and is - * available at http://www.eclipse.org/org/documents/edl-v10.php - * - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or - * without modification, are permitted provided that the following - * conditions are met: - * - * - Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * - * - Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following - * disclaimer in the documentation and/or other materials provided - * with the distribution. - * - * - Neither the name of the Eclipse Foundation, Inc. nor the - * names of its contributors may be used to endorse or promote - * products derived from this software without specific prior - * written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND - * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, - * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES - * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER - * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, - * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF - * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package org.eclipse.jgit.ignore; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import org.junit.Test; - - -/** - * Tests ignore pattern matches - */ -@SuppressWarnings("deprecation") -public class IgnoreMatcherTest { - - @Test - public void testBasic() { - String pattern = "/test.stp"; - assertMatched(pattern, "/test.stp"); - - pattern = "#/test.stp"; - assertNotMatched(pattern, "/test.stp"); - } - - @Test - public void testFileNameWildcards() { - //Test basic * and ? for any pattern + any character - String pattern = "*.st?"; - assertMatched(pattern, "/test.stp"); - assertMatched(pattern, "/anothertest.stg"); - assertMatched(pattern, "/anothertest.st0"); - assertNotMatched(pattern, "/anothertest.sta1"); - //Check that asterisk does not expand to "/" - assertNotMatched(pattern, "/another/test.sta1"); - - //Same as above, with a leading slash to ensure that doesn't cause problems - pattern = "/*.st?"; - assertMatched(pattern, "/test.stp"); - assertMatched(pattern, "/anothertest.stg"); - assertMatched(pattern, "/anothertest.st0"); - assertNotMatched(pattern, "/anothertest.sta1"); - //Check that asterisk does not expand to "/" - assertNotMatched(pattern, "/another/test.sta1"); - - //Test for numbers - pattern = "*.sta[0-5]"; - assertMatched(pattern, "/test.sta5"); - assertMatched(pattern, "/test.sta4"); - assertMatched(pattern, "/test.sta3"); - assertMatched(pattern, "/test.sta2"); - assertMatched(pattern, "/test.sta1"); - assertMatched(pattern, "/test.sta0"); - assertMatched(pattern, "/anothertest.sta2"); - assertNotMatched(pattern, "test.stag"); - assertNotMatched(pattern, "test.sta6"); - - //Test for letters - pattern = "/[tv]est.sta[a-d]"; - assertMatched(pattern, "/test.staa"); - assertMatched(pattern, "/test.stab"); - assertMatched(pattern, "/test.stac"); - assertMatched(pattern, "/test.stad"); - assertMatched(pattern, "/vest.stac"); - assertNotMatched(pattern, "test.stae"); - assertNotMatched(pattern, "test.sta9"); - - //Test child directory/file is matched - pattern = "/src/ne?"; - assertMatched(pattern, "/src/new/"); - assertMatched(pattern, "/src/new"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/src/new/a/a.c"); - assertNotMatched(pattern, "/src/new.c"); - - //Test name-only fnmatcher matches - pattern = "ne?"; - assertMatched(pattern, "/src/new/"); - assertMatched(pattern, "/src/new"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/src/new/a/a.c"); - assertMatched(pattern, "/neb"); - assertNotMatched(pattern, "/src/new.c"); - } - - @Test - public void testTargetWithoutLeadingSlash() { - //Test basic * and ? for any pattern + any character - String pattern = "/*.st?"; - assertMatched(pattern, "test.stp"); - assertMatched(pattern, "anothertest.stg"); - assertMatched(pattern, "anothertest.st0"); - assertNotMatched(pattern, "anothertest.sta1"); - //Check that asterisk does not expand to "" - assertNotMatched(pattern, "another/test.sta1"); - - //Same as above, with a leading slash to ensure that doesn't cause problems - pattern = "/*.st?"; - assertMatched(pattern, "test.stp"); - assertMatched(pattern, "anothertest.stg"); - assertMatched(pattern, "anothertest.st0"); - assertNotMatched(pattern, "anothertest.sta1"); - //Check that asterisk does not expand to "" - assertNotMatched(pattern, "another/test.sta1"); - - //Test for numbers - pattern = "/*.sta[0-5]"; - assertMatched(pattern, "test.sta5"); - assertMatched(pattern, "test.sta4"); - assertMatched(pattern, "test.sta3"); - assertMatched(pattern, "test.sta2"); - assertMatched(pattern, "test.sta1"); - assertMatched(pattern, "test.sta0"); - assertMatched(pattern, "anothertest.sta2"); - assertNotMatched(pattern, "test.stag"); - assertNotMatched(pattern, "test.sta6"); - - //Test for letters - pattern = "/[tv]est.sta[a-d]"; - assertMatched(pattern, "test.staa"); - assertMatched(pattern, "test.stab"); - assertMatched(pattern, "test.stac"); - assertMatched(pattern, "test.stad"); - assertMatched(pattern, "vest.stac"); - assertNotMatched(pattern, "test.stae"); - assertNotMatched(pattern, "test.sta9"); - - //Test child directory/file is matched - pattern = "/src/ne?"; - assertMatched(pattern, "src/new/"); - assertMatched(pattern, "src/new"); - assertMatched(pattern, "src/new/a.c"); - assertMatched(pattern, "src/new/a/a.c"); - assertNotMatched(pattern, "src/new.c"); - - //Test name-only fnmatcher matches - pattern = "ne?"; - assertMatched(pattern, "src/new/"); - assertMatched(pattern, "src/new"); - assertMatched(pattern, "src/new/a.c"); - assertMatched(pattern, "src/new/a/a.c"); - assertMatched(pattern, "neb"); - assertNotMatched(pattern, "src/new.c"); - } - - @Test - public void testParentDirectoryGitIgnores() { - //Contains git ignore patterns such as might be seen in a parent directory - - //Test for wildcards - String pattern = "/*/*.c"; - assertMatched(pattern, "/file/a.c"); - assertMatched(pattern, "/src/a.c"); - assertNotMatched(pattern, "/src/new/a.c"); - - //Test child directory/file is matched - pattern = "/src/new"; - assertMatched(pattern, "/src/new/"); - assertMatched(pattern, "/src/new"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/src/new/a/a.c"); - assertNotMatched(pattern, "/src/new.c"); - - //Test child directory is matched, slash after name - pattern = "/src/new/"; - assertMatched(pattern, "/src/new/"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/src/new/a/a.c"); - assertNotMatched(pattern, "/src/new"); - assertNotMatched(pattern, "/src/new.c"); - - //Test directory is matched by name only - pattern = "b1"; - assertMatched(pattern, "/src/new/a/b1/a.c"); - assertNotMatched(pattern, "/src/new/a/b2/file.c"); - assertNotMatched(pattern, "/src/new/a/bb1/file.c"); - assertNotMatched(pattern, "/src/new/a/file.c"); - } - - @Test - public void testTrailingSlash() { - String pattern = "/src/"; - assertMatched(pattern, "/src/"); - assertMatched(pattern, "/src/new"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/src/a.c"); - assertNotMatched(pattern, "/src"); - assertNotMatched(pattern, "/srcA/"); - } - - @Test - public void testNameOnlyMatches() { - /* - * Name-only matches do not contain any path separators - */ - //Test matches for file extension - String pattern = "*.stp"; - assertMatched(pattern, "/test.stp"); - assertMatched(pattern, "/src/test.stp"); - assertNotMatched(pattern, "/test.stp1"); - assertNotMatched(pattern, "/test.astp"); - - //Test matches for name-only, applies to file name or folder name - pattern = "src"; - assertMatched(pattern, "/src"); - assertMatched(pattern, "/src/"); - assertMatched(pattern, "/src/a.c"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/new/src/a.c"); - assertMatched(pattern, "/file/src"); - - //Test matches for name-only, applies only to folder names - pattern = "src/"; - assertMatched(pattern, "/src/"); - assertMatched(pattern, "/src/a.c"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/new/src/a.c"); - assertNotMatched(pattern, "/src"); - assertNotMatched(pattern, "/file/src"); - - //Test matches for name-only, applies to file name or folder name - //With a small wildcard - pattern = "?rc"; - assertMatched(pattern, "/src/a.c"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/new/src/a.c"); - assertMatched(pattern, "/file/src"); - assertMatched(pattern, "/src/"); - - //Test matches for name-only, applies to file name or folder name - //With a small wildcard - pattern = "?r[a-c]"; - assertMatched(pattern, "/src/a.c"); - assertMatched(pattern, "/src/new/a.c"); - assertMatched(pattern, "/new/src/a.c"); - assertMatched(pattern, "/file/src"); - assertMatched(pattern, "/src/"); - assertMatched(pattern, "/srb/a.c"); - assertMatched(pattern, "/grb/new/a.c"); - assertMatched(pattern, "/new/crb/a.c"); - assertMatched(pattern, "/file/3rb"); - assertMatched(pattern, "/xrb/"); - assertMatched(pattern, "/3ra/a.c"); - assertMatched(pattern, "/5ra/new/a.c"); - assertMatched(pattern, "/new/1ra/a.c"); - assertMatched(pattern, "/file/dra"); - assertMatched(pattern, "/era/"); - assertNotMatched(pattern, "/crg"); - assertNotMatched(pattern, "/cr3"); - } - - @Test - public void testNegation() { - String pattern = "!/test.stp"; - assertMatched(pattern, "/test.stp"); - } - - @Test - public void testGetters() { - IgnoreRule r = new IgnoreRule("/pattern/"); - assertFalse(r.getNameOnly()); - assertTrue(r.dirOnly()); - assertFalse(r.getNegation()); - assertEquals(r.getPattern(), "/pattern"); - - r = new IgnoreRule("/patter?/"); - assertFalse(r.getNameOnly()); - assertTrue(r.dirOnly()); - assertFalse(r.getNegation()); - assertEquals(r.getPattern(), "/patter?"); - - r = new IgnoreRule("patt*"); - assertTrue(r.getNameOnly()); - assertFalse(r.dirOnly()); - assertFalse(r.getNegation()); - assertEquals(r.getPattern(), "patt*"); - - r = new IgnoreRule("pattern"); - assertTrue(r.getNameOnly()); - assertFalse(r.dirOnly()); - assertFalse(r.getNegation()); - assertEquals(r.getPattern(), "pattern"); - - r = new IgnoreRule("!pattern"); - assertTrue(r.getNameOnly()); - assertFalse(r.dirOnly()); - assertTrue(r.getNegation()); - assertEquals(r.getPattern(), "pattern"); - - r = new IgnoreRule("!/pattern"); - assertFalse(r.getNameOnly()); - assertFalse(r.dirOnly()); - assertTrue(r.getNegation()); - assertEquals(r.getPattern(), "/pattern"); - - r = new IgnoreRule("!/patter?"); - assertFalse(r.getNameOnly()); - assertFalse(r.dirOnly()); - assertTrue(r.getNegation()); - assertEquals(r.getPattern(), "/patter?"); - } - - @Test - public void testResetState() { - String pattern = "/build/*"; - String target = "/build"; - // Don't use the assert methods of this class, as we want to test - // whether the state in IgnoreRule is reset properly - IgnoreRule r = new IgnoreRule(pattern); - // Result should be the same for the same inputs - assertFalse(r.isMatch(target, true)); - assertFalse(r.isMatch(target, true)); - } - - /** - * Check for a match. If target ends with "/", match will assume that the - * target is meant to be a directory. - * @param pattern - * Pattern as it would appear in a .gitignore file - * @param target - * Target file path relative to repository's GIT_DIR - */ - public void assertMatched(String pattern, String target) { - boolean value = match(pattern, target); - assertTrue("Expected a match for: " + pattern + " with: " + target, - value); - } - - /** - * Check for a match. If target ends with "/", match will assume that the - * target is meant to be a directory. - * @param pattern - * Pattern as it would appear in a .gitignore file - * @param target - * Target file path relative to repository's GIT_DIR - */ - public void assertNotMatched(String pattern, String target) { - boolean value = match(pattern, target); - assertFalse("Expected no match for: " + pattern + " with: " + target, - value); - } - - /** - * Check for a match. If target ends with "/", match will assume that the - * target is meant to be a directory. - * - * @param pattern - * Pattern as it would appear in a .gitignore file - * @param target - * Target file path relative to repository's GIT_DIR - * @return Result of IgnoreRule.isMatch(String, boolean) - */ - private static boolean match(String pattern, String target) { - IgnoreRule r = new IgnoreRule(pattern); - //If speed of this test is ever an issue, we can use a presetRule field - //to avoid recompiling a pattern each time. - return r.isMatch(target, target.endsWith("/")); - } -} diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreRuleSpecialCasesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreRuleSpecialCasesTest.java index 7afa69f44..a7a78f841 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreRuleSpecialCasesTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreRuleSpecialCasesTest.java @@ -46,64 +46,32 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assume.assumeTrue; -import java.util.Arrays; - import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; -@RunWith(Parameterized.class) -@SuppressWarnings({ "deprecation", "boxing" }) +@SuppressWarnings({ "boxing" }) public class IgnoreRuleSpecialCasesTest { - @Parameters(name = "OldRule? {0}") - public static Iterable data() { - return Arrays.asList(new Boolean[][] { { Boolean.FALSE }, - { Boolean.TRUE } }); - } - - @Parameter - public Boolean useOldRule; - private void assertMatch(final String pattern, final String input, final boolean matchExpected, Boolean... assume) { boolean assumeDir = input.endsWith("/"); - if (useOldRule.booleanValue()) { - final IgnoreRule matcher = new IgnoreRule(pattern); - if (assume.length == 0 || !assume[0].booleanValue()) - assertEquals(matchExpected, matcher.isMatch(input, assumeDir)); - else - assumeTrue(matchExpected == matcher.isMatch(input, assumeDir)); + FastIgnoreRule matcher = new FastIgnoreRule(pattern); + if (assume.length == 0 || !assume[0].booleanValue()) { + assertEquals(matchExpected, matcher.isMatch(input, assumeDir)); } else { - FastIgnoreRule matcher = new FastIgnoreRule(pattern); - if (assume.length == 0 || !assume[0].booleanValue()) - assertEquals(matchExpected, matcher.isMatch(input, assumeDir)); - else - assumeTrue(matchExpected == matcher.isMatch(input, assumeDir)); + assumeTrue(matchExpected == matcher.isMatch(input, assumeDir)); } } private void assertFileNameMatch(final String pattern, final String input, final boolean matchExpected) { boolean assumeDir = input.endsWith("/"); - if (useOldRule.booleanValue()) { - final IgnoreRule matcher = new IgnoreRule(pattern); - assertEquals(matchExpected, matcher.isMatch(input, assumeDir)); - } else { - FastIgnoreRule matcher = new FastIgnoreRule(pattern); - assertEquals(matchExpected, matcher.isMatch(input, assumeDir)); - } + FastIgnoreRule matcher = new FastIgnoreRule(pattern); + assertEquals(matchExpected, matcher.isMatch(input, assumeDir)); } @Test public void testVerySimplePatternCase0() throws Exception { - if (useOldRule) - System.err - .println("IgnoreRule can't understand blank lines, skipping"); - Boolean assume = useOldRule; - assertMatch("", "", false, assume); + assertMatch("", "", false); } @Test @@ -805,12 +773,9 @@ public void testSpecialGroupCase9() throws Exception { @Test public void testSpecialGroupCase10() throws Exception { - if (useOldRule) - System.err.println("IgnoreRule can't understand [[:], skipping"); - Boolean assume = useOldRule; // Second bracket is threated literally, so both [ and : should match - assertMatch("[[:]", ":", true, assume); - assertMatch("[[:]", "[", true, assume); + assertMatch("[[:]", ":", true); + assertMatch("[[:]", "[", true); } @Test @@ -866,12 +831,8 @@ public void testEscapedBracket6() throws Exception { @Test public void testEscapedBackslash() throws Exception { - if (useOldRule) - System.err - .println("IgnoreRule can't understand escaped backslashes, skipping"); - Boolean assume = useOldRule; // In Git CLI a\\b matches a\b file - assertMatch("a\\\\b", "a\\b", true, assume); + assertMatch("a\\\\b", "a\\b", true); } @Test diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/IgnoreRule.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/IgnoreRule.java deleted file mode 100644 index f14d1bd0b..000000000 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/IgnoreRule.java +++ /dev/null @@ -1,278 +0,0 @@ -/* - * Copyright (C) 2010, Red Hat Inc. - * and other copyright owners as documented in the project's IP log. - * - * This program and the accompanying materials are made available - * under the terms of the Eclipse Distribution License v1.0 which - * accompanies this distribution, is reproduced below, and is - * available at http://www.eclipse.org/org/documents/edl-v10.php - * - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or - * without modification, are permitted provided that the following - * conditions are met: - * - * - Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * - * - Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following - * disclaimer in the documentation and/or other materials provided - * with the distribution. - * - * - Neither the name of the Eclipse Foundation, Inc. nor the - * names of its contributors may be used to endorse or promote - * products derived from this software without specific prior - * written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND - * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, - * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES - * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER - * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, - * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF - * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package org.eclipse.jgit.ignore; - -import org.eclipse.jgit.errors.InvalidPatternException; -import org.eclipse.jgit.fnmatch.FileNameMatcher; - -/** - * A single ignore rule corresponding to one line in a .gitignore or ignore - * file. Parses the ignore pattern - * - * Inspiration from: Ferry Huberts - * - * @deprecated this rule does not support double star pattern and is slow - * parsing glob expressions. Consider to use {@link FastIgnoreRule} - * instead. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=440732 - */ -@Deprecated -public class IgnoreRule { - private String pattern; - private boolean negation; - private boolean nameOnly; - private boolean dirOnly; - private FileNameMatcher matcher; - - /** - * Create a new ignore rule with the given pattern. Assumes that - * the pattern is already trimmed. - * - * @param pattern - * Base pattern for the ignore rule. This pattern will - * be parsed to generate rule parameters. - */ - public IgnoreRule (String pattern) { - this.pattern = pattern; - negation = false; - nameOnly = false; - dirOnly = false; - matcher = null; - setup(); - } - - /** - * Remove leading/trailing characters as needed. Set up - * rule variables for later matching. - */ - private void setup() { - int startIndex = 0; - int endIndex = pattern.length(); - if (pattern.startsWith("!")) { //$NON-NLS-1$ - startIndex++; - negation = true; - } - - if (pattern.endsWith("/")) { //$NON-NLS-1$ - endIndex --; - dirOnly = true; - } - - pattern = pattern.substring(startIndex, endIndex); - boolean hasSlash = pattern.contains("/"); //$NON-NLS-1$ - - if (!hasSlash) - nameOnly = true; - else if (!pattern.startsWith("/")) { //$NON-NLS-1$ - //Contains "/" but does not start with one - //Adding / to the start should not interfere with matching - pattern = "/" + pattern; //$NON-NLS-1$ - } - - if (pattern.contains("*") || pattern.contains("?") || pattern.contains("[")) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ - try { - matcher = new FileNameMatcher(pattern, Character.valueOf('/')); - } catch (InvalidPatternException e) { - // Ignore pattern exceptions - } - } - } - - - /** - * @return - * True if the pattern is just a file name and not a path - */ - public boolean getNameOnly() { - return nameOnly; - } - - /** - * - * @return - * True if the pattern should match directories only - */ - public boolean dirOnly() { - return dirOnly; - } - - /** - * - * @return - * True if the pattern had a "!" in front of it - */ - public boolean getNegation() { - return negation; - } - - /** - * @return - * The blob pattern to be used as a matcher - */ - public String getPattern() { - return pattern; - } - - /** - * Returns true if a match was made. - *
- * This function does NOT return the actual ignore status of the - * target! Please consult {@link #getResult()} for the ignore status. The actual - * ignore status may be true or false depending on whether this rule is - * an ignore rule or a negation rule. - * - * @param target - * Name pattern of the file, relative to the base directory of this rule - * @param isDirectory - * Whether the target file is a directory or not - * @return - * True if a match was made. This does not necessarily mean that - * the target is ignored. Call {@link IgnoreRule#getResult() getResult()} for the result. - */ - public boolean isMatch(String target, boolean isDirectory) { - if (!target.startsWith("/")) //$NON-NLS-1$ - target = "/" + target; //$NON-NLS-1$ - - if (matcher == null) { - if (target.equals(pattern)) { - //Exact match - if (dirOnly && !isDirectory) - //Directory expectations not met - return false; - else - //Directory expectations met - return true; - } - - /* - * Add slashes for startsWith check. This avoids matching e.g. - * "/src/new" to /src/newfile" but allows "/src/new" to match - * "/src/new/newfile", as is the git standard - */ - if ((target).startsWith(pattern + "/")) //$NON-NLS-1$ - return true; - - if (nameOnly) { - //Iterate through each sub-name - final String[] segments = target.split("/"); //$NON-NLS-1$ - for (int idx = 0; idx < segments.length; idx++) { - final String segmentName = segments[idx]; - // String.split("/") creates empty segment for leading slash - if (segmentName.length() == 0) - continue; - if (segmentName.equals(pattern) && - doesMatchDirectoryExpectations(isDirectory, idx, segments.length)) - return true; - } - } - - } else { - matcher.reset(); - matcher.append(target); - if (matcher.isMatch()) - return true; - - final String[] segments = target.split("/"); //$NON-NLS-1$ - if (nameOnly) { - for (int idx = 0; idx < segments.length; idx++) { - final String segmentName = segments[idx]; - // String.split("/") creates empty segment for leading slash - if (segmentName.length() == 0) - continue; - //Iterate through each sub-directory - matcher.reset(); - matcher.append(segmentName); - if (matcher.isMatch() && - doesMatchDirectoryExpectations(isDirectory, idx, segments.length)) - return true; - } - } else { - //TODO: This is the slowest operation - //This matches e.g. "/src/ne?" to "/src/new/file.c" - matcher.reset(); - - for (int idx = 0; idx < segments.length; idx++) { - final String segmentName = segments[idx]; - // String.split("/") creates empty segment for leading slash - if (segmentName.length() == 0) - continue; - - matcher.append("/" + segmentName); //$NON-NLS-1$ - - if (matcher.isMatch() - && doesMatchDirectoryExpectations(isDirectory, idx, - segments.length)) - return true; - } - } - } - - return false; - } - - /** - * If a call to isMatch(String, boolean) was previously - * made, this will return whether or not the target was ignored. Otherwise - * this just indicates whether the rule is non-negation or negation. - * - * @return - * True if the target is to be ignored, false otherwise. - */ - public boolean getResult() { - return !negation; - } - - private boolean doesMatchDirectoryExpectations(boolean isDirectory, int segmentIdx, int segmentLength) { - // The segment we are checking is a directory, expectations are met. - if (segmentIdx < segmentLength - 1) { - return true; - } - - // We are checking the last part of the segment for which isDirectory has to be considered. - return !dirOnly || isDirectory; - } - - @Override - public String toString() { - return pattern; - } -} From 10412ddfedfe80537ffa497c7960443d703ee129 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 25 Apr 2015 00:51:02 +0200 Subject: [PATCH 04/26] Delete deprecated PackWriter.preparePack() methods Change-Id: I62befa4a933c9ffd42d14519f555554cc513ddd9 Signed-off-by: Matthias Sohn --- .../internal/storage/pack/PackWriter.java | 94 ++----------------- 1 file changed, 7 insertions(+), 87 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 6d0c8e6cd..1422c5ef0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -136,8 +136,8 @@ * Typical usage consists of creating instance intended for some pack, * configuring options, preparing the list of objects by calling * {@link #preparePack(Iterator)} or - * {@link #preparePack(ProgressMonitor, Collection, Collection)}, and finally - * producing the stream with + * {@link #preparePack(ProgressMonitor, Set, Set)}, and finally producing the + * stream with * {@link #writePack(ProgressMonitor, ProgressMonitor, OutputStream)}. *

*

@@ -293,7 +293,7 @@ public static Iterable getInstances() { * Create writer for specified repository. *

* Objects for packing are specified in {@link #preparePack(Iterator)} or - * {@link #preparePack(ProgressMonitor, Collection, Collection)}. + * {@link #preparePack(ProgressMonitor, Set, Set)}. * * @param repo * repository where objects are stored. @@ -306,7 +306,7 @@ public PackWriter(final Repository repo) { * Create a writer to load objects from the specified reader. *

* Objects for packing are specified in {@link #preparePack(Iterator)} or - * {@link #preparePack(ProgressMonitor, Collection, Collection)}. + * {@link #preparePack(ProgressMonitor, Set, Set)}. * * @param reader * reader to read from the repository with. @@ -319,7 +319,7 @@ public PackWriter(final ObjectReader reader) { * Create writer for specified repository. *

* Objects for packing are specified in {@link #preparePack(Iterator)} or - * {@link #preparePack(ProgressMonitor, Collection, Collection)}. + * {@link #preparePack(ProgressMonitor, Set, Set)}. * * @param repo * repository where objects are stored. @@ -334,7 +334,7 @@ public PackWriter(final Repository repo, final ObjectReader reader) { * Create writer with a specified configuration. *

* Objects for packing are specified in {@link #preparePack(Iterator)} or - * {@link #preparePack(ProgressMonitor, Collection, Collection)}. + * {@link #preparePack(ProgressMonitor, Set, Set)}. * * @param config * configuration for the pack writer. @@ -495,7 +495,7 @@ public void setIndexDisabled(boolean noIndex) { /** * @return true to ignore objects that are uninteresting and also not found * on local disk; false to throw a {@link MissingObjectException} - * out of {@link #preparePack(ProgressMonitor, Collection, Collection)} if an + * out of {@link #preparePack(ProgressMonitor, Set, Set)} if an * uninteresting object is not in the source repository. By default, * true, permitting gracefully ignoring of uninteresting objects. */ @@ -648,86 +648,6 @@ public void preparePack(final Iterator objectsSource) } } - /** - * Prepare the list of objects to be written to the pack stream. - *

- * Basing on these 2 sets, another set of objects to put in a pack file is - * created: this set consists of all objects reachable (ancestors) from - * interesting objects, except uninteresting objects and their ancestors. - * This method uses class {@link ObjectWalk} extensively to find out that - * appropriate set of output objects and their optimal order in output pack. - * Order is consistent with general git in-pack rules: sort by object type, - * recency, path and delta-base first. - *

- * - * @param countingMonitor - * progress during object enumeration. - * @param want - * collection of objects to be marked as interesting (start - * points of graph traversal). - * @param have - * collection of objects to be marked as uninteresting (end - * points of graph traversal). - * @throws IOException - * when some I/O problem occur during reading objects. - * @deprecated to be removed in 2.0; use the Set version of this method. - */ - @Deprecated - public void preparePack(ProgressMonitor countingMonitor, - final Collection want, - final Collection have) throws IOException { - preparePack(countingMonitor, ensureSet(want), ensureSet(have)); - } - - /** - * Prepare the list of objects to be written to the pack stream. - *

- * Basing on these 2 sets, another set of objects to put in a pack file is - * created: this set consists of all objects reachable (ancestors) from - * interesting objects, except uninteresting objects and their ancestors. - * This method uses class {@link ObjectWalk} extensively to find out that - * appropriate set of output objects and their optimal order in output pack. - * Order is consistent with general git in-pack rules: sort by object type, - * recency, path and delta-base first. - *

- * - * @param countingMonitor - * progress during object enumeration. - * @param walk - * ObjectWalk to perform enumeration. - * @param interestingObjects - * collection of objects to be marked as interesting (start - * points of graph traversal). - * @param uninterestingObjects - * collection of objects to be marked as uninteresting (end - * points of graph traversal). - * @throws IOException - * when some I/O problem occur during reading objects. - * @deprecated to be removed in 2.0; use the Set version of this method. - */ - @Deprecated - public void preparePack(ProgressMonitor countingMonitor, - ObjectWalk walk, - final Collection interestingObjects, - final Collection uninterestingObjects) - throws IOException { - preparePack(countingMonitor, walk, - ensureSet(interestingObjects), - ensureSet(uninterestingObjects)); - } - - @SuppressWarnings("unchecked") - private static Set ensureSet(Collection objs) { - Set set; - if (objs instanceof Set) - set = (Set) objs; - else if (objs == null) - set = Collections.emptySet(); - else - set = new HashSet(objs); - return set; - } - /** * Prepare the list of objects to be written to the pack stream. *

From 6f7130140410d9c8269b846e782f27df17783c35 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Mon, 4 May 2015 11:00:57 +0200 Subject: [PATCH 05/26] Fix possible AIOOB in DirCacheTree.contains() When DirCacheTree.contains() is called and 'aOff' is greater than 'aLen' an ArrayIndexOutOfBoundsException was thrown. This fix makes DirCacheTree.contains() more robust and allows parsing such index files without throwing AIOOB. I couldn't create a test case leading to this situation but I have seen such situations while inspecting Bug: 465393. It seems that such situations are created on Windows when there are invalid pathes in the index. There may be a not yet known bug leading to such situations in combination with invalid pathes. Bug: 465393 Change-Id: I6535d924a22cba9a05df0ccd7e6dc2c9ddc42375 --- .../src/org/eclipse/jgit/dircache/DirCacheTree.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java index 30932e827..83aa8fa4d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheTree.java @@ -399,7 +399,7 @@ final boolean contains(final byte[] a, int aOff, final int aLen) { for (int eOff = 0; eOff < eLen && aOff < aLen; eOff++, aOff++) if (e[eOff] != a[aOff]) return false; - if (aOff == aLen) + if (aOff >= aLen) return false; return a[aOff] == '/'; } From ae592cc65576d1acac2bb6a8ed12c9160b403f0a Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 6 May 2015 15:47:34 -0700 Subject: [PATCH 06/26] Add repository name to failures in HTTP server log If UploadPack or ReceivePack has an exception record an identifier associated with the repository as part of the log message. This can help the HTTP admin track down the offending repository and take action to repair the root cause. Change-Id: I58f22b33cdb40994f044a26fba9fe965b45be51d --- org.eclipse.jgit.http.server/META-INF/MANIFEST.MF | 1 + .../jgit/http/server/HttpServerText.properties | 4 ++-- .../jgit/http/server/ReceivePackServlet.java | 13 +++++++++---- .../eclipse/jgit/http/server/ServletUtils.java | 10 ++++++++++ .../jgit/http/server/UploadPackServlet.java | 15 ++++++++++----- org.eclipse.jgit/META-INF/MANIFEST.MF | 4 +++- 6 files changed, 35 insertions(+), 12 deletions(-) diff --git a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF index 94d62f34d..2c76d7454 100644 --- a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF @@ -18,6 +18,7 @@ Bundle-RequiredExecutionEnvironment: JavaSE-1.7 Import-Package: javax.servlet;version="[2.5.0,3.0.0)", javax.servlet.http;version="[2.5.0,3.0.0)", org.eclipse.jgit.errors;version="[4.0.0,4.1.0)", + org.eclipse.jgit.internal.storage.dfs;version="[4.0.0,4.1.0)", org.eclipse.jgit.internal.storage.file;version="[4.0.0,4.1.0)", org.eclipse.jgit.lib;version="[4.0.0,4.1.0)", org.eclipse.jgit.nls;version="[4.0.0,4.1.0)", diff --git a/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties b/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties index dbc5bf7b8..83c39277c 100644 --- a/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties +++ b/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties @@ -11,8 +11,8 @@ encodingNotSupportedByThisLibrary={0} "{1}": not supported by this library. expectedRepositoryAttribute=Expected Repository attribute filterMustNotBeNull=filter must not be null internalServerError=Internal server error -internalErrorDuringReceivePack=Internal error during receive-pack -internalErrorDuringUploadPack=Internal error during upload-pack +internalErrorDuringReceivePack=Internal error during receive-pack to {0} +internalErrorDuringUploadPack=Internal error during upload-pack from {0} internalServerErrorRequestAttributeWasAlreadySet=Internal server error, request attribute {0} was already set when {1} was invoked. invalidBoolean=Invalid boolean {0} = {1} invalidIndex=Invalid index: {0} diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java index a8e312d3f..9756feb0b 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java @@ -62,6 +62,7 @@ import static org.eclipse.jgit.util.HttpSupport.HDR_USER_AGENT; import java.io.IOException; +import java.text.MessageFormat; import java.util.List; import javax.servlet.Filter; @@ -192,14 +193,12 @@ public void flush() throws IOException { out.close(); } catch (UnpackException e) { // This should be already reported to the client. - getServletContext().log( - HttpServerText.get().internalErrorDuringReceivePack, - e.getCause()); + log(rp.getRepository(), e.getCause()); consumeRequestBody(req); out.close(); } catch (Throwable e) { - getServletContext().log(HttpServerText.get().internalErrorDuringReceivePack, e); + log(rp.getRepository(), e); if (!rsp.isCommitted()) { rsp.reset(); sendError(req, rsp, SC_INTERNAL_SERVER_ERROR); @@ -207,4 +206,10 @@ public void flush() throws IOException { return; } } + + private void log(Repository git, Throwable e) { + getServletContext().log(MessageFormat.format( + HttpServerText.get().internalErrorDuringReceivePack, + ServletUtils.identify(git)), e); + } } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java index 8d56d84c9..042ccf3dd 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java @@ -62,6 +62,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.internal.storage.dfs.DfsRepository; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; @@ -273,6 +274,15 @@ private static String etag(final byte[] content) { return ObjectId.fromRaw(md.digest()).getName(); } + static String identify(Repository git) { + if (git instanceof DfsRepository) { + return ((DfsRepository) git).getDescription().getRepositoryName(); + } else if (git.getDirectory() != null) { + return git.getDirectory().getPath(); + } + return "unknown"; + } + private ServletUtils() { // static utility class only } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java index 7aefcbd80..443eebb4d 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java @@ -61,6 +61,7 @@ import static org.eclipse.jgit.util.HttpSupport.HDR_USER_AGENT; import java.io.IOException; +import java.text.MessageFormat; import java.util.List; import javax.servlet.Filter; @@ -76,9 +77,9 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.InternalHttpServerGlue; import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser; +import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.eclipse.jgit.transport.UploadPack; import org.eclipse.jgit.transport.UploadPackInternalServerErrorException; -import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.transport.resolver.UploadPackFactory; @@ -203,14 +204,12 @@ public void flush() throws IOException { } catch (UploadPackInternalServerErrorException e) { // Special case exception, error message was sent to client. - getServletContext().log( - HttpServerText.get().internalErrorDuringUploadPack, - e.getCause()); + log(up.getRepository(), e.getCause()); consumeRequestBody(req); out.close(); } catch (Throwable e) { - getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e); + log(up.getRepository(), e); if (!rsp.isCommitted()) { rsp.reset(); sendError(req, rsp, SC_INTERNAL_SERVER_ERROR); @@ -218,4 +217,10 @@ public void flush() throws IOException { return; } } + + private void log(Repository git, Throwable e) { + getServletContext().log(MessageFormat.format( + HttpServerText.get().internalErrorDuringUploadPack, + ServletUtils.identify(git)), e); + } } diff --git a/org.eclipse.jgit/META-INF/MANIFEST.MF b/org.eclipse.jgit/META-INF/MANIFEST.MF index 42c48a17b..4e0e34152 100644 --- a/org.eclipse.jgit/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit/META-INF/MANIFEST.MF @@ -57,7 +57,9 @@ Export-Package: org.eclipse.jgit.api;version="4.0.0"; org.eclipse.jgit.ignore;version="4.0.0", org.eclipse.jgit.ignore.internal;version="4.0.0";x-friends:="org.eclipse.jgit.test", org.eclipse.jgit.internal;version="4.0.0";x-friends:="org.eclipse.jgit.test,org.eclipse.jgit.http.test", - org.eclipse.jgit.internal.storage.dfs;version="4.0.0";x-friends:="org.eclipse.jgit.test", + org.eclipse.jgit.internal.storage.dfs;version="4.0.0"; + x-friends:="org.eclipse.jgit.test, + org.eclipse.jgit.http.server", org.eclipse.jgit.internal.storage.file;version="4.0.0"; x-friends:="org.eclipse.jgit.test, org.eclipse.jgit.junit, From a870a8a03cf66024eae9a9425e5f699c5df8522e Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 6 May 2015 16:02:27 -0700 Subject: [PATCH 07/26] Skip logging stack trace on corrupt objects Instead of dumping a full stack trace when a client sends an invalid commit, record only a short line explaining the attempt: Cannot receive Invalid commit c0ff33...: invalid author into /tmp/jgit.git The text alone is sufficient to explain the problem and the stack trace does not lend any additional useful information. ObjectChecker is quite clear about its rejection cases. Change-Id: Ifc8cf06032489dc6431be1ba66101cf3d4299218 --- .../eclipse/jgit/http/server/HttpServerText.properties | 1 + .../org/eclipse/jgit/http/server/HttpServerText.java | 1 + .../eclipse/jgit/http/server/ReceivePackServlet.java | 10 ++++++++++ 3 files changed, 12 insertions(+) diff --git a/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties b/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties index 83c39277c..28432b0d7 100644 --- a/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties +++ b/org.eclipse.jgit.http.server/resources/org/eclipse/jgit/http/server/HttpServerText.properties @@ -21,6 +21,7 @@ noResolverAvailable=No resolver available parameterNotSet=Parameter {0} not set pathForParamNotFound={0} (for {1}) not found pathNotSupported={0} not supported +receivedCorruptObject=Cannot receive {0} into {1} repositoryAccessForbidden=Git access forbidden repositoryNotFound=Git repository not found servletAlreadyInitialized=Servlet already initialized diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java index a8dd1b10c..dff90a6c8 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/HttpServerText.java @@ -76,6 +76,7 @@ public static HttpServerText get() { /***/ public String parameterNotSet; /***/ public String pathForParamNotFound; /***/ public String pathNotSupported; + /***/ public String receivedCorruptObject; /***/ public String repositoryAccessForbidden; /***/ public String repositoryNotFound; /***/ public String servletAlreadyInitialized; diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java index 9756feb0b..cd066b0c7 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java @@ -75,6 +75,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.UnpackException; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.InternalHttpServerGlue; @@ -191,6 +192,15 @@ public void flush() throws IOException { rp.receive(getInputStream(req), out, null); out.close(); + } catch (CorruptObjectException e ) { + // This should be already reported to the client. + getServletContext().log(MessageFormat.format( + HttpServerText.get().receivedCorruptObject, + e.getMessage(), + ServletUtils.identify(rp.getRepository()))); + consumeRequestBody(req); + out.close(); + } catch (UnpackException e) { // This should be already reported to the client. log(rp.getRepository(), e.getCause()); From ec6ec3b10fb1ef8dd73a499d0b1f7a7d711b84dd Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Mon, 4 May 2015 11:45:22 +0200 Subject: [PATCH 08/26] FS_Win32: Avoid an IOException on Windows if bash is not in PATH Change-Id: I3145f74ecee9f5b368e7f4b9fd7cb906f407eff5 Signed-off-by: Sebastian Schuberth Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/util/FS_Win32.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java index 6f553ccfa..41e8113a4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java @@ -111,18 +111,20 @@ protected File discoverGitPrefix() { if (gitExe != null) return resolveGrandparentFile(gitExe); - // This isn't likely to work, if bash is in $PATH, git should - // also be in $PATH. But its worth trying. - // - String w = readPipe(userHome(), // - new String[] { "bash", "--login", "-c", "which git" }, // //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ - Charset.defaultCharset().name()); - if (w != null) { - // The path may be in cygwin/msys notation so resolve it right away - gitExe = resolve(null, w); - if (gitExe != null) - return resolveGrandparentFile(gitExe); + if (searchPath(path, "bash.exe") != null) { //$NON-NLS-1$ + // This isn't likely to work, but its worth trying: + // If bash is in $PATH, git should also be in $PATH. + String w = readPipe(userHome(), + new String[] { "bash", "--login", "-c", "which git" }, // //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ + Charset.defaultCharset().name()); + if (w != null) { + // The path may be in cygwin/msys notation so resolve it right away + gitExe = resolve(null, w); + if (gitExe != null) + return resolveGrandparentFile(gitExe); + } } + return null; } From 3d231bd95edfc5b212fdd77c0b9ff30ecd973200 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 8 May 2015 11:02:15 +0200 Subject: [PATCH 09/26] GroupHead: Remove a redundant call to String.format() Change-Id: I8f5fc09469b56d73d3838e7bcfecfd21140429eb Signed-off-by: Sebastian Schuberth --- org.eclipse.jgit/src/org/eclipse/jgit/fnmatch/GroupHead.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/fnmatch/GroupHead.java b/org.eclipse.jgit/src/org/eclipse/jgit/fnmatch/GroupHead.java index 0243bdc21..8af52288f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/fnmatch/GroupHead.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/fnmatch/GroupHead.java @@ -114,9 +114,9 @@ final class GroupHead extends AbstractHead { characterClasses.add(LetterPattern.INSTANCE); characterClasses.add(DigitPattern.INSTANCE); } else { - final String message = String.format(MessageFormat.format( + final String message = MessageFormat.format( JGitText.get().characterClassIsNotSupported, - characterClass)); + characterClass); throw new InvalidPatternException(message, wholePattern); } From 926ad4296e1a65db6a6a860122419402294343ce Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 8 May 2015 11:02:44 +0200 Subject: [PATCH 10/26] IndexDiffFilter: Simplify a boolean expression Change-Id: Ibdd0338b638b864d6572045b084b08a04471ecf7 Signed-off-by: Sebastian Schuberth --- .../src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java index 5aab24caf..3ef3d9791 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/IndexDiffFilter.java @@ -199,7 +199,7 @@ public boolean include(TreeWalk tw) throws MissingObjectException, // If i is cnt then the path does not appear in any other tree, // and this working tree entry can be safely ignored. - return i == cnt ? false : true; + return i != cnt; } else { // In working tree and not ignored, and not in DirCache. return true; From 11c393a1d45db23edb79542fadb301b96498ce05 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth Date: Fri, 8 May 2015 11:04:27 +0200 Subject: [PATCH 11/26] Do not concatenate strings as arguments to StringBuilder.append() That more or less defeats the purpose of using a StringBuilder. Change-Id: I519f7bf1c9b6670e63c3714210f834ee845dc69f Signed-off-by: Sebastian Schuberth --- .../jgit/internal/storage/pack/BinaryDelta.java | 16 ++++++++++++---- .../jgit/internal/storage/pack/ObjectToPack.java | 8 ++++---- .../eclipse/jgit/transport/BaseReceivePack.java | 8 +++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/BinaryDelta.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/BinaryDelta.java index 97092cd5e..2565931e0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/BinaryDelta.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/BinaryDelta.java @@ -255,8 +255,13 @@ public static String format(byte[] delta, boolean includeHeader) { shift += 7; } while ((c & 0x80) != 0); - if (includeHeader) - r.append("DELTA( BASE=" + baseLen + " RESULT=" + resLen + " )\n"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + if (includeHeader) { + r.append("DELTA( BASE="); //$NON-NLS-1$ + r.append(baseLen); + r.append(" RESULT="); //$NON-NLS-1$ + r.append(resLen); + r.append(" )\n"); //$NON-NLS-1$ + } while (deltaPtr < delta.length) { final int cmd = delta[deltaPtr++] & 0xff; @@ -285,8 +290,11 @@ public static String format(byte[] delta, boolean includeHeader) { if (copySize == 0) copySize = 0x10000; - r.append(" COPY (" + copyOffset + ", " + copySize + ")\n"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ - + r.append(" COPY ("); //$NON-NLS-1$ + r.append(copyOffset); + r.append(", "); //$NON-NLS-1$ + r.append(copySize); + r.append(")\n"); //$NON-NLS-1$ } else if (cmd != 0) { // Anything else the data is literal within the delta // itself. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/ObjectToPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/ObjectToPack.java index 33cb6af1e..a0896577f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/ObjectToPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/ObjectToPack.java @@ -390,15 +390,15 @@ public String toString() { if (isEdge()) buf.append(" edge"); if (getDeltaDepth() > 0) - buf.append(" depth=" + getDeltaDepth()); + buf.append(" depth=").append(getDeltaDepth()); if (isDeltaRepresentation()) { if (getDeltaBase() != null) - buf.append(" base=inpack:" + getDeltaBase().name()); + buf.append(" base=inpack:").append(getDeltaBase().name()); else - buf.append(" base=edge:" + getDeltaBaseId().name()); + buf.append(" base=edge:").append(getDeltaBaseId().name()); } if (isWritten()) - buf.append(" offset=" + getOffset()); + buf.append(" offset=").append(getOffset()); buf.append("]"); return buf.toString(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index cf6b2fd3d..36ea647c0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1461,9 +1461,11 @@ protected void sendStatusReport(final boolean forClient, case REJECTED_MISSING_OBJECT: if (cmd.getMessage() == null) r.append("missing object(s)"); //$NON-NLS-1$ - else if (cmd.getMessage().length() == Constants.OBJECT_ID_STRING_LENGTH) - r.append("object " + cmd.getMessage() + " missing"); //$NON-NLS-1$ //$NON-NLS-2$ - else + else if (cmd.getMessage().length() == Constants.OBJECT_ID_STRING_LENGTH) { + r.append("object "); //$NON-NLS-1$ + r.append(cmd.getMessage()); + r.append(" missing"); //$NON-NLS-1$ + } else r.append(cmd.getMessage()); break; From 53e39094bf012a4f5b3fe5557219707cb7b0f010 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 9 May 2015 10:47:13 -0700 Subject: [PATCH 12/26] ObjectWalk: make setRetainBody(false) the default Despite being the primary author of RevWalk and ObjectWalk I still fail to remember to setRetainBody(false) in application code using an ObjectWalk to examine the graph. Document the default for RevWalk is setRetainBody(true), where the application usually wants the commit bodies to display or inspect. Change the default for ObjectWalk to setRetainBody(false), as nearly all callers want only the graph shape and do not need the larger text inside a commit body. This allows some code in JGit to be simplified. Change-Id: I367e42209e805bd5e1f41b4072aeb2fa98ec9d99 --- .../eclipse/jgit/revwalk/FooterLineTest.java | 10 ++++---- .../eclipse/jgit/api/StashCreateCommand.java | 1 - .../eclipse/jgit/api/StashListCommand.java | 9 +++----- .../eclipse/jgit/blame/BlameGenerator.java | 1 - .../internal/storage/pack/PackWriter.java | 2 -- .../org/eclipse/jgit/revwalk/ObjectWalk.java | 1 + .../org/eclipse/jgit/revwalk/RevCommit.java | 23 +++++++++++-------- .../src/org/eclipse/jgit/revwalk/RevTag.java | 23 +++++++++++-------- .../src/org/eclipse/jgit/revwalk/RevWalk.java | 9 ++++++-- .../jgit/transport/BaseReceivePack.java | 1 - 10 files changed, 44 insertions(+), 36 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java index 0ad3b7742..fbd512711 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/FooterLineTest.java @@ -360,10 +360,10 @@ private RevCommit parse(final String msg) throws IOException { buf.append("\n"); buf.append(msg); - final RevWalk walk = new RevWalk(db); - walk.setRetainBody(true); - final RevCommit c = new RevCommit(ObjectId.zeroId()); - c.parseCanonical(walk, Constants.encode(buf.toString())); - return c; + try (RevWalk walk = new RevWalk(db)) { + RevCommit c = new RevCommit(ObjectId.zeroId()); + c.parseCanonical(walk, Constants.encode(buf.toString())); + return c; + } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashCreateCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashCreateCommand.java index af35f772c..f4d443d72 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashCreateCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashCreateCommand.java @@ -188,7 +188,6 @@ public StashCreateCommand setIncludeUntracked(boolean includeUntracked) { private RevCommit parseCommit(final ObjectReader reader, final ObjectId headId) throws IOException { final RevWalk walk = new RevWalk(reader); - walk.setRetainBody(true); return walk.parseCommit(headId); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashListCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashListCommand.java index bbbb7ace3..59a83aae3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashListCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashListCommand.java @@ -96,10 +96,8 @@ public Collection call() throws GitAPIException, final List stashCommits = new ArrayList( stashEntries.size()); - final RevWalk walk = new RevWalk(repo); - walk.setRetainBody(true); - try { - for (ReflogEntry entry : stashEntries) + try (RevWalk walk = new RevWalk(repo)) { + for (ReflogEntry entry : stashEntries) { try { stashCommits.add(walk.parseCommit(entry.getNewId())); } catch (IOException e) { @@ -107,8 +105,7 @@ public Collection call() throws GitAPIException, JGitText.get().cannotReadCommit, entry.getNewId()), e); } - } finally { - walk.dispose(); + } } return stashCommits; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index eb64425c6..ae713e2e9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -179,7 +179,6 @@ private void initRevPool(boolean reverse) { else revPool = new RevWalk(getRepository()); - revPool.setRetainBody(true); SEEN = revPool.newFlag("SEEN"); //$NON-NLS-1$ reader = revPool.getObjectReader(); treeWalk = new TreeWalk(reader); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java index 1422c5ef0..b30315af7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriter.java @@ -1587,8 +1587,6 @@ private void findObjectsToPack(final ProgressMonitor countingMonitor, stats.interestingObjects = Collections.unmodifiableSet(new HashSet(want)); stats.uninterestingObjects = Collections.unmodifiableSet(new HashSet(have)); - walker.setRetainBody(false); - canBuildBitmaps = config.isBuildBitmaps() && !shallowPack && have.isEmpty() diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java index a0af067dc..27cb0474a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java @@ -133,6 +133,7 @@ public ObjectWalk(final Repository repo) { */ public ObjectWalk(ObjectReader or) { super(or); + setRetainBody(false); rootObjects = new ArrayList(); pendingObjects = new BlockObjQueue(); pathBuf = new byte[256]; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java index fac4f809f..37f1d7b8d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java @@ -92,29 +92,34 @@ public static RevCommit parse(byte[] raw) { /** * Parse a commit from its canonical format. - * + *

* This method inserts the commit directly into the caller supplied revision * pool, making it appear as though the commit exists in the repository, * even if it doesn't. The repository under the pool is not affected. + *

+ * The body of the commit (message, author, committer) is always retained in + * the returned {@code RevCommit}, even if the supplied {@code RevWalk} has + * been configured with {@code setRetainBody(false)}. * * @param rw * the revision pool to allocate the commit within. The commit's * tree and parent pointers will be obtained from this pool. * @param raw - * the canonical formatted commit to be parsed. + * the canonical formatted commit to be parsed. This buffer will + * be retained by the returned {@code RevCommit} and must not be + * modified by the caller. * @return the parsed commit, in an isolated revision pool that is not * available to the caller. * @throws IOException * in case of RevWalk initialization fails */ public static RevCommit parse(RevWalk rw, byte[] raw) throws IOException { - ObjectInserter.Formatter fmt = new ObjectInserter.Formatter(); - boolean retain = rw.isRetainBody(); - rw.setRetainBody(true); - RevCommit r = rw.lookupCommit(fmt.idFor(Constants.OBJ_COMMIT, raw)); - r.parseCanonical(rw, raw); - rw.setRetainBody(retain); - return r; + try (ObjectInserter.Formatter fmt = new ObjectInserter.Formatter()) { + RevCommit r = rw.lookupCommit(fmt.idFor(Constants.OBJ_COMMIT, raw)); + r.parseCanonical(rw, raw); + r.buffer = raw; + return r; + } } static final RevCommit[] NO_PARENTS = {}; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java index 1a59b440c..75201164c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java @@ -87,16 +87,22 @@ public static RevTag parse(byte[] raw) throws CorruptObjectException { /** * Parse an annotated tag from its canonical format. - * + *

* This method inserts the tag directly into the caller supplied revision * pool, making it appear as though the tag exists in the repository, even * if it doesn't. The repository under the pool is not affected. + *

+ * The body of the tag (message, tagger, signature) is always retained in + * the returned {@code RevTag}, even if the supplied {@code RevWalk} has + * been configured with {@code setRetainBody(false)}. * * @param rw * the revision pool to allocate the tag within. The tag's object * pointer will be obtained from this pool. * @param raw - * the canonical formatted tag to be parsed. + * the canonical formatted tag to be parsed. This buffer will be + * retained by the returned {@code RevTag} and must not be + * modified by the caller. * @return the parsed tag, in an isolated revision pool that is not * available to the caller. * @throws CorruptObjectException @@ -104,13 +110,12 @@ public static RevTag parse(byte[] raw) throws CorruptObjectException { */ public static RevTag parse(RevWalk rw, byte[] raw) throws CorruptObjectException { - ObjectInserter.Formatter fmt = new ObjectInserter.Formatter(); - boolean retain = rw.isRetainBody(); - rw.setRetainBody(true); - RevTag r = rw.lookupTag(fmt.idFor(Constants.OBJ_TAG, raw)); - r.parseCanonical(rw, raw); - rw.setRetainBody(retain); - return r; + try (ObjectInserter.Formatter fmt = new ObjectInserter.Formatter()) { + RevTag r = rw.lookupTag(fmt.idFor(Constants.OBJ_TAG, raw)); + r.parseCanonical(rw, raw); + r.buffer = raw; + return r; + } } private RevObject object; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java index 4ce422ca9..d1009abae 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java @@ -192,7 +192,7 @@ public class RevWalk implements Iterable, AutoCloseable { private TreeFilter treeFilter; - private boolean retainBody; + private boolean retainBody = true; private boolean rewriteParents = true; @@ -233,7 +233,6 @@ private RevWalk(ObjectReader or, boolean closeReader) { sorting = EnumSet.of(RevSort.NONE); filter = RevFilter.ALL; treeFilter = TreeFilter.ALL; - retainBody = true; this.closeReader = closeReader; } @@ -607,6 +606,9 @@ boolean getRewriteParents() { * Usually the body is always retained, but some application code might not * care and would prefer to discard the body of a commit as early as * possible, to reduce memory usage. + *

+ * True by default on {@link RevWalk} and false by default for + * {@link ObjectWalk}. * * @return true if the body should be retained; false it is discarded. */ @@ -620,6 +622,9 @@ public boolean isRetainBody() { * If a body of a commit or tag is not retained, the application must * call {@link #parseBody(RevObject)} before the body can be safely * accessed through the type specific access methods. + *

+ * True by default on {@link RevWalk} and false by default for + * {@link ObjectWalk}. * * @param retain true to retain bodies; false to discard them early. */ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 36ea647c0..a18df0dd0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1143,7 +1143,6 @@ private void checkConnectivity() throws IOException { parser = null; try (final ObjectWalk ow = new ObjectWalk(db)) { - ow.setRetainBody(false); if (baseObjects != null) { ow.sort(RevSort.TOPO); if (!baseObjects.isEmpty()) From af7dcd6e1b22d21f4563a75181c5d269fd8105b5 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 8 May 2015 22:48:10 -0700 Subject: [PATCH 13/26] RevWalk: Discard uninteresting commits unless RevSort.BOUNDARY Previously using an ObjectWalk meant uninteresting commits may keep their commit message buffers in memory just in case they were found to be on the boundary and were output as UNINTERESTING for the caller. This was incorrect inside StartGenerator. ObjectWalk hides these internal UNINTERESTING cases from its caller unless RevSort.BOUNDARY was explicitly set, and its false by default. Callers never see one of these saved uninteresting commits. Change the test to allow early dispose unless the application has explicitly asked for RevSort.BOUNDARY. This allows uninteresting commit buffers to be discarded and garbage collected in ObjectWalks when the caller will never be given the RevCommit. Change-Id: Ic1419cc1d9ee95f4d09386dd0730d54c12dcc157 --- .../src/org/eclipse/jgit/revwalk/StartGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java index 593e09e25..02469d6de 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java @@ -144,7 +144,7 @@ RevCommit next() throws MissingObjectException, } else { g = new PendingGenerator(w, pending, rf, pendingOutputType); - if (boundary) { + if (walker.hasRevSort(RevSort.BOUNDARY)) { // Because the boundary generator may produce uninteresting // commits we cannot allow the pending generator to dispose // of them early. From ca7daa5226d88c8533c2316e5bc8a66ea8a373c5 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 8 May 2015 22:26:28 -0700 Subject: [PATCH 14/26] ObjectReader: remove the walkAdvice API This was added a very long time ago to support the failed DHT storage implementation. Since then no storage system was able to make use of this API, but it pollutes internals of the walkers. Kill the API on ObjectReader and drop the invocations from the walker code. Change-Id: I36608afdac13a6c3084d7c7e0af5e0cb22900332 --- .../org/eclipse/jgit/lib/ObjectReader.java | 41 ------------------- .../jgit/revwalk/MergeBaseGenerator.java | 1 - .../org/eclipse/jgit/revwalk/ObjectWalk.java | 14 ------- .../jgit/revwalk/PendingGenerator.java | 2 - .../eclipse/jgit/revwalk/StartGenerator.java | 2 - 5 files changed, 60 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java index 524dafb88..4c4e53455 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectReader.java @@ -53,9 +53,6 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.storage.pack.ObjectReuseAsIs; -import org.eclipse.jgit.revwalk.ObjectWalk; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; /** * Reads an {@link ObjectDatabase} for a single thread. @@ -398,44 +395,6 @@ public void release() { }; } - /** - * Advice from a {@link RevWalk} that a walk is starting from these roots. - * - * @param walk - * the revision pool that is using this reader. - * @param roots - * starting points of the revision walk. The starting points have - * their headers parsed, but might be missing bodies. - * @throws IOException - * the reader cannot initialize itself to support the walk. - */ - public void walkAdviceBeginCommits(RevWalk walk, Collection roots) - throws IOException { - // Do nothing by default, most readers don't want or need advice. - } - - /** - * Advice from an {@link ObjectWalk} that trees will be traversed. - * - * @param ow - * the object pool that is using this reader. - * @param min - * the first commit whose root tree will be read. - * @param max - * the last commit whose root tree will be read. - * @throws IOException - * the reader cannot initialize itself to support the walk. - */ - public void walkAdviceBeginTrees(ObjectWalk ow, RevCommit min, RevCommit max) - throws IOException { - // Do nothing by default, most readers don't want or need advice. - } - - /** Advice from that a walk is over. */ - public void walkAdviceEnd() { - // Do nothing by default, most readers don't want or need advice. - } - /** * Advise the reader to avoid unreachable objects. *

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/MergeBaseGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/MergeBaseGenerator.java index 7822947c9..f1d7dc836 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/MergeBaseGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/MergeBaseGenerator.java @@ -137,7 +137,6 @@ RevCommit next() throws MissingObjectException, for (;;) { final RevCommit c = pending.next(); if (c == null) { - walker.reader.walkAdviceEnd(); return null; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java index 27cb0474a..18c4233eb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java @@ -99,10 +99,6 @@ public class ObjectWalk extends RevWalk { private BlockObjQueue pendingObjects; - private RevCommit firstCommit; - - private RevCommit lastCommit; - private TreeVisit freeVisit; private TreeVisit currVisit; @@ -260,8 +256,6 @@ public RevCommit next() throws MissingObjectException, for (;;) { final RevCommit r = super.next(); if (r == null) { - if (firstCommit != null) - reader.walkAdviceBeginTrees(this, firstCommit, lastCommit); return null; } if ((r.flags & UNINTERESTING) != 0) { @@ -270,9 +264,6 @@ public RevCommit next() throws MissingObjectException, return r; continue; } - if (firstCommit == null) - firstCommit = r; - lastCommit = r; pendingObjects.add(r.getTree()); return r; } @@ -366,7 +357,6 @@ public RevObject nextObject() throws MissingObjectException, for (;;) { RevObject o = pendingObjects.next(); if (o == null) { - reader.walkAdviceEnd(); return null; } int flags = o.flags; @@ -634,8 +624,6 @@ private void growPathBuf(int ptr) { public void dispose() { super.dispose(); pendingObjects = new BlockObjQueue(); - firstCommit = null; - lastCommit = null; currVisit = null; freeVisit = null; } @@ -649,8 +637,6 @@ protected void reset(final int retainFlags) { rootObjects = new ArrayList(); pendingObjects = new BlockObjQueue(); - firstCommit = null; - lastCommit = null; currVisit = null; freeVisit = null; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java index f24c27873..94ae2c993 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/PendingGenerator.java @@ -128,7 +128,6 @@ RevCommit next() throws MissingObjectException, for (;;) { final RevCommit c = pending.next(); if (c == null) { - walker.reader.walkAdviceEnd(); return null; } @@ -177,7 +176,6 @@ else if (canDispose) c.disposeBody(); } } catch (StopWalkException swe) { - walker.reader.walkAdviceEnd(); pending.clear(); return null; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java index 02469d6de..1ec629031 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/StartGenerator.java @@ -85,8 +85,6 @@ RevCommit next() throws MissingObjectException, final TreeFilter tf = w.getTreeFilter(); AbstractRevQueue q = walker.queue; - w.reader.walkAdviceBeginCommits(w, w.roots); - if (rf == RevFilter.MERGE_BASE) { // Computing for merge bases is a special case and does not // use the bulk of the generator pipeline. From e4e947049f60d72f766d90dff22b488cbe06ba95 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 10 May 2015 10:42:09 -0700 Subject: [PATCH 15/26] Expose disposeBody() on RevCommit and RevTag Applications that use a commit message once and do not need it again can free the body to save memory. Expose the disposeBody() methods to support this and use it in pgm.Log which only visits each commit once. Change-Id: I4142a0749c24f15386ee7fb119934a0432234de3 --- .../src/org/eclipse/jgit/pgm/Log.java | 1 + .../src/org/eclipse/jgit/revwalk/RevCommit.java | 14 +++++++++++++- .../src/org/eclipse/jgit/revwalk/RevTag.java | 13 ++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java index 048526ee8..688de2c49 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java @@ -267,6 +267,7 @@ protected void show(final RevCommit c) throws Exception { outw.print(s); outw.println(); } + c.disposeBody(); outw.println(); if (showNotes(c)) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java index 37f1d7b8d..c23e4e328 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java @@ -609,7 +609,19 @@ public void reset() { inDegree = 0; } - final void disposeBody() { + /** + * Discard the message buffer to reduce memory usage. + *

+ * After discarding the memory usage of the {@code RevCommit} is reduced to + * only the {@link #getTree()} and {@link #getParents()} pointers and the + * time in {@link #getCommitTime()}. Accessing other properties such as + * {@link #getAuthorIdent()}, {@link #getCommitterIdent()} or either message + * function requires reloading the buffer by invoking + * {@link RevWalk#parseBody(RevObject)}. + * + * @since 4.0 + */ + public final void disposeBody() { buffer = null; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java index 75201164c..bf2785e0d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevTag.java @@ -270,7 +270,18 @@ public final String getTagName() { return tagName; } - final void disposeBody() { + /** + * Discard the message buffer to reduce memory usage. + *

+ * After discarding the memory usage of the {@code RevTag} is reduced to + * only the {@link #getObject()} pointer and {@link #getTagName()}. + * Accessing other properties such as {@link #getTaggerIdent()} or either + * message function requires reloading the buffer by invoking + * {@link RevWalk#parseBody(RevObject)}. + * + * @since 4.0 + */ + public final void disposeBody() { buffer = null; } } From bfdd9630833a32856fc3b5396b17406192ea7634 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 10 May 2015 12:19:12 -0700 Subject: [PATCH 16/26] FS_POSIX: Rework umask detection to make it settable Avoid always calling `sh -c umask` on startup, instead deferring the invocation until the first time a working tree file needs to use the execute bit. This allows servers using bare repos to avoid a costly fork+exec for a value that is never used. Store the umask as an int instead of two Boolean. This is slightly smaller memory (one int vs. two references) and makes it easier for an application to force setting the umask to a value that overrides whatever the shell told JGit. Simplify the code to bail by returning early when canExecute is false, which is the common case for working tree files. Change-Id: Ie713647615bc5bdf5d71b731a6748c28ea21c900 --- .../org/eclipse/jgit/util/FSJava7Test.java | 57 ++--- .../src/org/eclipse/jgit/util/FS_POSIX.java | 222 ++++++------------ 2 files changed, 86 insertions(+), 193 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java index 9fba374dc..e6a244e14 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSJava7Test.java @@ -46,15 +46,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeFalse; -import static org.junit.Assume.assumeNotNull; import static org.junit.Assume.assumeTrue; -import java.io.BufferedReader; import java.io.File; import java.io.IOException; -import java.io.InputStreamReader; -import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermission; @@ -123,29 +118,15 @@ public void testSymlinkAttributes() throws IOException, InterruptedException { @Test public void testExecutableAttributes() throws Exception { - FS fs = FS.DETECTED; + FS fs = FS.DETECTED.newInstance(); // If this assumption fails the test is halted and ignored. assumeTrue(fs instanceof FS_POSIX); + ((FS_POSIX) fs).setUmask(0022); File f = new File(trash, "bla"); assertTrue(f.createNewFile()); assertFalse(fs.canExecute(f)); - String umask = readUmask(); - assumeNotNull(umask); - - char others = umask.charAt(umask.length() - 1); - - boolean badUmask; - if (others != '0' && others != '2' && others != '4' && others != '6') { - // umask is set in the way that "others" can not "execute" => git - // CLI will not set "execute" attribute for "others", so we also - // don't care - badUmask = true; - } else { - badUmask = false; - } - Set permissions = readPermissions(f); assertTrue(!permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); assertTrue(!permissions.contains(PosixFilePermission.GROUP_EXECUTE)); @@ -158,27 +139,21 @@ public void testExecutableAttributes() throws Exception { permissions.contains(PosixFilePermission.OWNER_EXECUTE)); assertTrue("'group' execute permission not set", permissions.contains(PosixFilePermission.GROUP_EXECUTE)); - if (badUmask) { - assertFalse("'others' execute permission set", - permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); - System.err.println("WARNING: your system's umask: \"" + umask - + "\" doesn't allow FSJava7Test to test if setting posix" - + " permissions for \"others\" works properly"); - assumeFalse(badUmask); - } else { - assertTrue("'others' execute permission not set", - permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); - } - } + assertTrue("'others' execute permission not set", + permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); - private String readUmask() throws Exception { - Process p = Runtime.getRuntime().exec( - new String[] { "sh", "-c", "umask" }, null, null); - final BufferedReader lineRead = new BufferedReader( - new InputStreamReader(p.getInputStream(), Charset - .defaultCharset().name())); - p.waitFor(); - return lineRead.readLine(); + ((FS_POSIX) fs).setUmask(0033); + fs.setExecute(f, false); + assertFalse(fs.canExecute(f)); + fs.setExecute(f, true); + + permissions = readPermissions(f); + assertTrue("'owner' execute permission not set", + permissions.contains(PosixFilePermission.OWNER_EXECUTE)); + assertFalse("'group' execute permission set", + permissions.contains(PosixFilePermission.GROUP_EXECUTE)); + assertFalse("'others' execute permission set", + permissions.contains(PosixFilePermission.OTHERS_EXECUTE)); } private Set readPermissions(File f) throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 47760f661..9c92d7c4c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -66,140 +66,70 @@ * @since 3.0 */ public class FS_POSIX extends FS { + private static final int DEFAULT_UMASK = 0022; + private volatile int umask = -1; - static { - String umask = readUmask(); - - // umask return value consists of 3 or 4 digits, like "002" or "0002" - if (umask != null && umask.length() > 0 && umask.matches("\\d{3,4}")) { //$NON-NLS-1$ - EXECUTE_FOR_OTHERS = isGranted(PosixFilePermission.OTHERS_EXECUTE, - umask); - EXECUTE_FOR_GROUP = isGranted(PosixFilePermission.GROUP_EXECUTE, - umask); - } else { - EXECUTE_FOR_OTHERS = null; - EXECUTE_FOR_GROUP = null; - } + /** Default constructor. */ + protected FS_POSIX() { } /** - * @since 4.0 + * Constructor + * + * @param src + * FS to copy some settings from */ - protected static final Boolean EXECUTE_FOR_OTHERS; - - /** - * @since 4.0 - */ - protected static final Boolean EXECUTE_FOR_GROUP; + protected FS_POSIX(FS src) { + super(src); + if (src instanceof FS_POSIX) { + umask = ((FS_POSIX) src).umask; + } + } @Override public FS newInstance() { - return new FS_POSIX(); + return new FS_POSIX(this); } /** - * Derives requested permission from given octal umask value as defined e.g. - * in http://linux.die.net/man/2/ - * umask. - *

- * The umask expected here must consist of 3 or 4 digits. Last three digits - * are significant here because they represent file permissions granted to - * the "owner", "group" and "others" (in this order). - *

- * Each single digit from the umask represents 3 bits of the mask standing - * for "read, write, execute" permissions (in this - * order). - *

- * The possible umask values table: + * Set the umask, overriding any value observed from the shell. * - * 

-	 * Value : Bits:Abbr.: Permission
-	 *     0 : 000 :rwx  : read, write and execute
-	 *     1 : 001 :rw   : read and write
-	 *     2 : 010 :rx   : read and execute
-	 *     3 : 011 :r    : read only
-	 *     4 : 100 :wx   : write and execute
-	 *     5 : 101 :w    : write only
-	 *     6 : 110 :x    : execute only
-	 *     7 : 111 :     : no permissions
-	 *
- *

- * Note, that umask value is used to "mask" the requested permissions on - * file creation by combining the requested permission bit with the - * negated value of the umask bit. - *

- * Simply speaking, if a bit is not set in the umask, then the - * appropriate right will be granted if requested. If a bit is - * set in the umask value, then the appropriate permission will be not - * granted. - *

- * Example: - *

  • umask 023 ("000 010 011" or rwx rx r) combined with the request to - * create an executable file with full set of permissions for everyone (777) - * results in the file with permissions 754 (rwx rx r). - *
  • umask 002 ("000 000 010" or rwx rwx rx) combined with the request to - * create an executable file with full set of permissions for everyone (777) - * results in the file with permissions 775 (rwx rwx rx). - *
  • umask 002 ("000 000 010" or rwx rwx rx) combined with the request to - * create a file without executable rights for everyone (666) results in the - * file with permissions 664 (rw rw r). - * - * @param p - * non null permission * @param umask - * octal umask value represented by at least three digits. The - * digits (read from the end to beginning of the umask) represent - * permissions for "others", "group" and "owner". - * - * @return true if the requested permission is set according to given umask + * mask to apply when creating files. * @since 4.0 */ - protected static Boolean isGranted(PosixFilePermission p, String umask) { - char val; - switch (p) { - case OTHERS_EXECUTE: - // Read last digit, because umask is ordered as: User/Group/Others. - val = umask.charAt(umask.length() - 1); - return isExecuteGranted(val); - case GROUP_EXECUTE: - val = umask.charAt(umask.length() - 2); - return isExecuteGranted(val); - default: - throw new UnsupportedOperationException( - "isGranted() for " + p + " is not implemented!"); //$NON-NLS-1$ //$NON-NLS-2$ + public void setUmask(int umask) { + this.umask = umask; + } + + private int umask() { + int u = umask; + if (u == -1) { + u = readUmask(); + umask = u; } + return u; } - /** - * @param c - * character representing octal permission value from the table - * in {@link #isGranted(PosixFilePermission, String)} - * @return true if the "execute" permission is granted according to given - * character - */ - private static Boolean isExecuteGranted(char c) { - if (c == '0' || c == '2' || c == '4' || c == '6') - return Boolean.TRUE; - return Boolean.FALSE; - } - - /** - * @return umask returned from running umask command in a shell - * @since 4.0 - */ - protected static String readUmask() { - Process p; + /** @return mask returned from running {@code umask} command in shell. */ + private static int readUmask() { try { - p = Runtime.getRuntime().exec( - new String[] { "sh", "-c", "umask" }, null, null); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + Process p = Runtime.getRuntime().exec( + new String[] { "sh", "-c", "umask" }, //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + null, null); try (BufferedReader lineRead = new BufferedReader( new InputStreamReader(p.getInputStream(), Charset .defaultCharset().name()))) { - p.waitFor(); - return lineRead.readLine(); + if (p.waitFor() == 0) { + String s = lineRead.readLine(); + if (s.matches("0?\\d{3}")) { //$NON-NLS-1$ + return Integer.parseInt(s, 8); + } + } + return DEFAULT_UMASK; } } catch (Exception e) { - return null; + return DEFAULT_UMASK; } } @@ -229,23 +159,6 @@ protected File discoverGitPrefix() { return null; } - /** - * Default constructor - */ - protected FS_POSIX() { - super(); - } - - /** - * Constructor - * - * @param src - * FS to copy some settings from - */ - protected FS_POSIX(FS src) { - super(src); - } - @Override public boolean isCaseSensitive() { return !SystemReader.getInstance().isMacOS(); @@ -265,35 +178,40 @@ public boolean canExecute(File f) { public boolean setExecute(File f, boolean canExecute) { if (!isFile(f)) return false; - // only if the execute has to be set, and we know the umask - if (canExecute && EXECUTE_FOR_OTHERS != null) { - try { - Path path = f.toPath(); - Set pset = Files - .getPosixFilePermissions(path); - // user is always allowed to set execute - pset.add(PosixFilePermission.OWNER_EXECUTE); + if (!canExecute) + return f.setExecutable(false); - if (EXECUTE_FOR_GROUP.booleanValue()) - pset.add(PosixFilePermission.GROUP_EXECUTE); + try { + Path path = f.toPath(); + Set pset = Files.getPosixFilePermissions(path); - if (EXECUTE_FOR_OTHERS.booleanValue()) - pset.add(PosixFilePermission.OTHERS_EXECUTE); + // owner (user) is always allowed to execute. + pset.add(PosixFilePermission.OWNER_EXECUTE); - Files.setPosixFilePermissions(path, pset); - return true; - } catch (IOException e) { - // The interface doesn't allow to throw IOException - final boolean debug = Boolean.parseBoolean(SystemReader - .getInstance().getProperty("jgit.fs.debug")); //$NON-NLS-1$ - if (debug) - System.err.println(e); - return false; - } + int mask = umask(); + apply(pset, mask, PosixFilePermission.GROUP_EXECUTE, 1 << 3); + apply(pset, mask, PosixFilePermission.OTHERS_EXECUTE, 1); + Files.setPosixFilePermissions(path, pset); + return true; + } catch (IOException e) { + // The interface doesn't allow to throw IOException + final boolean debug = Boolean.parseBoolean(SystemReader + .getInstance().getProperty("jgit.fs.debug")); //$NON-NLS-1$ + if (debug) + System.err.println(e); + return false; + } + } + + private static void apply(Set set, + int umask, PosixFilePermission perm, int test) { + if ((umask & test) == 0) { + // If bit is clear in umask, permission is allowed. + set.add(perm); + } else { + // If bit is set in umask, permission is denied. + set.remove(perm); } - // if umask is not working for some reason: fall back to default (buggy) - // implementation which does not consider umask: see bug 424395 - return f.setExecutable(canExecute); } @Override From 8b9623511f216027294b0cf4d2ecb7710915238a Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 13 Apr 2015 01:01:13 +0200 Subject: [PATCH 17/26] Use ANY_DIFF filter in ResolveMerger only for bare repositories As Chris pointed out change I822721c76c64e614f87a080ced2457941f53adcd slowed down merge since ANY_DIFF filter is much less efficient than the manual detection of diffs done in ResolveMerger.processEntry() since it avoids unnecessary filesystem calls using the git index. Hence only set the ANY_DIFF filter on bare repositories which don't have a working tree to scan. To test performance I used the setup described in Chris' comment on change I822721c76c64e614f87a080ced2457941f53adcd and modified ResolveMerger.mergeTrees() to not add the working tree in order to simulate merging in a bare repository. At least on Mac I couldn't detect a speedup, with and without the ANY_DIFF filter merge test takes an average 0.67sec. Change-Id: I17b3a06f369cee009490f54ad1a2deb6c145c7cf Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/merge/ResolveMerger.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index 953d3a2cd..3654ffd1e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java @@ -90,6 +90,7 @@ import org.eclipse.jgit.treewalk.NameConflictTreeWalk; import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.WorkingTreeIterator; +import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.TemporaryBuffer; @@ -1012,8 +1013,11 @@ protected boolean mergeTrees(AbstractTreeIterator baseTree, tw.addTree(headTree); tw.addTree(mergeTree); tw.addTree(buildIt); - if (workingTreeIterator != null) + if (workingTreeIterator != null) { tw.addTree(workingTreeIterator); + } else { + tw.setFilter(TreeFilter.ANY_DIFF); + } if (!mergeTreeWalk(tw, ignoreConflicts)) { return false; From 3bc44010108a34e5fee3473412a0a28d2b19258d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 9 May 2015 00:56:43 +0200 Subject: [PATCH 18/26] Update javax.servlet to 3.1 Change-Id: Ifad154ed2f52f0102d297ac7fd4943ff1f309b9e Signed-off-by: Matthias Sohn --- org.eclipse.jgit.http.server/META-INF/MANIFEST.MF | 4 ++-- org.eclipse.jgit.http.test/META-INF/MANIFEST.MF | 4 ++-- org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF | 4 ++-- .../org.eclipse.jgit.target/jgit-4.3.target | 6 +++--- .../org.eclipse.jgit.target/jgit-4.4.target | 6 +++--- .../org.eclipse.jgit.target/jgit-4.5.target | 6 +++--- .../orbit/R20150124073747-Luna-SR2.tpd | 4 ++-- .../orbit/S20150202203538-Mars-M5.tpd | 4 ++-- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF index 2c76d7454..5304f83be 100644 --- a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF @@ -15,8 +15,8 @@ Export-Package: org.eclipse.jgit.http.server;version="4.0.0", javax.servlet.http" Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: JavaSE-1.7 -Import-Package: javax.servlet;version="[2.5.0,3.0.0)", - javax.servlet.http;version="[2.5.0,3.0.0)", +Import-Package: javax.servlet;version="[2.5.0,3.2.0)", + javax.servlet.http;version="[2.5.0,3.2.0)", org.eclipse.jgit.errors;version="[4.0.0,4.1.0)", org.eclipse.jgit.internal.storage.dfs;version="[4.0.0,4.1.0)", org.eclipse.jgit.internal.storage.file;version="[4.0.0,4.1.0)", diff --git a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF index f38827a7b..184c8a440 100644 --- a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF @@ -6,8 +6,8 @@ Bundle-Version: 4.0.0.qualifier Bundle-Vendor: %provider_name Bundle-Localization: plugin Bundle-RequiredExecutionEnvironment: JavaSE-1.7 -Import-Package: javax.servlet;version="[2.5.0,3.0.0)", - javax.servlet.http;version="[2.5.0,3.0.0)", +Import-Package: javax.servlet;version="[2.5.0,3.2.0)", + javax.servlet.http;version="[2.5.0,3.2.0)", org.eclipse.jetty.continuation;version="[7.6.0,8.0.0)", org.eclipse.jetty.http;version="[7.6.0,8.0.0)", org.eclipse.jetty.io;version="[7.6.0,8.0.0)", diff --git a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF index 8bd5a697e..4e421ebcd 100644 --- a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF @@ -7,8 +7,8 @@ Bundle-Localization: plugin Bundle-Vendor: %provider_name Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: JavaSE-1.7 -Import-Package: javax.servlet;version="[2.5.0,3.0.0)", - javax.servlet.http;version="[2.5.0,3.0.0)", +Import-Package: javax.servlet;version="[2.5.0,3.2.0)", + javax.servlet.http;version="[2.5.0,3.2.0)", org.apache.commons.logging;version="[1.1.1,2.0.0)", org.eclipse.jetty.http;version="[7.6.0,8.0.0)", org.eclipse.jetty.security;version="[7.6.0,8.0.0)", diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target index 44be1e513..525e56981 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target @@ -1,7 +1,7 @@ - + @@ -49,8 +49,8 @@ - - + + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target index 38e25a416..083ece881 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target @@ -1,7 +1,7 @@ - + @@ -49,8 +49,8 @@ - - + + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target index e5d14330e..42adbb3d9 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target @@ -1,7 +1,7 @@ - + @@ -49,8 +49,8 @@ - - + + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20150124073747-Luna-SR2.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20150124073747-Luna-SR2.tpd index d79b41ade..bad350b9c 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20150124073747-Luna-SR2.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/R20150124073747-Luna-SR2.tpd @@ -28,8 +28,8 @@ location "http://download.eclipse.org/tools/orbit/downloads/drops/R2015012407374 com.jcraft.jsch.source [0.1.51.v201410302000,0.1.51.v201410302000] org.junit [4.11.0.v201303080030,4.11.0.v201303080030] org.junit.source [4.11.0.v201303080030,4.11.0.v201303080030] - javax.servlet [2.5.0.v201103041518,2.5.0.v201103041518] - javax.servlet.source [2.5.0.v201103041518,2.5.0.v201103041518] + javax.servlet [3.1.0.v20140303-1611,3.1.0.v20140303-1611] + javax.servlet.source [3.1.0.v20140303-1611,3.1.0.v20140303-1611] org.tukaani.xz [1.3.0.v201308270617,1.3.0.v201308270617] org.tukaani.xz.source [1.3.0.v201308270617,1.3.0.v201308270617] org.slf4j.api [1.7.2.v20121108-1250,1.7.2.v20121108-1250] diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20150202203538-Mars-M5.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20150202203538-Mars-M5.tpd index 3ec1001c9..94419cb97 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20150202203538-Mars-M5.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/orbit/S20150202203538-Mars-M5.tpd @@ -28,8 +28,8 @@ location "http://download.eclipse.org/tools/orbit/downloads/drops/S2015020220353 com.jcraft.jsch.source [0.1.51.v201410302000,0.1.51.v201410302000] org.junit [4.11.0.v201303080030,4.11.0.v201303080030] org.junit.source [4.11.0.v201303080030,4.11.0.v201303080030] - javax.servlet [2.5.0.v201103041518,2.5.0.v201103041518] - javax.servlet.source [2.5.0.v201103041518,2.5.0.v201103041518] + javax.servlet [3.1.0.v201410161800,3.1.0.v201410161800] + javax.servlet.source [3.1.0.v201410161800,3.1.0.v201410161800] org.tukaani.xz [1.3.0.v201308270617,1.3.0.v201308270617] org.tukaani.xz.source [1.3.0.v201308270617,1.3.0.v201308270617] org.slf4j.api [1.7.2.v20121108-1250,1.7.2.v20121108-1250] From a24b7c3cc7053aaf9ac3621120b4fa42badcd4fe Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 10 May 2015 09:48:38 +0200 Subject: [PATCH 19/26] Update to Jetty 9.2.10 Change-Id: Iace29e6e99836019bb603ce06a08b91bada7c627 Signed-off-by: Matthias Sohn --- .../META-INF/MANIFEST.MF | 28 +++++++-------- .../jgit/http/test/AsIsServiceTest.java | 2 +- .../test/DefaultReceivePackFactoryTest.java | 2 +- .../test/DefaultUploadPackFactoryTest.java | 2 +- .../http/test/DumbClientSmartServerTest.java | 6 ++-- .../http/test/SmartClientSmartServerTest.java | 12 ++++--- .../META-INF/MANIFEST.MF | 20 +++++------ .../eclipse/jgit/junit/http/AppServer.java | 20 +++++++---- .../jgit/junit/http/RecordingLogger.java | 5 +++ .../jgit/junit/http/TestRequestLog.java | 2 +- .../org.eclipse.jgit.target/jgit-4.3.target | 36 +++++++++---------- .../org.eclipse.jgit.target/jgit-4.3.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.4.target | 36 +++++++++---------- .../org.eclipse.jgit.target/jgit-4.4.tpd | 2 +- .../org.eclipse.jgit.target/jgit-4.5.target | 36 +++++++++---------- .../org.eclipse.jgit.target/jgit-4.5.tpd | 2 +- .../projects/jetty-7.6.14.tpd | 20 ----------- .../projects/jetty-9.2.10.tpd | 20 +++++++++++ pom.xml | 2 +- 19 files changed, 136 insertions(+), 119 deletions(-) delete mode 100644 org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-7.6.14.tpd create mode 100644 org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-9.2.10.tpd diff --git a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF index 184c8a440..c65cbf161 100644 --- a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF @@ -8,20 +8,20 @@ Bundle-Localization: plugin Bundle-RequiredExecutionEnvironment: JavaSE-1.7 Import-Package: javax.servlet;version="[2.5.0,3.2.0)", javax.servlet.http;version="[2.5.0,3.2.0)", - org.eclipse.jetty.continuation;version="[7.6.0,8.0.0)", - org.eclipse.jetty.http;version="[7.6.0,8.0.0)", - org.eclipse.jetty.io;version="[7.6.0,8.0.0)", - org.eclipse.jetty.security;version="[7.6.0,8.0.0)", - org.eclipse.jetty.security.authentication;version="[7.6.0,8.0.0)", - org.eclipse.jetty.server;version="[7.6.0,8.0.0)", - org.eclipse.jetty.server.handler;version="[7.6.0,8.0.0)", - org.eclipse.jetty.server.nio;version="[7.6.0,8.0.0)", - org.eclipse.jetty.servlet;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.component;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.log;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.security;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.thread;version="[7.6.0,8.0.0)", + org.eclipse.jetty.continuation;version="[9.0.0,10.0.0)", + org.eclipse.jetty.http;version="[9.0.0,10.0.0)", + org.eclipse.jetty.io;version="[9.0.0,10.0.0)", + org.eclipse.jetty.security;version="[9.0.0,10.0.0)", + org.eclipse.jetty.security.authentication;version="[9.0.0,10.0.0)", + org.eclipse.jetty.server;version="[9.0.0,10.0.0)", + org.eclipse.jetty.server.handler;version="[9.0.0,10.0.0)", + org.eclipse.jetty.server.nio;version="[9.0.0,10.0.0)", + org.eclipse.jetty.servlet;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.component;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.log;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.security;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.thread;version="[9.0.0,10.0.0)", org.eclipse.jgit.errors;version="[4.0.0,4.1.0)", org.eclipse.jgit.http.server;version="[4.0.0,4.1.0)", org.eclipse.jgit.http.server.glue;version="[4.0.0,4.1.0)", diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AsIsServiceTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AsIsServiceTest.java index a86ae0930..c6b8f092b 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AsIsServiceTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/AsIsServiceTest.java @@ -133,7 +133,7 @@ private static final class R extends HttpServletRequestWrapper { private final String host; R(final String user, final String host) { - super(new Request() /* can't pass null, sigh */); + super(new Request(null, null) /* can't pass null, sigh */); this.user = user; this.host = host; } diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultReceivePackFactoryTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultReceivePackFactoryTest.java index 04f7c52f9..772865df7 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultReceivePackFactoryTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultReceivePackFactoryTest.java @@ -204,7 +204,7 @@ private static final class R extends HttpServletRequestWrapper { private final String host; R(final String user, final String host) { - super(new Request() /* can't pass null, sigh */); + super(new Request(null, null) /* can't pass null, sigh */); this.user = user; this.host = host; } diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultUploadPackFactoryTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultUploadPackFactoryTest.java index bc1370367..c9d43cdaf 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultUploadPackFactoryTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DefaultUploadPackFactoryTest.java @@ -160,7 +160,7 @@ private static final class R extends HttpServletRequestWrapper { private final String host; R(final String user, final String host) { - super(new Request() /* can't pass null, sigh */); + super(new Request(null, null) /* can't pass null, sigh */); this.user = user; this.host = host; } diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java index 7b4270f1b..e385b9538 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java @@ -199,7 +199,8 @@ public void testListRemote() throws IOException { .startsWith("JGit/")); assertEquals("*/*", info.getRequestHeader(HDR_ACCEPT)); assertEquals(200, info.getStatus()); - assertEquals("text/plain;charset=UTF-8", info + assertEquals("text/plain; charset=UTF-8", + info .getResponseHeader(HDR_CONTENT_TYPE)); AccessEvent head = requests.get(1); @@ -268,7 +269,8 @@ public void testInitialClone_Packed() throws Exception { assertEquals("GET", req.get(0).getMethod()); assertEquals(0, req.get(0).getParameters().size()); assertEquals(200, req.get(0).getStatus()); - assertEquals("text/plain;charset=UTF-8", req.get(0).getResponseHeader( + assertEquals("text/plain; charset=UTF-8", + req.get(0).getResponseHeader( HDR_CONTENT_TYPE)); } diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java index 4e75a5632..1b6c552ce 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java @@ -59,9 +59,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.List; import java.util.Map; +import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -72,7 +74,6 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.servlet.FilterHolder; -import org.eclipse.jetty.servlet.FilterMapping; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jgit.errors.RemoteRepositoryException; @@ -185,7 +186,8 @@ public void init(FilterConfig filterConfig) throws ServletException { public void destroy() { // } - }), "/" + srcName + "/git-upload-pack", FilterMapping.DEFAULT); + }), "/" + srcName + "/git-upload-pack", + EnumSet.of(DispatcherType.REQUEST)); broken.addServlet(new ServletHolder(gs), "/*"); server.setUp(); @@ -480,7 +482,7 @@ public void testInitialClone_BrokenServer() throws Exception { } catch (TransportException err) { String exp = brokenURI + ": expected" + " Content-Type application/x-git-upload-pack-result;" - + " received Content-Type text/plain;charset=UTF-8"; + + " received Content-Type text/plain; charset=UTF-8"; assertEquals(exp, err.getMessage()); } } finally { @@ -504,8 +506,8 @@ public void testInitialClone_BrokenServer() throws Exception { assertEquals(join(brokenURI, "git-upload-pack"), service.getPath()); assertEquals(0, service.getParameters().size()); assertEquals(200, service.getStatus()); - assertEquals("text/plain;charset=UTF-8", service - .getResponseHeader(HDR_CONTENT_TYPE)); + assertEquals("text/plain; charset=UTF-8", + service.getResponseHeader(HDR_CONTENT_TYPE)); } @Test diff --git a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF index 4e421ebcd..6ebb18aa7 100644 --- a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF @@ -10,16 +10,16 @@ Bundle-RequiredExecutionEnvironment: JavaSE-1.7 Import-Package: javax.servlet;version="[2.5.0,3.2.0)", javax.servlet.http;version="[2.5.0,3.2.0)", org.apache.commons.logging;version="[1.1.1,2.0.0)", - org.eclipse.jetty.http;version="[7.6.0,8.0.0)", - org.eclipse.jetty.security;version="[7.6.0,8.0.0)", - org.eclipse.jetty.security.authentication;version="[7.6.0,8.0.0)", - org.eclipse.jetty.server;version="[7.6.0,8.0.0)", - org.eclipse.jetty.server.handler;version="[7.6.0,8.0.0)", - org.eclipse.jetty.server.nio;version="[7.6.0,8.0.0)", - org.eclipse.jetty.servlet;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.component;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.log;version="[7.6.0,8.0.0)", - org.eclipse.jetty.util.security;version="[7.6.0,8.0.0)", + org.eclipse.jetty.http;version="[9.0.0,10.0.0)", + org.eclipse.jetty.security;version="[9.0.0,10.0.0)", + org.eclipse.jetty.security.authentication;version="[9.0.0,10.0.0)", + org.eclipse.jetty.server;version="[9.0.0,10.0.0)", + org.eclipse.jetty.server.handler;version="[9.0.0,10.0.0)", + org.eclipse.jetty.server.nio;version="[9.0.0,10.0.0)", + org.eclipse.jetty.servlet;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.component;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.log;version="[9.0.0,10.0.0)", + org.eclipse.jetty.util.security;version="[9.0.0,10.0.0)", org.eclipse.jgit.errors;version="[4.0.0,4.1.0)", org.eclipse.jgit.http.server;version="[4.0.0,4.1.0)", org.eclipse.jgit.internal.storage.file;version="[4.0.0,4.1.0)", diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java index 6b0e0603e..ce04bdf2c 100644 --- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java +++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java @@ -60,10 +60,12 @@ import org.eclipse.jetty.security.MappedLoginService; import org.eclipse.jetty.security.authentication.BasicAuthenticator; import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.server.handler.ContextHandlerCollection; -import org.eclipse.jetty.server.nio.SelectChannelConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Password; @@ -95,14 +97,22 @@ public class AppServer { private final Server server; - private final Connector connector; + private final ServerConnector connector; private final ContextHandlerCollection contexts; private final TestRequestLog log; public AppServer() { - connector = new SelectChannelConnector(); + server = new Server(); + + HttpConfiguration http_config = new HttpConfiguration(); + http_config.setSecureScheme("https"); + http_config.setSecurePort(8443); + http_config.setOutputBufferSize(32768); + + connector = new ServerConnector(server, + new HttpConnectionFactory(http_config)); connector.setPort(0); try { final InetAddress me = InetAddress.getByName("localhost"); @@ -116,7 +126,6 @@ public AppServer() { log = new TestRequestLog(); log.setHandler(contexts); - server = new Server(); server.setConnectors(new Connector[] { connector }); server.setHandler(log); } @@ -173,7 +182,6 @@ protected void loadUsers() throws IOException { cm.setPathSpec("/*"); ConstraintSecurityHandler sec = new ConstraintSecurityHandler(); - sec.setStrict(false); sec.setRealmName(realm); sec.setAuthenticator(authType); sec.setLoginService(users); @@ -232,7 +240,7 @@ public URI getURI() { /** @return the local port number the server is listening on. */ public int getPort() { assertAlreadySetUp(); - return ((SelectChannelConnector) connector).getLocalPort(); + return connector.getLocalPort(); } /** @return all requests since the server was started. */ diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/RecordingLogger.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/RecordingLogger.java index 0accfc8b6..7600843d2 100644 --- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/RecordingLogger.java +++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/RecordingLogger.java @@ -187,4 +187,9 @@ public void debug(Throwable thrown) { public void ignore(Throwable arg0) { // Ignore (not relevant to test failures) } + + @Override + public void debug(String msg, long value) { + // Ignore (not relevant to test failures) + } } diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java index f71bc9350..14ea03a92 100644 --- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java +++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java @@ -48,11 +48,11 @@ import java.util.List; import java.util.concurrent.Semaphore; +import javax.servlet.DispatcherType; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.server.DispatcherType; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.handler.HandlerWrapper; diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target index 525e56981..b82b50cc5 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.target @@ -1,26 +1,26 @@ - + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.tpd index 1d16481cd..f038aa0d4 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.3.tpd @@ -1,6 +1,6 @@ target "jgit-4.3" with source configurePhase -include "projects/jetty-7.6.14.tpd" +include "projects/jetty-9.2.10.tpd" include "orbit/R20150124073747-Luna-SR2.tpd" location "http://download.eclipse.org/releases/kepler/" { diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target index 083ece881..f012800ac 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.target @@ -1,26 +1,26 @@ - + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.tpd index afbe9c9f7..0760bcdff 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.4.tpd @@ -1,6 +1,6 @@ target "jgit-4.4" with source configurePhase -include "projects/jetty-7.6.14.tpd" +include "projects/jetty-9.2.10.tpd" include "orbit/R20150124073747-Luna-SR2.tpd" location "http://download.eclipse.org/releases/luna/" { diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target index 42adbb3d9..475fa04cd 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.target @@ -1,26 +1,26 @@ - + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd index 33b088a68..71c963479 100644 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/jgit-4.5.tpd @@ -1,6 +1,6 @@ target "jgit-4.5" with source configurePhase -include "projects/jetty-7.6.14.tpd" +include "projects/jetty-9.2.10.tpd" include "orbit/S20150202203538-Mars-M5.tpd" location "http://download.eclipse.org/releases/mars/" { diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-7.6.14.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-7.6.14.tpd deleted file mode 100644 index 2e338d637..000000000 --- a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-7.6.14.tpd +++ /dev/null @@ -1,20 +0,0 @@ -target "jetty-7.6.14" with source configurePhase - -location jetty-7.6.14 "http://download.eclipse.org/jetty/updates/jetty-bundles-7.x/7.6.14.v20131031/" { - org.eclipse.jetty.client [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.client.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.continuation [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.continuation.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.http [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.http.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.io [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.io.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.security [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.security.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.server [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.server.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.servlet [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.servlet.source [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.util [7.6.14.v20131031,7.6.14.v20131031] - org.eclipse.jetty.util.source [7.6.14.v20131031,7.6.14.v20131031] -} diff --git a/org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-9.2.10.tpd b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-9.2.10.tpd new file mode 100644 index 000000000..5a7ee2e2f --- /dev/null +++ b/org.eclipse.jgit.packaging/org.eclipse.jgit.target/projects/jetty-9.2.10.tpd @@ -0,0 +1,20 @@ +target "jetty-9.2.10" with source configurePhase + +location jetty-9.2.10 "http://download.eclipse.org/jetty/updates/jetty-bundles-9.x/9.2.10.v20150310/" { + org.eclipse.jetty.client [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.client.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.continuation [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.continuation.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.http [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.http.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.io [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.io.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.security [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.security.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.server [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.server.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.servlet [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.servlet.source [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.util [9.2.10.v20150310,9.2.10.v20150310] + org.eclipse.jetty.util.source [9.2.10.v20150310,9.2.10.v20150310] +} diff --git a/pom.xml b/pom.xml index 1e00f2457..f02fa296a 100644 --- a/pom.xml +++ b/pom.xml @@ -185,7 +185,7 @@ 1.6 4.3.1 2.5 - 7.6.14.v20131031 + 9.2.10.v20150310 2.6.1 4.1.3 1.7.2 From 8ff08455f62f5696b718ff558507cce3709d604b Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 9 May 2015 18:11:20 -0700 Subject: [PATCH 20/26] Fix memory leak in dfs.DeltaBaseCase The LRU chain management code was broken leading to situations where the chain was incomplete. This prevented the cache from removing items when it exceeded its memory target, causing a leak. One case was repeated hit on the head of the chain. moveToHead(e) was invoked linking the head back to itself in a cycle orphaning the rest of the table. Add some unit tests to cover this and a few other paths. Change-Id: Ib27486eaa1b1d2bf1c745a56d0a5832bfb029322 --- .../storage/dfs/DeltaBaseCacheTest.java | 152 ++++++++++++++++++ .../internal/storage/dfs/DeltaBaseCache.java | 91 +++++++---- 2 files changed, 216 insertions(+), 27 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCacheTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCacheTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCacheTest.java new file mode 100644 index 000000000..5bef9fa29 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCacheTest.java @@ -0,0 +1,152 @@ +/* + * Copyright (C) 2015, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.internal.storage.dfs; + +import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +import org.eclipse.jgit.internal.storage.dfs.DeltaBaseCache.Entry; +import org.eclipse.jgit.junit.TestRng; +import org.junit.Before; +import org.junit.Test; + +public class DeltaBaseCacheTest { + private static final int SZ = 512; + + private DfsPackKey key; + private DeltaBaseCache cache; + private TestRng rng; + + @Before + public void setUp() { + key = new DfsPackKey(); + cache = new DeltaBaseCache(SZ); + rng = new TestRng(getClass().getSimpleName()); + } + + @Test + public void testObjectLargerThanCacheDoesNotEvict() { + byte[] obj12 = put(12, 32); + put(24, SZ + 5); + assertNull("does not store large object", cache.get(key, 24)); + get(obj12, 12); + } + + @Test + public void testCacheLruExpires1() { + byte[] obj1 = put(1, SZ / 4); + put(2, SZ / 4); + byte[] obj3 = put(3, SZ / 4); + put(4, SZ / 4); + assertEquals(SZ, cache.getMemoryUsed()); + + get(obj3, 3); + get(obj1, 1); + put(5, SZ / 2); + assertEquals(SZ, cache.getMemoryUsed()); + assertEquals(SZ, cache.getMemoryUsedByTableForTest()); + assertEquals(SZ, cache.getMemoryUsedByLruChainForTest()); + assertNull(cache.get(key, 4)); + assertNull(cache.get(key, 2)); + + get(obj1, 1); + get(obj3, 3); + } + + @Test + public void testCacheLruExpires2() { + int pos0 = (0 << 10) | 2; + int pos1 = (1 << 10) | 2; + int pos2 = (2 << 10) | 2; + int pos5 = (5 << 10) | 2; + int pos6 = (6 << 10) | 2; + + put(pos0, SZ / 4); + put(pos5, SZ / 4); + byte[] obj1 = put(pos1, SZ / 4); + byte[] obj2 = put(pos2, SZ / 4); + assertEquals(SZ, cache.getMemoryUsed()); + + byte[] obj6 = put(pos6, SZ / 2); + assertEquals(SZ, cache.getMemoryUsed()); + assertEquals(SZ, cache.getMemoryUsedByTableForTest()); + assertEquals(SZ, cache.getMemoryUsedByLruChainForTest()); + assertNull(cache.get(key, pos0)); + assertNull(cache.get(key, pos5)); + + get(obj1, pos1); + get(obj2, pos2); + get(obj6, pos6); + } + + @Test + public void testCacheMemoryUsedConsistentWithExpectations() { + put(1, 32); + put(2, 32); + put(3, 32); + + assertNotNull(cache.get(key, 1)); + assertNotNull(cache.get(key, 1)); + + assertEquals(32 * 3, cache.getMemoryUsed()); + assertEquals(32 * 3, cache.getMemoryUsedByTableForTest()); + assertEquals(32 * 3, cache.getMemoryUsedByLruChainForTest()); + } + + private void get(byte[] data, int position) { + Entry e = cache.get(key, position); + assertNotNull("expected entry at " + position, e); + assertEquals("expected blob for " + position, OBJ_BLOB, e.type); + assertSame("expected data for " + position, data, e.data); + } + + private byte[] put(int position, int sz) { + byte[] data = rng.nextBytes(sz); + cache.put(key, position, OBJ_BLOB, data); + return data; + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java index 53c05f013..c7bdbff8a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java @@ -70,8 +70,11 @@ private static int hash(long position) { private int curByteCount; DeltaBaseCache(DfsReader reader) { - DfsReaderOptions options = reader.getOptions(); - maxByteCount = options.getDeltaBaseCacheLimit(); + this(reader.getOptions().getDeltaBaseCacheLimit()); + } + + DeltaBaseCache(int maxBytes) { + maxByteCount = maxBytes; table = new Slot[1 << TABLE_BITS]; } @@ -84,6 +87,7 @@ Entry get(DfsPackKey key, long position) { moveToHead(e); return buf; } + return null; } } return null; @@ -101,23 +105,15 @@ void put(DfsPackKey key, long offset, int objectType, byte[] data) { e.data = new SoftReference(new Entry(data, objectType)); e.tableNext = table[tableIdx]; table[tableIdx] = e; - moveToHead(e); + lruPushHead(e); } private void releaseMemory() { while (curByteCount > maxByteCount && lruTail != null) { - Slot currOldest = lruTail; - Slot nextOldest = currOldest.lruPrev; - - curByteCount -= currOldest.size; - unlink(currOldest); - removeFromTable(currOldest); - - if (nextOldest == null) - lruHead = null; - else - nextOldest.lruNext = null; - lruTail = nextOldest; + Slot e = lruTail; + curByteCount -= e.size; + lruRemove(e); + removeFromTable(e); } } @@ -136,27 +132,68 @@ private void removeFromTable(Slot e) { return; } } + + throw new IllegalStateException(String.format( + "entry for %s:%d not in table", //$NON-NLS-1$ + e.pack, Long.valueOf(e.offset))); } private void moveToHead(final Slot e) { - unlink(e); - e.lruPrev = null; - e.lruNext = lruHead; - if (lruHead != null) - lruHead.lruPrev = e; + if (e != lruHead) { + lruRemove(e); + lruPushHead(e); + } + } + + private void lruRemove(final Slot e) { + Slot p = e.lruPrev; + Slot n = e.lruNext; + + if (p != null) { + p.lruNext = n; + } else { + lruHead = n; + } + + if (n != null) { + n.lruPrev = p; + } else { + lruTail = p; + } + } + + private void lruPushHead(final Slot e) { + Slot n = lruHead; + e.lruNext = n; + if (n != null) + n.lruPrev = e; else lruTail = e; + + e.lruPrev = null; lruHead = e; } - private void unlink(final Slot e) { - Slot prev = e.lruPrev; - Slot next = e.lruNext; + int getMemoryUsed() { + return curByteCount; + } - if (prev != null) - prev.lruNext = next; - if (next != null) - next.lruPrev = prev; + int getMemoryUsedByLruChainForTest() { + int r = 0; + for (Slot e = lruHead; e != null; e = e.lruNext) { + r += e.size; + } + return r; + } + + int getMemoryUsedByTableForTest() { + int r = 0; + for (int i = 0; i < table.length; i++) { + for (Slot e = table[i]; e != null; e = e.tableNext) { + r += e.size; + } + } + return r; } static class Entry { From 2eb16aa6ca5415ae4e741c4d1056890e07fa27e5 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 9 May 2015 19:53:53 -0700 Subject: [PATCH 21/26] Remove SoftReference from dfs.DeltaBaseCache The Java GC doesn't always clear these before running out of memory and failing allocations. In practice OpenJDK 7 is leaving these live, removing any advantage of the SoftReference to attempt to shed memory when the GC is unable to continue allocating. Instead follow the pattern of the DfsBlockCache and use hard refs to the object data. Require applications to configure the cache size more accurately given expected memory usage. Change-Id: I87586b3e71b1cba0308a6a278d42e971be4bccd3 --- .../internal/storage/dfs/DeltaBaseCache.java | 89 +++++++------------ 1 file changed, 33 insertions(+), 56 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java index c7bdbff8a..64a63d7c7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DeltaBaseCache.java @@ -43,7 +43,6 @@ package org.eclipse.jgit.internal.storage.dfs; -import java.lang.ref.SoftReference; /** * Caches recently used objects for {@link DfsReader}. @@ -60,34 +59,28 @@ private static int hash(long position) { } private int maxByteCount; - - private final Slot[] table; - - private Slot lruHead; - - private Slot lruTail; - private int curByteCount; + private final Entry[] table; + + private Entry lruHead; + private Entry lruTail; + DeltaBaseCache(DfsReader reader) { this(reader.getOptions().getDeltaBaseCacheLimit()); } DeltaBaseCache(int maxBytes) { maxByteCount = maxBytes; - table = new Slot[1 << TABLE_BITS]; + table = new Entry[1 << TABLE_BITS]; } Entry get(DfsPackKey key, long position) { - Slot e = table[hash(position)]; + Entry e = table[hash(position)]; for (; e != null; e = e.tableNext) { if (e.offset == position && key.equals(e.pack)) { - Entry buf = e.data.get(); - if (buf != null) { - moveToHead(e); - return buf; - } - return null; + moveToHead(e); + return e; } } return null; @@ -101,8 +94,7 @@ void put(DfsPackKey key, long offset, int objectType, byte[] data) { releaseMemory(); int tableIdx = hash(offset); - Slot e = new Slot(key, offset, data.length); - e.data = new SoftReference(new Entry(data, objectType)); + Entry e = new Entry(key, offset, objectType, data); e.tableNext = table[tableIdx]; table[tableIdx] = e; lruPushHead(e); @@ -110,16 +102,16 @@ void put(DfsPackKey key, long offset, int objectType, byte[] data) { private void releaseMemory() { while (curByteCount > maxByteCount && lruTail != null) { - Slot e = lruTail; - curByteCount -= e.size; + Entry e = lruTail; + curByteCount -= e.data.length; lruRemove(e); removeFromTable(e); } } - private void removeFromTable(Slot e) { + private void removeFromTable(Entry e) { int tableIdx = hash(e.offset); - Slot p = table[tableIdx]; + Entry p = table[tableIdx]; if (p == e) { table[tableIdx] = e.tableNext; @@ -138,16 +130,16 @@ private void removeFromTable(Slot e) { e.pack, Long.valueOf(e.offset))); } - private void moveToHead(final Slot e) { + private void moveToHead(Entry e) { if (e != lruHead) { lruRemove(e); lruPushHead(e); } } - private void lruRemove(final Slot e) { - Slot p = e.lruPrev; - Slot n = e.lruNext; + private void lruRemove(Entry e) { + Entry p = e.lruPrev; + Entry n = e.lruNext; if (p != null) { p.lruNext = n; @@ -162,8 +154,8 @@ private void lruRemove(final Slot e) { } } - private void lruPushHead(final Slot e) { - Slot n = lruHead; + private void lruPushHead(Entry e) { + Entry n = lruHead; e.lruNext = n; if (n != null) n.lruPrev = e; @@ -180,8 +172,8 @@ int getMemoryUsed() { int getMemoryUsedByLruChainForTest() { int r = 0; - for (Slot e = lruHead; e != null; e = e.lruNext) { - r += e.size; + for (Entry e = lruHead; e != null; e = e.lruNext) { + r += e.data.length; } return r; } @@ -189,43 +181,28 @@ int getMemoryUsedByLruChainForTest() { int getMemoryUsedByTableForTest() { int r = 0; for (int i = 0; i < table.length; i++) { - for (Slot e = table[i]; e != null; e = e.tableNext) { - r += e.size; + for (Entry e = table[i]; e != null; e = e.tableNext) { + r += e.data.length; } } return r; } static class Entry { + final DfsPackKey pack; + final long offset; + final int type; final byte[] data; - final int type; + Entry tableNext; + Entry lruPrev; + Entry lruNext; - Entry(final byte[] aData, final int aType) { - data = aData; - type = aType; - } - } - - private static class Slot { - final DfsPackKey pack; - - final long offset; - - final int size; - - Slot tableNext; - - Slot lruPrev; - - Slot lruNext; - - SoftReference data; - - Slot(DfsPackKey key, long offset, int size) { + Entry(DfsPackKey key, long offset, int type, byte[] data) { this.pack = key; this.offset = offset; - this.size = size; + this.type = type; + this.data = data; } } } From f2be5bca043f3e8ddfbc8b03fc17ff4a65896045 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 14 May 2015 16:24:15 -0700 Subject: [PATCH 22/26] Allow ObjectWalk to be filtered by an arbitrary predicate This will make it possible to declare a collection of objects as ineligible for the walk en masse, for example if they are known to be uninteresting via a bitmap. Change-Id: I637008b25bf9fb57df60ebb2133a70214930546a Signed-off-by: Jonathan Nieder --- .../org/eclipse/jgit/revwalk/ObjectWalk.java | 52 ++++++++++- .../jgit/revwalk/filter/ObjectFilter.java | 87 +++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java index 18c4233eb..89a06b141 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java @@ -60,6 +60,7 @@ import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.filter.ObjectFilter; import org.eclipse.jgit.util.RawParseUtils; /** @@ -99,6 +100,8 @@ public class ObjectWalk extends RevWalk { private BlockObjQueue pendingObjects; + private ObjectFilter objectFilter; + private TreeVisit freeVisit; private TreeVisit currVisit; @@ -132,6 +135,7 @@ public ObjectWalk(ObjectReader or) { setRetainBody(false); rootObjects = new ArrayList(); pendingObjects = new BlockObjQueue(); + objectFilter = ObjectFilter.ALL; pathBuf = new byte[256]; } @@ -250,6 +254,38 @@ public void sort(RevSort s, boolean use) { boundary = hasRevSort(RevSort.BOUNDARY); } + /** + * Get the currently configured object filter. + * + * @return the current filter. Never null as a filter is always needed. + * + * @since 4.1 + */ + public ObjectFilter getObjectFilter() { + return objectFilter; + } + + /** + * Set the object filter for this walker. This filter affects the objects + * visited by {@link #nextObject()}. It does not affect the commits + * listed by {@link #next()}. + *

    + * If the filter returns false for an object, then that object is skipped + * and objects reachable from it are not enqueued to be walked recursively. + * This can be used to speed up the object walk by skipping subtrees that + * are known to be uninteresting. + * + * @param newFilter + * the new filter. If null the special {@link ObjectFilter#ALL} + * filter will be used instead, which as it matches every object. + * + * @since 4.1 + */ + public void setObjectFilter(ObjectFilter newFilter) { + assertNotStarted(); + objectFilter = newFilter != null ? newFilter : ObjectFilter.ALL; + } + @Override public RevCommit next() throws MissingObjectException, IncorrectObjectTypeException, IOException { @@ -258,13 +294,19 @@ public RevCommit next() throws MissingObjectException, if (r == null) { return null; } + final RevTree t = r.getTree(); if ((r.flags & UNINTERESTING) != 0) { - markTreeUninteresting(r.getTree()); - if (boundary) + if (objectFilter.include(this, t)) { + markTreeUninteresting(t); + } + if (boundary) { return r; + } continue; } - pendingObjects.add(r.getTree()); + if (objectFilter.include(this, t)) { + pendingObjects.add(t); + } return r; } } @@ -296,6 +338,10 @@ public RevObject nextObject() throws MissingObjectException, idBuffer.fromRaw(buf, ptr); ptr += ID_SZ; + if (!objectFilter.include(this, idBuffer)) { + continue; + } + RevObject obj = objects.get(idBuffer); if (obj != null && (obj.flags & SEEN) != 0) continue; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java new file mode 100644 index 000000000..a1d66c8e3 --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java @@ -0,0 +1,87 @@ +/** + * Copyright (C) 2015, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.revwalk.filter; + +import java.io.IOException; + +import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.errors.StopWalkException; +import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.revwalk.ObjectWalk; + +/** + * Selects interesting objects when walking. + *

    + * Applications should install the filter on an ObjectWalk by + * {@link ObjectWalk#setObjectFilter(ObjectFilter)} prior to starting traversal. + * + * @since 4.1 + */ +public abstract class ObjectFilter { + /** Default filter that always returns true. */ + public static final ObjectFilter ALL = new AllFilter(); + + private static final class AllFilter extends ObjectFilter { + @Override + public boolean include(ObjectWalk walker, AnyObjectId o) { + return true; + } + } + + /** + * Determine if the named object should be included in the walk. + * + * @param walker + * the active walker this filter is being invoked from within. + * @param objid + * the object currently being tested. + * @throws IOException + * an object the filter needed to consult to determine its answer + * was missing, of the wrong type, or could not be read. + */ + public abstract boolean include(ObjectWalk walker, AnyObjectId objid) + throws MissingObjectException, IncorrectObjectTypeException, + IOException; +} From f91f4e5ed60cde2e1b71c0faea5dcbec7e8b4560 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 15 May 2015 11:23:55 -0700 Subject: [PATCH 23/26] Fix typo in ObjectWalk#getObjectFilter javadoc While trying to decide between "which matches every object" and "as it matches every object", I became distracted and wrote both. Change-Id: I867ce29664e661a81a9d441e59ffd0b72270dd98 Signed-off-by: Jonathan Nieder --- org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java index 89a06b141..10968ee83 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java @@ -277,7 +277,7 @@ public ObjectFilter getObjectFilter() { * * @param newFilter * the new filter. If null the special {@link ObjectFilter#ALL} - * filter will be used instead, which as it matches every object. + * filter will be used instead, as it matches every object. * * @since 4.1 */ From b35b09d09aa0346f1b80f722076c6fd57b90f1d8 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 15 May 2015 11:37:57 -0700 Subject: [PATCH 24/26] Correct @since tags for ObjectFilter Although the stable-4.0 branch already exists, 4.0 development is still happening on master until IP logs are sent for review, which will happen at the end of May. Change-Id: I863ba85c6303f8ef2eb13bca5e2d30e5d3c58b29 Signed-off-by: Jonathan Nieder --- org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java | 4 ++-- .../src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java index 10968ee83..9c3af5a5d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java @@ -259,7 +259,7 @@ public void sort(RevSort s, boolean use) { * * @return the current filter. Never null as a filter is always needed. * - * @since 4.1 + * @since 4.0 */ public ObjectFilter getObjectFilter() { return objectFilter; @@ -279,7 +279,7 @@ public ObjectFilter getObjectFilter() { * the new filter. If null the special {@link ObjectFilter#ALL} * filter will be used instead, as it matches every object. * - * @since 4.1 + * @since 4.0 */ public void setObjectFilter(ObjectFilter newFilter) { assertNotStarted(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java index a1d66c8e3..44ebb9c60 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java @@ -57,7 +57,7 @@ * Applications should install the filter on an ObjectWalk by * {@link ObjectWalk#setObjectFilter(ObjectFilter)} prior to starting traversal. * - * @since 4.1 + * @since 4.0 */ public abstract class ObjectFilter { /** Default filter that always returns true. */ From 0243da320ed6260c61eac07dadc1a7c39904ad1c Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 15 May 2015 15:54:38 +0200 Subject: [PATCH 25/26] Fix typo in reflog message written by RebaseCommand.tryFastForward() Change-Id: I1ad544f2b5673ed3b4a2206b5eb4ce20fd3c86d2 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java index 62001d0a7..82444ba60 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java @@ -1227,7 +1227,7 @@ private RevCommit tryFastForward(String headName, RevCommit oldCommit, RefUpdate rup = repo.updateRef(headName); rup.setExpectedOldObjectId(oldCommit); rup.setNewObjectId(newCommit); - rup.setRefLogMessage("Fast-foward from " + oldCommit.name() //$NON-NLS-1$ + rup.setRefLogMessage("Fast-forward from " + oldCommit.name() //$NON-NLS-1$ + " to " + newCommit.name(), false); //$NON-NLS-1$ Result res = rup.update(walk); switch (res) { From 4cbe66af47a093fb503053ac3aa6e9da01dd3d1b Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 15 May 2015 23:20:46 +0200 Subject: [PATCH 26/26] Fix warnings in ObjectFilter - add missing tags in JavaDoc of ObjectFilter.include() - remove unnecessary import Change-Id: I24b9dcc49f66380f77345d704df70c05f7f74db8 Signed-off-by: Matthias Sohn --- .../jgit/revwalk/filter/ObjectFilter.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java index 44ebb9c60..2ad273d9e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/filter/ObjectFilter.java @@ -47,7 +47,6 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; -import org.eclipse.jgit.errors.StopWalkException; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.revwalk.ObjectWalk; @@ -74,12 +73,19 @@ public boolean include(ObjectWalk walker, AnyObjectId o) { * Determine if the named object should be included in the walk. * * @param walker - * the active walker this filter is being invoked from within. + * the active walker this filter is being invoked from within. * @param objid - * the object currently being tested. + * the object currently being tested. + * @return {@code true} if the named object should be included in the walk. + * @throws MissingObjectException + * an object the filter needed to consult to determine its + * answer was missing + * @throws IncorrectObjectTypeException + * an object the filter needed to consult to determine its + * answer was of the wrong type * @throws IOException - * an object the filter needed to consult to determine its answer - * was missing, of the wrong type, or could not be read. + * an object the filter needed to consult to determine its + * answer could not be read. */ public abstract boolean include(ObjectWalk walker, AnyObjectId objid) throws MissingObjectException, IncorrectObjectTypeException,