From ec5fc57b791081fa073fc5fd91286347238f8f7c Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sun, 3 Mar 2019 13:14:46 +0900 Subject: [PATCH 1/6] Bazel: Stop using native.git_repository The native.git_repository method doesn't work in the latest version of bazel, and causes the build to fail with: type 'struct' has no method git_repository() Change-Id: Id6a57369b681c0afe811e9e3740b141fb7fb4653 Signed-off-by: David Pursehouse --- tools/bazlets.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/bazlets.bzl b/tools/bazlets.bzl index f97b72c82..f089af473 100644 --- a/tools/bazlets.bzl +++ b/tools/bazlets.bzl @@ -1,10 +1,12 @@ +load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") + NAME = "com_googlesource_gerrit_bazlets" def load_bazlets( commit, local_path = None): if not local_path: - native.git_repository( + git_repository( name = NAME, remote = "https://gerrit.googlesource.com/bazlets", commit = commit, From 9665d86ba1dd2937ca26f6aba63bb16aa277f888 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Mon, 25 Feb 2019 08:35:12 +0900 Subject: [PATCH 2/6] ObjectDirectory: Clean up logging Externalize the message and log the pack file with absolute path. Change-Id: I019052dfae8fd96ab67da08b3287d699287004cb Signed-off-by: David Pursehouse --- .../org/eclipse/jgit/internal/JGitText.properties | 1 + .../src/org/eclipse/jgit/internal/JGitText.java | 1 + .../jgit/internal/storage/file/ObjectDirectory.java | 7 +++++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index b0c952cd4..8ecc5fd69 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -712,6 +712,7 @@ truncatedHunkOldLinesMissing=Truncated hunk, at least {0} old lines is missing tSizeMustBeGreaterOrEqual1=tSize must be >= 1 unableToCheckConnectivity=Unable to check connectivity. unableToCreateNewObject=Unable to create new object: {0} +unableToReadPackfile=Unable to read packfile {0} unableToRemovePath=Unable to remove path ''{0}'' unableToStore=Unable to store {0}. unableToWrite=Unable to write {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 6e99ca739..da0ba4a77 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -773,6 +773,7 @@ public static JGitText get() { /***/ public String tSizeMustBeGreaterOrEqual1; /***/ public String unableToCheckConnectivity; /***/ public String unableToCreateNewObject; + /***/ public String unableToReadPackfile; /***/ public String unableToRemovePath; /***/ public String unableToStore; /***/ public String unableToWrite; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index a6aaac7de..d8441b899 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -360,7 +360,9 @@ boolean hasPackedObject(AnyObjectId objectId) { // The hasObject call should have only touched the index, // so any failure here indicates the index is unreadable // by this process, and the pack is likewise not readable. - LOG.warn("Unable to read packfile " + p.getPackFile(), e); + LOG.warn(MessageFormat.format( + JGitText.get().unableToReadPackfile, + p.getPackFile().getAbsolutePath()), e); removePack(p); } } @@ -653,7 +655,8 @@ private void handlePackError(IOException e, PackFile p) { if ((e instanceof CorruptObjectException) || (e instanceof PackInvalidException)) { warnTmpl = JGitText.get().corruptPack; - LOG.warn("Packfile " + p.getPackFile() + " is corrupted", e); + LOG.warn(MessageFormat.format(warnTmpl, + p.getPackFile().getAbsolutePath()), e); // Assume the pack is corrupted, and remove it from the list. removePack(p); } else if (e instanceof FileNotFoundException) { From 436c99ce5946f31f06b8704b1fd33136f39dc814 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Sat, 23 Feb 2019 21:57:09 +0000 Subject: [PATCH 3/6] PackFile: report correct message for checksum mismatch When the packfile checksum does not match the expected one report the correct checksum error instead of reporting that the number of objects is incorrect. Change-Id: I040f36dacc4152ae05453e7acbf8dfccceb46e0d Signed-off-by: Luca Milanesio Signed-off-by: Matthias Sohn --- .../org/eclipse/jgit/internal/JGitText.properties | 2 +- .../eclipse/jgit/internal/storage/file/PackFile.java | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 8ecc5fd69..c535bf50f 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -509,7 +509,7 @@ openingConnection=Opening connection operationCanceled=Operation {0} was canceled outputHasAlreadyBeenStarted=Output has already been started. overflowedReftableBlock=Overflowed reftable block -packChecksumMismatch=Pack checksum mismatch detected for pack file {0} +packChecksumMismatch=Pack checksum mismatch detected for pack file {0}: .pack has {1} whilst .idx has {2} packCorruptedWhileWritingToFilesystem=Pack corrupted while writing to filesystem packDoesNotMatchIndex=Pack {0} does not match index packedRefsHandleIsStale=packed-refs handle is stale, {0}. retry diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index 71f64ae11..e85d85cbd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -187,7 +187,8 @@ private synchronized PackIndex idx() throws IOException { } else if (!Arrays.equals(packChecksum, idx.packChecksum)) { throw new PackMismatchException(MessageFormat.format( JGitText.get().packChecksumMismatch, - packFile.getPath())); + packFile.getPath(), packChecksum, + idx.packChecksum)); } loadedIdx = idx; } catch (InterruptedIOException e) { @@ -753,10 +754,10 @@ private void onOpenPack() throws IOException { fd.readFully(buf, 0, 20); if (!Arrays.equals(buf, packChecksum)) { throw new PackMismatchException(MessageFormat.format( - JGitText.get().packObjectCountMismatch - , ObjectId.fromRaw(buf).name() - , ObjectId.fromRaw(idx.packChecksum).name() - , getPackFile())); + JGitText.get().packChecksumMismatch, + getPackFile(), + ObjectId.fromRaw(buf).name(), + ObjectId.fromRaw(idx.packChecksum).name())); } } From 882fed0d96c533513c43ae77aaff9cc07b94012c Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 7 Jul 2017 00:58:28 +0200 Subject: [PATCH 4/6] Cancel gc if thread was interrupted see https://groups.google.com/d/msg/repo-discuss/oDB2rl3doDc/tFEh5Xt0CAAJ Change-Id: Ia6d4631c64e065d8b9b09e0b45e7a9ea8ac3f41d Signed-off-by: Matthias Sohn --- .../storage/file/GcConcurrentTest.java | 52 +++++++++++++++++++ .../jgit/internal/storage/file/GC.java | 2 +- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java index 643bb4946..c60c357da 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcConcurrentTest.java @@ -47,27 +47,35 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import org.eclipse.jgit.errors.CancelledException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.pack.PackWriter; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.EmptyProgressMonitor; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Sets; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; import org.junit.Test; public class GcConcurrentTest extends GcTestCase { @@ -221,4 +229,48 @@ public void repackAndCheckBitmapUsage() throws Exception { assertEquals(getSinglePack(repository).getPackName(), newPackName); assertNotNull(getSinglePack(repository).getBitmapIndex()); } + + @Test + public void testInterruptGc() throws Exception { + FileBasedConfig c = repo.getConfig(); + c.setInt(ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_AUTOPACKLIMIT, 1); + c.save(); + SampleDataRepositoryTestCase.copyCGitTestPacks(repo); + ExecutorService executor = Executors.newSingleThreadExecutor(); + final CountDownLatch latch = new CountDownLatch(1); + Future> result = executor + .submit(new Callable>() { + + @Override + public Collection call() throws Exception { + long start = System.currentTimeMillis(); + System.out.println("starting gc"); + latch.countDown(); + Collection r = gc.gc(); + System.out.println("gc took " + + (System.currentTimeMillis() - start) + " ms"); + return r; + } + }); + try { + latch.await(); + Thread.sleep(5); + executor.shutdownNow(); + result.get(); + fail("thread wasn't interrupted"); + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + if (cause instanceof CancelledException) { + assertEquals(JGitText.get().operationCanceled, + cause.getMessage()); + } else if (cause instanceof IOException) { + Throwable cause2 = cause.getCause(); + assertTrue(cause2 instanceof InterruptedException + || cause2 instanceof ExecutionException); + } else { + fail("unexpected exception " + e); + } + } + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index 7bfec3fd9..015d44e87 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -1270,7 +1270,7 @@ private File nameFor(String name, String ext) { } private void checkCancelled() throws CancelledException { - if (pm.isCancelled()) { + if (pm.isCancelled() || Thread.currentThread().isInterrupted()) { throw new CancelledException(JGitText.get().operationCanceled); } } From a33e4dc58a87daf34072b82643aba0fd4456c165 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 4 Mar 2019 13:28:25 +0100 Subject: [PATCH 5/6] Properly format pack checksums in PackFile.idx() Change-Id: Id805850dbe9a3d633168f3056e06ddeafd86f961 Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/internal/storage/file/PackFile.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index e85d85cbd..ed238a40e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -187,8 +187,9 @@ private synchronized PackIndex idx() throws IOException { } else if (!Arrays.equals(packChecksum, idx.packChecksum)) { throw new PackMismatchException(MessageFormat.format( JGitText.get().packChecksumMismatch, - packFile.getPath(), packChecksum, - idx.packChecksum)); + packFile.getPath(), + ObjectId.fromRaw(packChecksum).name(), + ObjectId.fromRaw(idx.packChecksum).name())); } loadedIdx = idx; } catch (InterruptedIOException e) { From 997d785418d55dce5a1188fdb95e6d2b4ab0bde5 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 4 Mar 2019 13:44:24 +0100 Subject: [PATCH 6/6] Fix error log message in ObjectDirectory.handlePackError() Change-Id: I154f392ad025c4b642eb1123d375a0afaa853885 Signed-off-by: Matthias Sohn --- .../resources/org/eclipse/jgit/internal/JGitText.properties | 2 +- .../eclipse/jgit/internal/storage/file/ObjectDirectory.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index c535bf50f..b7805b19e 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -289,7 +289,7 @@ exceptionCaughtDuringExecutionOfTagCommand=Exception caught during execution of exceptionHookExecutionInterrupted=Execution of "{0}" hook interrupted. exceptionOccurredDuringAddingOfOptionToALogCommand=Exception occurred during adding of {0} as option to a Log command exceptionOccurredDuringReadingOfGIT_DIR=Exception occurred during reading of $GIT_DIR/{0}. {1} -exceptionWhileReadingPack=ERROR: Exception caught while accessing pack file {0}, the pack file might be corrupt, {1}. Caught {2} consecutive errors while trying to read this pack. +exceptionWhileReadingPack=Exception caught while accessing pack file {0}, the pack file might be corrupt. Caught {1} consecutive errors while trying to read this pack. expectedACKNAKFoundEOF=Expected ACK/NAK, found EOF expectedACKNAKGot=Expected ACK/NAK, got: {0} expectedBooleanStringValue=Expected boolean string value diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index d8441b899..5dbe5cd7d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -686,8 +686,8 @@ private void handlePackError(IOException e, PackFile p) { // Don't remove the pack from the list, as the error may be // transient. LOG.error(MessageFormat.format(errTmpl, - p.getPackFile().getAbsolutePath()), - Integer.valueOf(transientErrorCount), e); + p.getPackFile().getAbsolutePath(), + Integer.valueOf(transientErrorCount)), e); } } }