From 30fb4808f2407cbea6c8cf46189033b0231aa423 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Tue, 18 Feb 2020 20:44:10 +0100 Subject: [PATCH 1/4] Update reftable storage repo layout The change Ic0b974fa (c217d33, "Documentation/technical/reftable: improve repo layout") defines a new repository layout, which was agreed with the git-core mailing list. It addresses the following problems: * old git clients will not recognize reftable-based repositories, and look at encompassing directories. * Poorly written tools might write directly into .git/refs/heads/BRANCH. Since we consider JGit reftable as experimental (git-core doesn't support it yet), we have no backward compatibility. If you created a repository with reftable between mid-Nov 2019 and now, you can do the following to convert: mv .git/refs .git/reftable/tables.list git config core.repositoryformatversion 1 git config extensions.refStorage reftable Change-Id: I80df35b9d22a8ab893dcbe9fbd051d924788d6a5 Signed-off-by: Han-Wen Nienhuys Signed-off-by: Matthias Sohn --- .../storage/file/FileReftableTest.java | 42 +++++-- org.eclipse.jgit/.settings/.api_filters | 24 ++++ .../storage/file/FileReftableDatabase.java | 25 ++-- .../internal/storage/file/FileRepository.java | 114 +++++++++++++----- .../org/eclipse/jgit/lib/ConfigConstants.java | 21 ++++ .../src/org/eclipse/jgit/lib/Constants.java | 6 + 6 files changed, 181 insertions(+), 51 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java index bca113fbf..2ffbc6255 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java @@ -124,11 +124,6 @@ public void testRacyReload() throws Exception { } } - @Test - public void additionalRefsAreRemoved() { - assertFalse(new File(db.getDirectory(), Constants.HEAD).exists()); - } - @Test public void testCompactFully() throws Exception { ObjectId c1 = db.resolve("master^^"); @@ -141,9 +136,16 @@ public void testCompactFully() throws Exception { } File tableDir = new File(db.getDirectory(), Constants.REFTABLE); - assertTrue(tableDir.listFiles().length > 1); + assertTrue(tableDir.listFiles().length > 2); ((FileReftableDatabase)db.getRefDatabase()).compactFully(); - assertEquals(tableDir.listFiles().length,1); + assertEquals(tableDir.listFiles().length,2); + } + + @Test + public void testOpenConvert() throws Exception { + try (FileRepository repo = new FileRepository(db.getDirectory())) { + assertTrue(repo.getRefDatabase() instanceof FileReftableDatabase); + } } @Test @@ -162,7 +164,7 @@ public void testConvert() throws Exception { @Test public void testConvertToRefdir() throws Exception { - db.convertToPackedRefs(false); + db.convertToPackedRefs(false, false); assertTrue(db.getRefDatabase() instanceof RefDirectory); Ref h = db.exactRef("HEAD"); assertTrue(h.isSymbolic()); @@ -176,6 +178,30 @@ public void testConvertToRefdir() throws Exception { assertFalse(db.getRefDatabase().hasFastTipsWithSha1()); } + @Test + public void testConvertToRefdirReflog() throws Exception { + Ref a = db.exactRef("refs/heads/a"); + String aCommit = a.getObjectId().getName(); + RefUpdate u = db.updateRef("refs/heads/master"); + u.setForceUpdate(true); + u.setNewObjectId(ObjectId.fromString(aCommit)); + u.setForceRefLog(true); + u.setRefLogMessage("apple", false); + u.update(); + + RefUpdate v = db.updateRef("refs/heads/master"); + v.setForceUpdate(true); + v.setNewObjectId(ObjectId.fromString(bCommit)); + v.setForceRefLog(true); + v.setRefLogMessage("banana", false); + v.update(); + + db.convertToPackedRefs(true, false); + List logs = db.getReflogReader("refs/heads/master").getReverseEntries(2); + assertEquals(logs.get(0).getComment(), "banana"); + assertEquals(logs.get(1).getComment(), "apple"); + } + @Test public void testBatchrefUpdate() throws Exception { ObjectId cur = db.resolve("master"); diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index f3c9b99d1..5fc26a553 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -53,6 +53,24 @@ + + + + + + + + + + + + + + + + + + @@ -61,6 +79,12 @@ + + + + + + diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java index c9ee165a3..c0dc625d9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableDatabase.java @@ -97,6 +97,11 @@ public class FileReftableDatabase extends RefDatabase { private final FileReftableStack reftableStack; + FileReftableDatabase(FileRepository repo) throws IOException { + this(repo, new File(new File(repo.getDirectory(), Constants.REFTABLE), + Constants.TABLES_LIST)); + } + FileReftableDatabase(FileRepository repo, File refstackName) throws IOException { this.fileRepository = repo; this.reftableStack = new FileReftableStack(refstackName, @@ -121,8 +126,7 @@ ReflogReader getReflogReader(String refname) throws IOException { * @return whether the given repo uses reftable for refdb storage. */ public static boolean isReftable(File repoDir) { - return new File(repoDir, "refs").isFile() //$NON-NLS-1$ - && new File(repoDir, Constants.REFTABLE).isDirectory(); + return new File(repoDir, Constants.REFTABLE).isDirectory(); } /** {@inheritDoc} */ @@ -626,8 +630,6 @@ private static Ref refForWrite(RevWalk rw, Ref r) throws IOException { /** * @param repo * the repository - * @param refstackName - * the filename for the stack * @param writeLogs * whether to write reflogs * @return a reftable based RefDB from an existing repository. @@ -635,22 +637,25 @@ private static Ref refForWrite(RevWalk rw, Ref r) throws IOException { * on IO error */ public static FileReftableDatabase convertFrom(FileRepository repo, - File refstackName, boolean writeLogs) throws IOException { + boolean writeLogs) throws IOException { FileReftableDatabase newDb = null; + File reftableList = null; try { - File reftableDir = new File(repo.getDirectory(), Constants.REFTABLE); + File reftableDir = new File(repo.getDirectory(), + Constants.REFTABLE); + reftableList = new File(reftableDir, Constants.TABLES_LIST); if (!reftableDir.isDirectory()) { reftableDir.mkdir(); } - try (FileReftableStack stack = new FileReftableStack(refstackName, + try (FileReftableStack stack = new FileReftableStack(reftableList, reftableDir, null, () -> repo.getConfig())) { stack.addReftable(rw -> writeConvertTable(repo, rw, writeLogs)); } - refstackName = null; + reftableList = null; } finally { - if (refstackName != null) { - refstackName.delete(); + if (reftableList != null) { + reftableList.delete(); } } return newDb; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java index 2f6ef5113..9929c46af 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java @@ -51,10 +51,13 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.text.MessageFormat; import java.text.ParseException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -83,6 +86,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.ReflogEntry; import org.eclipse.jgit.lib.ReflogReader; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; @@ -205,18 +209,17 @@ public FileRepository(BaseRepositoryBuilder options) throws IOException { ConfigConstants.CONFIG_KEY_REPO_FORMAT_VERSION, 0); String reftype = repoConfig.getString( - "extensions", null, "refStorage"); //$NON-NLS-1$ //$NON-NLS-2$ + ConfigConstants.CONFIG_EXTENSIONS_SECTION, null, + ConfigConstants.CONFIG_KEY_REF_STORAGE); if (repositoryFormatVersion >= 1 && reftype != null) { - if (StringUtils.equalsIgnoreCase(reftype, "reftable")) { //$NON-NLS-1$ - refs = new FileReftableDatabase(this, - new File(getDirectory(), "refs")); //$NON-NLS-1$ + if (StringUtils.equalsIgnoreCase(reftype, + ConfigConstants.CONFIG_REF_STORAGE_REFTABLE)) { + refs = new FileReftableDatabase(this); } else if (StringUtils.equalsIgnoreCase(reftype, "reftree")) { //$NON-NLS-1$ refs = new RefTreeDatabase(this, new RefDirectory(this)); } else { throw new IOException(JGitText.get().unknownRepositoryFormat); } - } else if (FileReftableDatabase.isReftable(getDirectory())) { - refs = new FileReftableDatabase(this, new File(getDirectory(), "refs")); //$NON-NLS-1$ } else { refs = new RefDirectory(this); } @@ -640,15 +643,18 @@ public void autoGC(ProgressMonitor monitor) { * Converts the RefDatabase from reftable to RefDirectory. This operation is * not atomic. * + * @param writeLogs + * whether to write reflogs * @param backup * whether to rename or delete the old storage files. If set to - * true, the reftable list is left in "refs.old", and the - * reftable/ dir is left alone. If set to false, the reftable/ - * dir is removed, and "refs" file is removed. + * {@code true}, the reftable list is left in {@code refs.old}, + * and the {@code reftable/} dir is left alone. If set to + * {@code false}, the {@code reftable/} dir is removed, and + * {@code refs} file is removed. * @throws IOException * on IO problem */ - void convertToPackedRefs(boolean backup) throws IOException { + void convertToPackedRefs(boolean writeLogs, boolean backup) throws IOException { List all = refs.getRefs(); File packedRefs = new File(getDirectory(), Constants.PACKED_REFS); if (packedRefs.exists()) { @@ -657,26 +663,26 @@ void convertToPackedRefs(boolean backup) throws IOException { } File refsFile = new File(getDirectory(), "refs"); //$NON-NLS-1$ + File refsHeadsFile = new File(refsFile, "heads");//$NON-NLS-1$ + File headFile = new File(getDirectory(), Constants.HEAD); + FileReftableDatabase oldDb = (FileReftableDatabase) refs; - refs.close(); - - if (backup) { - File refsOld = new File(getDirectory(), "refs.old"); //$NON-NLS-1$ - if (refsOld.exists()) { - throw new IOException(MessageFormat.format( - JGitText.get().fileAlreadyExists, - "refs.old")); //$NON-NLS-1$ - } - FileUtils.rename(refsFile, refsOld); - } else { - refsFile.delete(); - } + // Remove the dummy files that ensure compatibility with older git + // versions (see convertToReftable). First make room for refs/heads/ + refsHeadsFile.delete(); + // RefDirectory wants to create the refs/ directory from scratch, so + // remove that too. + refsFile.delete(); + // remove HEAD so its previous invalid value doesn't cause issues. + headFile.delete(); // This is not atomic, but there is no way to instantiate a RefDirectory // that is disconnected from the current repo. - refs = new RefDirectory(this); + RefDirectory refDir = new RefDirectory(this); + refs = refDir; refs.create(); + ReflogWriter logWriter = refDir.newLogWriter(true); List symrefs = new ArrayList<>(); BatchRefUpdate bru = refs.newBatchUpdate(); for (Ref r : all) { @@ -686,6 +692,15 @@ void convertToPackedRefs(boolean backup) throws IOException { bru.addCommand(new ReceiveCommand(ObjectId.zeroId(), r.getObjectId(), r.getName())); } + + if (writeLogs) { + List logs = oldDb.getReflogReader(r.getName()) + .getReverseEntries(); + Collections.reverse(logs); + for (ReflogEntry e : logs) { + logWriter.log(r.getName(), e); + } + } } try (RevWalk rw = new RevWalk(this)) { @@ -721,20 +736,38 @@ void convertToPackedRefs(boolean backup) throws IOException { FileUtils.delete(reftableDir, FileUtils.RECURSIVE | FileUtils.IGNORE_ERRORS); } + repoConfig.unset(ConfigConstants.CONFIG_EXTENSIONS_SECTION, null, + ConfigConstants.CONFIG_KEY_REF_STORAGE); } + /** + * Converts the RefDatabase from RefDirectory to reftable. This operation is + * not atomic. + * + * @param writeLogs + * whether to write reflogs + * @param backup + * whether to rename or delete the old storage files. If set to + * {@code true}, the loose refs are left in {@code refs.old}, the + * packed-refs in {@code packed-refs.old} and reflogs in + * {@code refs.old/}. HEAD is left in {@code HEAD.old} and also + * {@code .log} is appended to additional refs. If set to + * {@code false}, the {@code refs/} and {@code logs/} directories + * and {@code HEAD} and additional symbolic refs are removed. + * @throws IOException + * on IO problem + */ @SuppressWarnings("nls") void convertToReftable(boolean writeLogs, boolean backup) throws IOException { - File newRefs = new File(getDirectory(), "refs.new"); File reftableDir = new File(getDirectory(), Constants.REFTABLE); - + File headFile = new File(getDirectory(), Constants.HEAD); if (reftableDir.exists() && reftableDir.listFiles().length > 0) { throw new IOException(JGitText.get().reftableDirExists); } // Ignore return value, as it is tied to temporary newRefs file. - FileReftableDatabase.convertFrom(this, newRefs, writeLogs); + FileReftableDatabase.convertFrom(this, writeLogs); File refsFile = new File(getDirectory(), "refs"); @@ -742,7 +775,6 @@ void convertToReftable(boolean writeLogs, boolean backup) File packedRefs = new File(getDirectory(), Constants.PACKED_REFS); File logsDir = new File(getDirectory(), Constants.LOGS); - List additional = getRefDatabase().getAdditionalRefs().stream() .map(Ref::getName).collect(toList()); additional.add(Constants.HEAD); @@ -761,7 +793,8 @@ void convertToReftable(boolean writeLogs, boolean backup) new File(getDirectory(), r + ".old")); } } else { - packedRefs.delete(); // ignore return value. + FileUtils.delete(packedRefs, FileUtils.SKIP_MISSING); + FileUtils.delete(headFile); FileUtils.delete(logsDir, FileUtils.RECURSIVE); FileUtils.delete(refsFile, FileUtils.RECURSIVE); for (String r : additional) { @@ -769,11 +802,26 @@ void convertToReftable(boolean writeLogs, boolean backup) } } - // Put new data. - FileUtils.rename(newRefs, refsFile); + FileUtils.mkdir(refsFile, true); + // By putting in a dummy HEAD, old versions of Git still detect a repo + // (that they can't read) + try (OutputStream os = new FileOutputStream(headFile)) { + os.write(Constants.encodeASCII("ref: refs/heads/.invalid")); + } + + // Some tools might write directly into .git/refs/heads/BRANCH. By + // putting a file here, this fails spectacularly. + FileUtils.createNewFile(new File(refsFile, "heads")); + + repoConfig.setString(ConfigConstants.CONFIG_EXTENSIONS_SECTION, null, + ConfigConstants.CONFIG_KEY_REF_STORAGE, + ConfigConstants.CONFIG_REF_STORAGE_REFTABLE); + repoConfig.setLong(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_REPO_FORMAT_VERSION, 1); + repoConfig.save(); refs.close(); - refs = new FileReftableDatabase(this, refsFile); + refs = new FileReftableDatabase(this); } /** @@ -797,7 +845,7 @@ public void convertRefStorage(String format, boolean writeLogs, } } else if (format.equals("refdir")) {//$NON-NLS-1$ if (refs instanceof FileReftableDatabase) { - convertToPackedRefs(backup); + convertToPackedRefs(writeLogs, backup); } } else { throw new IOException(String.format( diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 695903d04..99512a6a4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -443,6 +443,12 @@ public final class ConfigConstants { */ public static final String CONFIG_RENAMELIMIT_COPIES = "copies"; + /** + * A "refStorage" value in the "extensions". + * @since 5.6.2 + */ + public static final String CONFIG_REF_STORAGE_REFTABLE = "reftable"; + /** * The "renames" key in the "diff" section * @since 3.0 @@ -538,9 +544,24 @@ public final class ConfigConstants { */ public static final String CONFIG_KEY_MIN_RACY_THRESHOLD = "minRacyThreshold"; + + /** + * The "refStorage" key + * + * @since 5.6.2 + */ + public static final String CONFIG_KEY_REF_STORAGE = "refStorage"; + /** * The "jmx" section * @since 5.1.13 */ public static final String CONFIG_JMX_SECTION = "jmx"; + + /** + * The "extensions" section + * + * @since 5.6.2 + */ + public static final String CONFIG_EXTENSIONS_SECTION = "extensions"; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java index abd9dd6d3..3c05cab86 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -287,6 +287,12 @@ public final class Constants { */ public static final String REFTABLE = "reftable"; + /** + * Reftable table list name. + * @since 5.6.2 + */ + public static final String TABLES_LIST = "tables.list"; + /** Info refs folder */ public static final String INFO_REFS = "info/refs"; From 093fbbd11e01711efc1567d6c31516a448d2a455 Mon Sep 17 00:00:00 2001 From: Alex Blewitt Date: Tue, 25 Feb 2020 21:14:59 +0000 Subject: [PATCH 2/4] Expose FileStoreAttributes.setBackground() The FS.setAsyncFileStoreAttributes() static method calls FileStoreAttributes.setBackground() as its implementation, but there are other public attributes on this inner class already and there isn't a real reason why this needs to be private. By making it public we allow callers to be able to invoke it directly. Although it doesn't appear that it would make a difference, by calling a static method on the FS class, all static fields and the transitive closure of class dependencies must be loaded and initialised, which can be non-trivial. Callers referring to FS.setAsyncFileStoreAttributes() may be replaced with FS.FileStoreAttributes.setBackground() with no change of behaviour other than improved performance due to less class loading required. Bug: 560527 Change-Id: I9538acc90da8d18f53fd60d74eb54496857f93a5 Signed-off-by: Alex Blewitt --- org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 265950261..2446de4c1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -242,7 +242,16 @@ public static final class FileStoreAttributes { private static Map locks = new ConcurrentHashMap<>(); - private static void setBackground(boolean async) { + /** + * Whether FileStore attributes should be determined asynchronously + * + * @param async + * whether FileStore attributes should be determined + * asynchronously. If false access to cached attributes may block + * for some seconds for the first call per FileStore + * @since 5.6.2 + */ + public static void setBackground(boolean async) { background.set(async); } @@ -709,7 +718,9 @@ public static FS detect() { * asynchronously. If false access to cached attributes may block * for some seconds for the first call per FileStore * @since 5.1.9 + * @deprecated Use {@link FileStoreAttributes#setBackground} instead */ + @Deprecated public static void setAsyncFileStoreAttributes(boolean asynch) { FileStoreAttributes.setBackground(asynch); } From 1d41be7c54c698d21a54bc1a33b5b23e095d542a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 5 Mar 2020 13:13:47 +0900 Subject: [PATCH 3/4] Bump Bazel version to 2.2.0 Change-Id: I889052040f31708c6b8de0cf3c171a04722f7c96 Signed-off-by: David Pursehouse --- .bazelversion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bazelversion b/.bazelversion index 7ec1d6db4..ccbccc3dc 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -2.1.0 +2.2.0 From 241557137070d680cf730ba1633df5e4c4266a1d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 5 Mar 2020 13:53:10 +0100 Subject: [PATCH 4/4] Silence API errors introduced by 093fbbd1 Change-Id: I1c9d5a25bd06a1152e953c45b683375cb05aa254 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/.settings/.api_filters | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 5fc26a553..eb652a252 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -188,6 +188,14 @@ + + + + + + + +