Re: [jgit-dev] JGit removes Pack from list *even* if exists on the files

[lucamilanesio <luca.milanesio@xxxxxxxxx>]

Actually the JGit wiki is broken ... can't sign the ECA and post the fix :-(

Anyway, here is the patch:

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 e7308b5..1aa71e5 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

@@ -569,7 +569,6 @@ void selectObjectRepresentation(PackWriter packer, ObjectToPack otp,

 

        private void handlePackError(IOException e, PackFile p) {

                String warnTmpl = null;

-               String errTmpl = JGitText.get().exceptionWhileReadingPack;

                if ((e instanceof CorruptObjectException)

                                || (e instanceof PackInvalidException)) {

                        warnTmpl = JGitText.get().corruptPack;

@@ -577,11 +576,11 @@ private void handlePackError(IOException e, PackFile p) {

                        removePack(p);

                } else if (e instanceof FileNotFoundException) {

                        if (p.getPackFile().exists()) {

-                               errTmpl = JGitText.get().packInaccessible;

+                               warnTmpl = JGitText.get().packInaccessible;

                        } else {

                                warnTmpl = JGitText.get().packWasDeleted;

-                               removePack(p);

                        }

+                       removePack(p);

                } else if (FileUtils.isStaleFileHandleInCausalChain(e)) {

                        warnTmpl = JGitText.get().packHandleIsStale;

                        removePack(p);

@@ -598,7 +597,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()), e);

+                                       JGitText.get().exceptionWhileReadingPack, p.getPackFile()

+                                                       .getAbsolutePath()), e);

                }

        }

On Friday, March 10, 2017 at 12:12:53 AM UTC, lucamilanesio wrote:

Hi Gerrit and JGit users,

I have found a small but serious bug in the way that JGit manages the file-system related errors when reading pack files.

Look at this broken management of the FileNotFoundException (from https://goo.gl/oidSwf)

The FileNotFoundException is typically raised in three conditions:

- file doesn't exist

- incompatible read vs. read/write open modes

- filesystem locking

- temporary lack of resources (e.g. too many open files)

... so ... *why on earth* do we always assume that Pack doesn't exist and we remove it from the in-memory list?

The "removePack(p)" should be IMHO inside the else() block whilst if FileNotFoundException happens when the pack file exists => log the error and try next time again.

Does it make sense?

Going to post a 1-liner fix tomorrow :-)

Luca.