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 <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileReftableTest.java

@@ -124,11 +124,6 @@ public class FileReftableTest extends SampleDataRepositoryTestCase {
 		}
 	}
 
-	@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 class FileReftableTest extends SampleDataRepositoryTestCase {
 		}
 
 		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 class FileReftableTest extends SampleDataRepositoryTestCase {
 
 	@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 class FileReftableTest extends SampleDataRepositoryTestCase {
 		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<ReflogEntry> 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");

org.eclipse.jgit/.settings/.api_filters

@@ -53,6 +53,24 @@
                 <message_argument value="CONFIG_KEY_PACKED_GIT_WINDOWSIZE"/>
             </message_arguments>
         </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.6.2"/>
+                <message_argument value="CONFIG_EXTENSIONS_SECTION"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.6.2"/>
+                <message_argument value="CONFIG_KEY_REF_STORAGE"/>
+            </message_arguments>
+        </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.6.2"/>
+                <message_argument value="CONFIG_REF_STORAGE_REFTABLE"/>
+            </message_arguments>
+        </filter>
     </resource>
     <resource path="src/org/eclipse/jgit/lib/Constants.java" type="org.eclipse.jgit.lib.Constants">
         <filter id="1142947843">
@@ -61,6 +79,12 @@
                 <message_argument value="XDG_CONFIG_HOME"/>
             </message_arguments>
         </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="5.6.2"/>
+                <message_argument value="TABLES_LIST"/>
+            </message_arguments>
+        </filter>
     </resource>
     <resource path="src/org/eclipse/jgit/revwalk/ReachabilityChecker.java" type="org.eclipse.jgit.revwalk.ReachabilityChecker">
         <filter id="403804204">

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 @@ public class FileReftableDatabase extends RefDatabase {
 	 * @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$
+		return new File(repoDir, Constants.REFTABLE).isDirectory();
-				&& new File(repoDir, Constants.REFTABLE).isDirectory();
 	}
 
 	/** {@inheritDoc} */
@@ -626,8 +630,6 @@ public class FileReftableDatabase extends RefDatabase {
 	/**
 	 * @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 @@ public class FileReftableDatabase extends RefDatabase {
 	 *             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) {
+			if (reftableList != null) {
-				refstackName.delete();
+				reftableList.delete();
 			}
 		}
 		return newDb;

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java

@@ -51,10 +51,13 @@ import static java.util.stream.Collectors.toList;
 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.ProgressMonitor;
 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 class FileRepository extends Repository {
 				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$
+			if (StringUtils.equalsIgnoreCase(reftype,
-				refs = new FileReftableDatabase(this,
+					ConfigConstants.CONFIG_REF_STORAGE_REFTABLE)) {
-						new File(getDirectory(), "refs")); //$NON-NLS-1$
+				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 class FileRepository extends Repository {
 	 * 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
+	 *            {@code true}, the reftable list is left in {@code refs.old},
-	 *            reftable/ dir is left alone. If set to false, the reftable/
+	 *            and the {@code reftable/} dir is left alone. If set to
-	 *            dir is removed, and "refs" file is removed.
+	 *            {@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<Ref> all = refs.getRefs();
 		File packedRefs = new File(getDirectory(), Constants.PACKED_REFS);
 		if (packedRefs.exists()) {
@@ -657,26 +663,26 @@ public class FileRepository extends Repository {
 		}
 
 		File refsFile = new File(getDirectory(), "refs"); //$NON-NLS-1$
-
+		File refsHeadsFile = new File(refsFile, "heads");//$NON-NLS-1$
-		refs.close();
+		File headFile = new File(getDirectory(), Constants.HEAD);
-
+		FileReftableDatabase oldDb = (FileReftableDatabase) refs;
-		if (backup) {
+
-			File refsOld = new File(getDirectory(), "refs.old"); //$NON-NLS-1$
+		// Remove the dummy files that ensure compatibility with older git
-			if (refsOld.exists()) {
+		// versions (see convertToReftable). First make room for refs/heads/
-				throw new IOException(MessageFormat.format(
+		refsHeadsFile.delete();
-					JGitText.get().fileAlreadyExists,
+		// RefDirectory wants to create the refs/ directory from scratch, so
-						"refs.old")); //$NON-NLS-1$
+		// remove that too.
-			}
+		refsFile.delete();
-			FileUtils.rename(refsFile, refsOld);
+		// remove HEAD so its previous invalid value doesn't cause issues.
-		} else {
+		headFile.delete();
-			refsFile.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<Ref> symrefs = new ArrayList<>();
 		BatchRefUpdate bru = refs.newBatchUpdate();
 		for (Ref r : all) {
@@ -686,6 +692,15 @@ public class FileRepository extends Repository {
 				bru.addCommand(new ReceiveCommand(ObjectId.zeroId(),
 						r.getObjectId(), r.getName()));
 			}
+
+			if (writeLogs) {
+				List<ReflogEntry> 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 @@ public class FileRepository extends Repository {
 			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 @@ public class FileRepository extends Repository {
 		File packedRefs = new File(getDirectory(), Constants.PACKED_REFS);
 		File logsDir = new File(getDirectory(), Constants.LOGS);
 
-
 		List<String> additional = getRefDatabase().getAdditionalRefs().stream()
 				.map(Ref::getName).collect(toList());
 		additional.add(Constants.HEAD);
@@ -761,7 +793,8 @@ public class FileRepository extends Repository {
 					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 @@ public class FileRepository extends Repository {
 			}
 		}
 
-		// Put new data.
+		FileUtils.mkdir(refsFile, true);
-		FileUtils.rename(newRefs, refsFile);
+
+		// 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 class FileRepository extends Repository {
 			}
 		} else if (format.equals("refdir")) {//$NON-NLS-1$
 			if (refs instanceof FileReftableDatabase) {
-				convertToPackedRefs(backup);
+				convertToPackedRefs(writeLogs, backup);
 			}
 		} else {
 			throw new IOException(String.format(

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";
 }

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";