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 <thomas.wolf@paranor.ch>

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 f2f277c6ea..9be71c3a02 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 class ReflogConfigTest extends RepositoryTestCase {
 		// 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 class ReflogConfigTest extends RepositoryTestCase {
 		// 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 131f716cf0..98d6ea00e7 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.nio.channels.FileChannel;
 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.PersonIdent;
 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 @@ public class ReflogWriter {
 	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 @@ public class ReflogWriter {
 		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 98de3a91cc..cdfa949cad 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 class CoreConfig {
 	 * @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 class CoreConfig {
 		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 class CoreConfig {
 	 * @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 @@ public class CoreConfig {
 				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 class CoreConfig {
 	 * 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);
 	}
 
 	/**