From 838b5a84b5093c335b95a644b8888006d9e95493 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 7 Oct 2019 17:58:56 +0200 Subject: [PATCH] Store filesystem timestamp resolution in extra jgit config This avoids polluting hand-crafted user level config with auto-configured options which might disturb in environments where the user level config is replicated between different machines. Add a jgit config as parent of the system level config. Persist measured timestamp resolutions always in this jgit config and read it via the user global config. This has the effect that auto-configured timestamp resolution will be used by default and can be overridden in either the system level or user level config. Store the jgit config under the XDG_CONFIG_HOME directory following the XDG base directory specification [1] in order to ensure that we have write permissions to persist the file. This has the effect that each OS user will use its jgit config since they typically use different XDG_CONFIG_HOME directories. If the environment variable XDG_CONFIG_HOME is defined the jgit config file is located at $XDG_CONFIG_HOME/jgit/config otherwise the default is ~/.config/jgit/config. If you want to avoid redundant measurement for different OS users manually copy the values measured and auto-configured for one OS user to the system level git config. [1] https://wiki.archlinux.org/index.php/XDG_Base_Directory Bug: 551850 Change-Id: I0022bd40ae62f82e5b964c2ea25822eb55d94687 Signed-off-by: Matthias Sohn --- .../junit/LocalDiskRepositoryTestCase.java | 12 ++- .../eclipse/jgit/junit/MockSystemReader.java | 23 +++++ .../org/eclipse/jgit/util/FS_POSIXTest.java | 14 +-- org.eclipse.jgit/.settings/.api_filters | 34 ++++++++ .../eclipse/jgit/internal/JGitText.properties | 2 + .../org/eclipse/jgit/internal/JGitText.java | 2 + .../src/org/eclipse/jgit/lib/Constants.java | 9 ++ .../src/org/eclipse/jgit/util/FS.java | 22 ++--- .../org/eclipse/jgit/util/SystemReader.java | 86 ++++++++++++++++++- 9 files changed, 181 insertions(+), 23 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java index 5c2cd6ac6..eca8179ec 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java @@ -125,8 +125,9 @@ public abstract class LocalDiskRepositoryTestCase { public void setUp() throws Exception { tmp = File.createTempFile("jgit_test_", "_tmp"); CleanupThread.deleteOnShutdown(tmp); - if (!tmp.delete() || !tmp.mkdir()) + if (!tmp.delete() || !tmp.mkdir()) { throw new IOException("Cannot create " + tmp); + } mockSystemReader = new MockSystemReader(); SystemReader.setInstance(mockSystemReader); @@ -137,7 +138,11 @@ public abstract class LocalDiskRepositoryTestCase { // the same one here FS.getFileStoreAttributes(tmp.toPath().getParent()); - FileBasedConfig userConfig = new FileBasedConfig( + FileBasedConfig jgitConfig = new FileBasedConfig( + new File(tmp, "jgitconfig"), FS.DETECTED); + FileBasedConfig systemConfig = new FileBasedConfig(jgitConfig, + new File(tmp, "systemgitconfig"), FS.DETECTED); + FileBasedConfig userConfig = new FileBasedConfig(systemConfig, new File(tmp, "usergitconfig"), FS.DETECTED); // We have to set autoDetach to false for tests, because tests expect to be able // to clean up by recursively removing the repository, and background GC might be @@ -145,7 +150,10 @@ public abstract class LocalDiskRepositoryTestCase { userConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, null, ConfigConstants.CONFIG_KEY_AUTODETACH, false); userConfig.save(); + mockSystemReader.setJGitConfig(jgitConfig); + mockSystemReader.setSystemGitConfig(systemConfig); mockSystemReader.setUserGitConfig(userConfig); + ceilTestDirectories(getCeilings()); author = new PersonIdent("J. Author", "jauthor@example.com"); diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java index 13c293228..630c8a479 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/MockSystemReader.java @@ -103,6 +103,8 @@ public class MockSystemReader extends SystemReader { private FileBasedConfig userGitConfig; + private FileBasedConfig jgitConfig; + FileBasedConfig systemGitConfig; /** @@ -118,6 +120,16 @@ public class MockSystemReader extends SystemReader { return old; } + /** + * Set the jgit config stored at $XDG_CONFIG_HOME/jgit/config + * + * @param jgitConfig + * set the jgit configuration + */ + public void setJGitConfig(FileBasedConfig jgitConfig) { + this.jgitConfig = jgitConfig; + } + /** * Set the system-level git config * @@ -142,6 +154,7 @@ public class MockSystemReader extends SystemReader { init(Constants.GIT_COMMITTER_EMAIL_KEY); setProperty(Constants.OS_USER_DIR, "."); userGitConfig = new MockConfig(null, null); + jgitConfig = new MockConfig(null, null); systemGitConfig = new MockConfig(null, null); setCurrentPlatform(); } @@ -199,6 +212,11 @@ public class MockSystemReader extends SystemReader { return userGitConfig; } + @Override + public FileBasedConfig getJGitConfig() { + return jgitConfig; + } + @Override public StoredConfig getSystemConfig() throws IOException, ConfigInvalidException { @@ -333,4 +351,9 @@ public class MockSystemReader extends SystemReader { return "MockSystemReader"; } + @Override + public FileBasedConfig openJGitConfig(Config parent, FS fs) { + return jgitConfig; + } + } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java index 87349a25a..d7a7d8677 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FS_POSIXTest.java @@ -59,7 +59,7 @@ import org.junit.Before; import org.junit.Test; public class FS_POSIXTest { - private SystemReader originalSystemReaderInstance; + private FileBasedConfig jgitConfig; private FileBasedConfig systemConfig; @@ -70,6 +70,7 @@ public class FS_POSIXTest { @Before public void setUp() throws Exception { tmp = Files.createTempDirectory("jgit_test_"); + MockSystemReader mockSystemReader = new MockSystemReader(); SystemReader.setInstance(mockSystemReader); @@ -78,7 +79,10 @@ public class FS_POSIXTest { // The MockSystemReader must be configured first since we need to use // the same one here FS.getFileStoreAttributes(tmp.getParent()); - systemConfig = new FileBasedConfig( + + jgitConfig = new FileBasedConfig(new File(tmp.toFile(), "jgitconfig"), + FS.DETECTED); + systemConfig = new FileBasedConfig(jgitConfig, new File(tmp.toFile(), "systemgitconfig"), FS.DETECTED); userConfig = new FileBasedConfig(systemConfig, new File(tmp.toFile(), "usergitconfig"), FS.DETECTED); @@ -89,16 +93,14 @@ public class FS_POSIXTest { userConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, null, ConfigConstants.CONFIG_KEY_AUTODETACH, false); userConfig.save(); + mockSystemReader.setJGitConfig(jgitConfig); mockSystemReader.setSystemGitConfig(systemConfig); mockSystemReader.setUserGitConfig(userConfig); - - originalSystemReaderInstance = SystemReader.getInstance(); - SystemReader.setInstance(mockSystemReader); } @After public void tearDown() throws IOException { - SystemReader.setInstance(originalSystemReaderInstance); + SystemReader.setInstance(null); FileUtils.delete(tmp.toFile(), FileUtils.RECURSIVE | FileUtils.RETRY); } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 571751296..2d5208176 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -1,5 +1,13 @@ + + + + + + + + @@ -57,6 +65,14 @@ + + + + + + + + @@ -159,6 +175,12 @@ + + + + + + @@ -171,5 +193,17 @@ + + + + + + + + + + + + diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 6b09faf0a..b82512f71 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -201,8 +201,10 @@ countingObjects=Counting objects corruptPack=Pack file {0} is corrupt, removing it from pack list createBranchFailedUnknownReason=Create branch failed for unknown reason createBranchUnexpectedResult=Create branch returned unexpected result {0} +createJGitConfigFailed=Creating JGit config directory {} failed createNewFileFailed=Could not create new file {0} createRequiresZeroOldId=Create requires old ID to be zero +createXDGConfigHomeFailed=Creating XDG_CONFIG_HOME directory {} failed credentialPassword=Password credentialPassphrase=Passphrase credentialUsername=Username diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 6a7d22df9..61b145657 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -262,8 +262,10 @@ public class JGitText extends TranslationBundle { /***/ public String countingObjects; /***/ public String createBranchFailedUnknownReason; /***/ public String createBranchUnexpectedResult; + /***/ public String createJGitConfigFailed; /***/ public String createNewFileFailed; /***/ public String createRequiresZeroOldId; + /***/ public String createXDGConfigHomeFailed; /***/ public String credentialPassword; /***/ public String credentialPassphrase; /***/ public String credentialUsername; 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 a084c8287..9274fc677 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -342,6 +342,15 @@ public final class Constants { */ public static final String GIT_CONFIG_NOSYSTEM_KEY = "GIT_CONFIG_NOSYSTEM"; + /** + * The key of the XDG_CONFIG_HOME directory defined in the XDG base + * directory specification, see + * {@link "https://wiki.archlinux.org/index.php/XDG_Base_Directory"} + * + * @since 5.5.2 + */ + public static final String XDG_CONFIG_HOME = "XDG_CONFIG_HOME"; + /** * The environment variable that limits how close to the root of the file * systems JGit will traverse when looking for a repository root. 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 29519298c..b3d01d3b4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -545,9 +545,9 @@ public abstract class FS { private static void saveToConfig(FileStore s, FileStoreAttributes c) { - StoredConfig userConfig; + StoredConfig jgitConfig; try { - userConfig = SystemReader.getInstance().getUserConfig(); + jgitConfig = SystemReader.getInstance().getJGitConfig(); } catch (IOException | ConfigInvalidException e) { LOG.error(JGitText.get().saveFileStoreAttributesFailed, e); return; @@ -568,20 +568,19 @@ public abstract class FS { String key = getConfigKey(s); while (!succeeded && retries < max_retries) { try { - userConfig.load(); - userConfig.setString( + jgitConfig.setString( ConfigConstants.CONFIG_FILESYSTEM_SECTION, key, ConfigConstants.CONFIG_KEY_TIMESTAMP_RESOLUTION, String.format("%d %s", //$NON-NLS-1$ Long.valueOf(resolutionValue), resolutionUnit.name().toLowerCase())); - userConfig.setString( + jgitConfig.setString( ConfigConstants.CONFIG_FILESYSTEM_SECTION, key, ConfigConstants.CONFIG_KEY_MIN_RACY_THRESHOLD, String.format("%d %s", //$NON-NLS-1$ Long.valueOf(minRacyThresholdValue), minRacyThresholdUnit.name().toLowerCase())); - userConfig.save(); + jgitConfig.save(); succeeded = true; } catch (LockFailedException e) { // race with another thread, wait a bit and try again @@ -590,11 +589,11 @@ public abstract class FS { if (retries < max_retries) { Thread.sleep(100); LOG.debug("locking {} failed, retries {}/{}", //$NON-NLS-1$ - userConfig, Integer.valueOf(retries), + jgitConfig, Integer.valueOf(retries), Integer.valueOf(max_retries)); } else { LOG.warn(MessageFormat.format( - JGitText.get().lockFailedRetry, userConfig, + JGitText.get().lockFailedRetry, jgitConfig, Integer.valueOf(retries))); } } catch (InterruptedException e1) { @@ -603,12 +602,7 @@ public abstract class FS { } } catch (IOException e) { LOG.error(MessageFormat.format( - JGitText.get().cannotSaveConfig, userConfig), e); - break; - } catch (ConfigInvalidException e) { - LOG.error(MessageFormat.format( - JGitText.get().repositoryConfigFileInvalid, - userConfig, e.getMessage())); + JGitText.get().cannotSaveConfig, jgitConfig), e); break; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java index a2253bc11..59184ffda 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -50,6 +50,10 @@ import java.io.File; import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.security.AccessController; import java.security.PrivilegedAction; import java.text.DateFormat; @@ -60,6 +64,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.CorruptObjectException; +import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectChecker; @@ -137,6 +142,42 @@ public abstract class SystemReader { fs); } + private Path getXDGConfigHome(FS fs) { + String configHomePath = getenv(Constants.XDG_CONFIG_HOME); + if (StringUtils.isEmptyOrNull(configHomePath)) { + configHomePath = new File(fs.userHome(), ".config") //$NON-NLS-1$ + .getAbsolutePath(); + } + try { + Path xdgHomePath = Paths.get(configHomePath); + Files.createDirectories(xdgHomePath); + return xdgHomePath; + } catch (IOException | InvalidPathException e) { + LOG.error(JGitText.get().createXDGConfigHomeFailed, + configHomePath, e); + } + return null; + } + + @Override + public FileBasedConfig openJGitConfig(Config parent, FS fs) { + Path xdgPath = getXDGConfigHome(fs); + if (xdgPath != null) { + Path configPath = null; + try { + configPath = xdgPath.resolve("jgit"); //$NON-NLS-1$ + Files.createDirectories(configPath); + configPath = configPath.resolve(Constants.CONFIG); + return new FileBasedConfig(parent, configPath.toFile(), fs); + } catch (IOException e) { + LOG.error(JGitText.get().createJGitConfigFailed, configPath, + e); + } + } + return new FileBasedConfig(parent, + new File(fs.userHome(), ".jgitconfig"), fs); //$NON-NLS-1$ + } + @Override public String getHostname() { if (hostname == null) { @@ -198,6 +239,8 @@ public abstract class SystemReader { private AtomicReference userConfig = new AtomicReference<>(); + private AtomicReference jgitConfig = new AtomicReference<>(); + private void init() { // Creating ObjectChecker must be deferred. Unit tests change // behavior of is{Windows,MacOS} in constructor of subclass. @@ -274,6 +317,22 @@ public abstract class SystemReader { */ public abstract FileBasedConfig openSystemConfig(Config parent, FS fs); + /** + * Open the jgit configuration located at $XDG_CONFIG_HOME/jgit/config. Use + * {@link #getJGitConfig()} to get the current jgit configuration in the + * user home since it manages automatic reloading when the jgit config file + * was modified and avoids unnecessary reloads. + * + * @param parent + * a config with values not found directly in the returned config + * @param fs + * the file system abstraction which will be necessary to perform + * certain file system operations. + * @return the jgit configuration located at $XDG_CONFIG_HOME/jgit/config + * @since 5.5.2 + */ + public abstract FileBasedConfig openJGitConfig(Config parent, FS fs); + /** * Get the git configuration found in the user home. The configuration will * be reloaded automatically if the configuration file was modified. Also @@ -301,6 +360,31 @@ public abstract class SystemReader { return c; } + /** + * Get the jgit configuration located at $XDG_CONFIG_HOME/jgit/config. The + * configuration will be reloaded automatically if the configuration file + * was modified. If the configuration file wasn't modified returns the + * cached configuration. + * + * @return the jgit configuration located at $XDG_CONFIG_HOME/jgit/config + * @throws ConfigInvalidException + * if configuration is invalid + * @throws IOException + * if something went wrong when reading files + * @since 5.5.2 + */ + public StoredConfig getJGitConfig() + throws ConfigInvalidException, IOException { + FileBasedConfig c = jgitConfig.get(); + if (c == null) { + jgitConfig.compareAndSet(null, + openJGitConfig(null, FS.DETECTED)); + c = jgitConfig.get(); + } + updateAll(c); + return c; + } + /** * Get the gitconfig configuration found in the system-wide "etc" directory. * The configuration will be reloaded automatically if the configuration @@ -319,7 +403,7 @@ public abstract class SystemReader { FileBasedConfig c = systemConfig.get(); if (c == null) { systemConfig.compareAndSet(null, - openSystemConfig(null, FS.DETECTED)); + openSystemConfig(getJGitConfig(), FS.DETECTED)); c = systemConfig.get(); } updateAll(c);