From dac6801b47ef7924d6b265800816da1b571e6d70 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 5 Jun 2020 11:34:49 +0900 Subject: [PATCH] ObjectDirectory: Further clean up insertUnpackedObject - The code to move the file is repeated. Split it out into a utility method. - Remove the catch block for AtomicMoveNotSupportedException which is redundant because it's handled in exactly the same way as the IOException further down. The only exception we need to explicitly handle differently in this block is NoSuchFileException. - Improve the comments. Change-Id: Ifc5490953ffb25ecd1c48a06289eccb3f19910c6 Signed-off-by: David Pursehouse --- .../storage/file/ObjectDirectory.java | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) 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 33f7853ab..265b71dd2 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 @@ -19,7 +19,6 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.StandardCopyOption; @@ -686,42 +685,48 @@ public class ObjectDirectory extends FileObjectDatabase { FileUtils.delete(tmp, FileUtils.RETRY); return InsertLooseObjectResult.EXISTS_LOOSE; } + try { - Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst), - StandardCopyOption.ATOMIC_MOVE); - dst.setReadOnly(); - unpackedObjectCache.add(id); - return InsertLooseObjectResult.INSERTED; - } catch (AtomicMoveNotSupportedException e) { - LOG.error(e.getMessage(), e); - FileUtils.delete(tmp, FileUtils.RETRY); - return InsertLooseObjectResult.FAILURE; + return tryMove(tmp, dst, id); } catch (NoSuchFileException e) { // It's possible the directory doesn't exist yet as the object // directories are always lazily created. Note that we try the // rename/move first as the directory likely does exist. - - // Create the directory + // + // Create the directory. + // FileUtils.mkdir(dst.getParentFile(), true); } catch (IOException e) { + // Any other IO error is considered a failure. + // LOG.error(e.getMessage(), e); FileUtils.delete(tmp, FileUtils.RETRY); return InsertLooseObjectResult.FAILURE; } try { - Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst), - StandardCopyOption.ATOMIC_MOVE); - dst.setReadOnly(); - unpackedObjectCache.add(id); - return InsertLooseObjectResult.INSERTED; + return tryMove(tmp, dst, id); } catch (IOException e) { + // The object failed to be renamed into its proper location and + // it doesn't exist in the repository either. We really don't + // know what went wrong, so fail. + // LOG.error(e.getMessage(), e); FileUtils.delete(tmp, FileUtils.RETRY); return InsertLooseObjectResult.FAILURE; } } + private InsertLooseObjectResult tryMove(File tmp, File dst, + ObjectId id) + throws IOException { + Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst), + StandardCopyOption.ATOMIC_MOVE); + dst.setReadOnly(); + unpackedObjectCache.add(id); + return InsertLooseObjectResult.INSERTED; + } + boolean searchPacksAgain(PackList old) { // Whether to trust the pack folder's modification time. If set // to false we will always scan the .git/objects/pack folder to