From b87f1259d679c7af83b7fd1ab148c99c613beaf0 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Wed, 2 Oct 2019 17:33:43 +0200 Subject: [PATCH] Fix parsing of core.logAllRefUpdates Also correctly parse the "always" value (allowed in canonical git since git 2.12.0[1]). Adapt the ReflogWriter. [1] https://github.com/git/git/commit/341fb2862 Bug: 551664 Change-Id: I051c76ca355a2ac8d6092de65f44b18bf9aeb125 Signed-off-by: Thomas Wolf --- .../eclipse/jgit/lib/ReflogConfigTest.java | 28 +++++++- .../internal/storage/file/ReflogWriter.java | 34 +++++++--- .../src/org/eclipse/jgit/lib/CoreConfig.java | 68 +++++++++++++------ 3 files changed, 97 insertions(+), 33 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReflogConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReflogConfigTest.java index f2f277c6e..9be71c3a0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReflogConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReflogConfigTest.java @@ -45,7 +45,7 @@ package org.eclipse.jgit.lib; -import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -78,7 +78,10 @@ public void testlogAllRefUpdates() throws Exception { // set the logAllRefUpdates parameter to true and check it cfg.setBoolean("core", null, "logallrefupdates", true); cfg.save(); - assertTrue(cfg.get(CoreConfig.KEY).isLogAllRefUpdates()); + assertEquals(CoreConfig.LogRefUpdates.TRUE, + cfg.getEnum(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, + CoreConfig.LogRefUpdates.FALSE)); // do one commit and check that reflog size is increased to 1 commit("A Commit\n", commitTime, tz); @@ -90,13 +93,32 @@ public void testlogAllRefUpdates() throws Exception { // set the logAllRefUpdates parameter to false and check it cfg.setBoolean("core", null, "logallrefupdates", false); cfg.save(); - assertFalse(cfg.get(CoreConfig.KEY).isLogAllRefUpdates()); + assertEquals(CoreConfig.LogRefUpdates.FALSE, + cfg.getEnum(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, + CoreConfig.LogRefUpdates.TRUE)); // do one commit and check that reflog size is 2 commit("A Commit\n", commitTime, tz); + commitTime += 60 * 1000; assertTrue( "Reflog for HEAD should contain two entries", db.getReflogReader(Constants.HEAD).getReverseEntries().size() == 2); + + // set the logAllRefUpdates parameter to false and check it + cfg.setEnum("core", null, "logallrefupdates", + CoreConfig.LogRefUpdates.ALWAYS); + cfg.save(); + assertEquals(CoreConfig.LogRefUpdates.ALWAYS, + cfg.getEnum(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, + CoreConfig.LogRefUpdates.FALSE)); + + // do one commit and check that reflog size is 3 + commit("A Commit\n", commitTime, tz); + assertTrue("Reflog for HEAD should contain three entries", + db.getReflogReader(Constants.HEAD).getReverseEntries() + .size() == 3); } private void commit(String commitMsg, long commitTime, int tz) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java index 131f716cf..98d6ea00e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java @@ -61,6 +61,7 @@ import java.text.MessageFormat; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.CoreConfig; import org.eclipse.jgit.lib.ObjectId; @@ -68,6 +69,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.ReflogEntry; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.util.FileUtils; /** @@ -239,7 +241,7 @@ private FileOutputStream getFileOutputStream(File log) throws IOException { private ReflogWriter log(String refName, byte[] rec) throws IOException { File log = refdb.logFor(refName); boolean write = forceWrite - || (isLogAllRefUpdates() && shouldAutoCreateLog(refName)) + || shouldAutoCreateLog(refName) || log.isFile(); if (!write) return this; @@ -260,15 +262,27 @@ private ReflogWriter log(String refName, byte[] rec) throws IOException { return this; } - private boolean isLogAllRefUpdates() { - return refdb.getRepository().getConfig().get(CoreConfig.KEY) - .isLogAllRefUpdates(); - } - private boolean shouldAutoCreateLog(String refName) { - return refName.equals(HEAD) - || refName.startsWith(R_HEADS) - || refName.startsWith(R_REMOTES) - || refName.startsWith(R_NOTES); + Repository repo = refdb.getRepository(); + CoreConfig.LogRefUpdates value = repo.isBare() + ? CoreConfig.LogRefUpdates.FALSE + : CoreConfig.LogRefUpdates.TRUE; + value = repo.getConfig().getEnum(ConfigConstants.CONFIG_CORE_SECTION, + null, ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, value); + if (value != null) { + switch (value) { + case FALSE: + break; + case TRUE: + return refName.equals(HEAD) || refName.startsWith(R_HEADS) + || refName.startsWith(R_REMOTES) + || refName.startsWith(R_NOTES); + case ALWAYS: + return refName.equals(HEAD) || refName.startsWith(R_REFS); + default: + break; + } + } + return false; } } \ No newline at end of file diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java index 98de3a91c..cdfa949ca 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java @@ -79,40 +79,40 @@ public static enum AutoCRLF { * @since 4.3 */ public static enum EOL { - /** checkin with LF, checkout with CRLF. */ + /** Check in with LF, check out with CRLF. */ CRLF, - /** checkin with LF, checkout without conversion. */ + /** Check in with LF, check out without conversion. */ LF, - /** use the platform's native line ending. */ + /** Use the platform's native line ending. */ NATIVE; } /** - * EOL stream conversion protocol + * EOL stream conversion protocol. * * @since 4.3 */ public static enum EolStreamType { - /** convert to CRLF without binary detection */ + /** Convert to CRLF without binary detection. */ TEXT_CRLF, - /** convert to LF without binary detection */ + /** Convert to LF without binary detection. */ TEXT_LF, - /** convert to CRLF with binary detection */ + /** Convert to CRLF with binary detection. */ AUTO_CRLF, - /** convert to LF with binary detection */ + /** Convert to LF with binary detection. */ AUTO_LF, - /** do not convert */ + /** Do not convert. */ DIRECT; } /** - * Permissible values for {@code core.checkstat} + * Permissible values for {@code core.checkstat}. * * @since 3.0 */ @@ -130,11 +130,30 @@ public static enum CheckStat { DEFAULT } + /** + * Permissible values for {@code core.logAllRefUpdates}. + * + * @since 5.6 + */ + public static enum LogRefUpdates { + /** Don't create ref logs; default for bare repositories. */ + FALSE, + + /** + * Create ref logs for refs/heads/**, refs/remotes/**, refs/notes/**, + * and for HEAD. Default for non-bare repositories. + */ + TRUE, + + /** Create ref logs for all refs/** and for HEAD. */ + ALWAYS + } + private final int compression; private final int packIndexVersion; - private final boolean logAllRefUpdates; + private final LogRefUpdates logAllRefUpdates; private final String excludesfile; @@ -146,23 +165,26 @@ public static enum CheckStat { * @since 3.3 */ public static enum SymLinks { - /** Checkout symbolic links as plain files */ + /** Check out symbolic links as plain files . */ FALSE, - /** Checkout symbolic links as links */ + + /** Check out symbolic links as links. */ TRUE } /** - * Options for hiding files whose names start with a period + * Options for hiding files whose names start with a period. * * @since 3.5 */ public static enum HideDotFiles { - /** Do not hide .files */ + /** Do not hide .files. */ FALSE, - /** Hide add .files */ + + /** Hide add .files. */ TRUE, - /** Hide only .git */ + + /** Hide only .git. */ DOTGITONLY } @@ -171,8 +193,9 @@ private CoreConfig(Config rc) { ConfigConstants.CONFIG_KEY_COMPRESSION, DEFAULT_COMPRESSION); packIndexVersion = rc.getInt(ConfigConstants.CONFIG_PACK_SECTION, ConfigConstants.CONFIG_KEY_INDEXVERSION, 2); - logAllRefUpdates = rc.getBoolean(ConfigConstants.CONFIG_CORE_SECTION, - ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, true); + logAllRefUpdates = rc.getEnum(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, + LogRefUpdates.TRUE); excludesfile = rc.getString(ConfigConstants.CONFIG_CORE_SECTION, null, ConfigConstants.CONFIG_KEY_EXCLUDESFILE); attributesfile = rc.getString(ConfigConstants.CONFIG_CORE_SECTION, @@ -201,9 +224,14 @@ public int getPackIndexVersion() { * Whether to log all refUpdates * * @return whether to log all refUpdates + * @deprecated since 5.6; default value depends on whether the repository is + * bare. Use + * {@link Config#getEnum(String, String, String, Enum)} + * directly. */ + @Deprecated public boolean isLogAllRefUpdates() { - return logAllRefUpdates; + return !LogRefUpdates.FALSE.equals(logAllRefUpdates); } /**