From f383206ace187ec92672b806e18a54e8d398a27d Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Thu, 15 Aug 2019 01:25:28 +0200 Subject: [PATCH] Cache user global and system-wide git configurations So far the git configuration and the system wide git configuration were always reloaded when jgit accessed these global configuration files to access global configuration options which are not in the context of a single git repository. Cache these configurations in SystemReader and only reload them if their file metadata observed using FileSnapshot indicates a modification. Change-Id: I092fe11a5d95f1c5799273cacfc7a415d0b7786c Signed-off-by: Matthias Sohn Signed-off-by: Thomas Wolf --- .../http/test/SmartClientSmartServerTest.java | 18 ++- .../junit/LocalDiskRepositoryTestCase.java | 9 +- .../eclipse/jgit/junit/MockSystemReader.java | 41 +++++- org.eclipse.jgit.lfs/.settings/.api_filters | 11 ++ .../jgit/lfs/InstallBuiltinLfsCommand.java | 41 +++--- .../eclipse/jgit/api/CloneCommandTest.java | 6 +- .../org/eclipse/jgit/util/FS_POSIXTest.java | 74 ++++++---- org.eclipse.jgit/.settings/.api_filters | 26 ++++ .../eclipse/jgit/internal/JGitText.properties | 5 +- .../org/eclipse/jgit/internal/JGitText.java | 5 +- .../internal/storage/file/FileRepository.java | 85 +++-------- .../eclipse/jgit/transport/HttpConfig.java | 11 +- .../eclipse/jgit/transport/TransportHttp.java | 10 +- .../src/org/eclipse/jgit/util/FS.java | 25 ++-- .../src/org/eclipse/jgit/util/FS_POSIX.java | 64 +++------ .../org/eclipse/jgit/util/SystemReader.java | 133 +++++++++++++++--- 16 files changed, 337 insertions(+), 227 deletions(-) create mode 100644 org.eclipse.jgit.lfs/.settings/.api_filters diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java index b26324d4f..78bf778cc 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java @@ -107,7 +107,6 @@ import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook; import org.eclipse.jgit.transport.AdvertiseRefsHook; import org.eclipse.jgit.transport.CredentialItem; @@ -127,7 +126,6 @@ import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.transport.resolver.UploadPackFactory; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.SystemReader; import org.hamcrest.Matchers; @@ -652,8 +650,8 @@ public void testInitialClone_RedirectMultiple() throws Exception { @Test public void testInitialClone_RedirectMax() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setInt("http", null, "maxRedirects", 4); userConfig.save(); initialClone_Redirect(4, 302); @@ -661,8 +659,8 @@ public void testInitialClone_RedirectMax() throws Exception { @Test public void testInitialClone_RedirectTooOften() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setInt("http", null, "maxRedirects", 3); userConfig.save(); Repository dst = createBareRepository(); @@ -701,8 +699,8 @@ public void testInitialClone_RedirectLoop() throws Exception { @Test public void testInitialClone_RedirectOnPostAllowed() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setString("http", null, "followRedirects", "true"); userConfig.save(); Repository dst = createBareRepository(); @@ -764,8 +762,8 @@ public void testInitialClone_RedirectOnPostForbidden() throws Exception { @Test public void testInitialClone_RedirectForbidden() throws Exception { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setString("http", null, "followRedirects", "false"); userConfig.save(); 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 f8f0c18cb..29579d007 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 @@ -137,14 +137,15 @@ public void setUp() throws Exception { // the same one here FS.getFileStoreAttributes(tmp.toPath().getParent()); - mockSystemReader.userGitConfig = new FileBasedConfig(new File(tmp, - "usergitconfig"), FS.DETECTED); + FileBasedConfig userConfig = new FileBasedConfig( + 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 // in the middle of writing or deleting files, which would disrupt this. - mockSystemReader.userGitConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, + userConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, null, ConfigConstants.CONFIG_KEY_AUTODETACH, false); - mockSystemReader.userGitConfig.save(); + userConfig.save(); + 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 92b531d19..123fdb305 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 @@ -60,6 +60,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.SystemReader; @@ -100,10 +101,36 @@ public String toString() { final Map values = new HashMap<>(); - FileBasedConfig userGitConfig; + private FileBasedConfig userGitConfig; FileBasedConfig systemGitConfig; + /** + * Set the user-level git config + * + * @param userGitConfig + * set another user-level git config + * @return the old user-level git config + */ + public FileBasedConfig setUserGitConfig(FileBasedConfig userGitConfig) { + FileBasedConfig old = this.userGitConfig; + this.userGitConfig = userGitConfig; + return old; + } + + /** + * Set the system-level git config + * + * @param systemGitConfig + * the new system-level git config + * @return the old system-level config + */ + public FileBasedConfig setSystemGitConfig(FileBasedConfig systemGitConfig) { + FileBasedConfig old = this.systemGitConfig; + this.systemGitConfig = systemGitConfig; + return old; + } + /** * Constructor for MockSystemReader */ @@ -166,6 +193,18 @@ public FileBasedConfig openSystemConfig(Config parent, FS fs) { return systemGitConfig; } + @Override + public StoredConfig getUserConfig() + throws IOException, ConfigInvalidException { + return userGitConfig; + } + + @Override + public StoredConfig getSystemConfig() + throws IOException, ConfigInvalidException { + return systemGitConfig; + } + /** {@inheritDoc} */ @Override public String getHostname() { diff --git a/org.eclipse.jgit.lfs/.settings/.api_filters b/org.eclipse.jgit.lfs/.settings/.api_filters new file mode 100644 index 000000000..db45ade67 --- /dev/null +++ b/org.eclipse.jgit.lfs/.settings/.api_filters @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java index 028b19b2a..b7b0535ea 100644 --- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java +++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/InstallBuiltinLfsCommand.java @@ -43,14 +43,12 @@ package org.eclipse.jgit.lfs; import java.io.IOException; -import java.text.MessageFormat; +import org.eclipse.jgit.api.errors.InvalidConfigurationException; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lfs.internal.LfsText; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.LfsFactory.LfsInstallCommand; import org.eclipse.jgit.util.SystemReader; @@ -70,12 +68,28 @@ public class InstallBuiltinLfsCommand implements LfsInstallCommand { private Repository repository; - /** {@inheritDoc} */ + /** + * {@inheritDoc} + * + * @throws IOException + * if an I/O error occurs while accessing a git config or + * executing {@code git lfs install} in an external process + * @throws InvalidConfigurationException + * if a git configuration is invalid + * @throws InterruptedException + * if the current thread is interrupted while waiting for the + * {@code git lfs install} executed in an external process + */ @Override - public Void call() throws Exception { + public Void call() throws IOException, InvalidConfigurationException, + InterruptedException { StoredConfig cfg = null; if (repository == null) { - cfg = loadUserConfig(); + try { + cfg = SystemReader.getInstance().getUserConfig(); + } catch (ConfigInvalidException e) { + throw new InvalidConfigurationException(e.getMessage(), e); + } } else { cfg = repository.getConfig(); } @@ -116,19 +130,4 @@ public LfsInstallCommand setRepository(Repository repo) { return this; } - private StoredConfig loadUserConfig() throws IOException { - FileBasedConfig c = SystemReader.getInstance().openUserConfig(null, - FS.DETECTED); - try { - c.load(); - } catch (ConfigInvalidException e1) { - throw new IOException(MessageFormat - .format(LfsText.get().userConfigInvalid, c.getFile() - .getAbsolutePath(), e1), - e1); - } - - return c; - } - } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index 0d7009dca..613ca5ce9 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -68,9 +68,9 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.submodule.SubmoduleStatus; import org.eclipse.jgit.submodule.SubmoduleStatusType; import org.eclipse.jgit.submodule.SubmoduleWalk; @@ -633,8 +633,8 @@ public void testCloneWithAutoSetupRebase() throws Exception { ConfigConstants.CONFIG_BRANCH_SECTION, "test", ConfigConstants.CONFIG_KEY_REBASE, null)); - FileBasedConfig userConfig = SystemReader.getInstance().openUserConfig( - null, git.getRepository().getFS()); + StoredConfig userConfig = SystemReader.getInstance() + .getUserConfig(); userConfig.setString(ConfigConstants.CONFIG_BRANCH_SECTION, null, ConfigConstants.CONFIG_KEY_AUTOSETUPREBASE, ConfigConstants.CONFIG_KEY_ALWAYS); 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 9683f93e6..87349a25a 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 @@ -43,48 +43,63 @@ package org.eclipse.jgit.util; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.jgit.junit.MockSystemReader; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class FS_POSIXTest { private SystemReader originalSystemReaderInstance; - private FileBasedConfig mockSystemConfig; + private FileBasedConfig systemConfig; - private FileBasedConfig mockUserConfig; + private FileBasedConfig userConfig; + + private Path tmp; @Before public void setUp() throws Exception { - SystemReader systemReader = Mockito.mock(SystemReader.class); + tmp = Files.createTempDirectory("jgit_test_"); + MockSystemReader mockSystemReader = new MockSystemReader(); + SystemReader.setInstance(mockSystemReader); + + // Measure timer resolution before the test to avoid time critical tests + // are affected by time needed for measurement. + // The MockSystemReader must be configured first since we need to use + // the same one here + FS.getFileStoreAttributes(tmp.getParent()); + systemConfig = new FileBasedConfig( + new File(tmp.toFile(), "systemgitconfig"), FS.DETECTED); + userConfig = new FileBasedConfig(systemConfig, + new File(tmp.toFile(), "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 in the middle of writing or deleting files, + // which would disrupt this. + userConfig.setBoolean(ConfigConstants.CONFIG_GC_SECTION, null, + ConfigConstants.CONFIG_KEY_AUTODETACH, false); + userConfig.save(); + mockSystemReader.setSystemGitConfig(systemConfig); + mockSystemReader.setUserGitConfig(userConfig); originalSystemReaderInstance = SystemReader.getInstance(); - SystemReader.setInstance(systemReader); - - mockSystemConfig = mock(FileBasedConfig.class); - mockUserConfig = mock(FileBasedConfig.class); - when(systemReader.openSystemConfig(any(), any())) - .thenReturn(mockSystemConfig); - when(systemReader.openUserConfig(any(), any())) - .thenReturn(mockUserConfig); - - when(mockSystemConfig.getString(ConfigConstants.CONFIG_CORE_SECTION, - null, ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION)) - .thenReturn(null); + SystemReader.setInstance(mockSystemReader); } @After - public void tearDown() { + public void tearDown() throws IOException { SystemReader.setInstance(originalSystemReaderInstance); + FileUtils.delete(tmp.toFile(), FileUtils.RECURSIVE | FileUtils.RETRY); } @Test @@ -94,32 +109,31 @@ public void supportsAtomicCreateNewFile_shouldReturnSupportedAsDefault() { @Test public void supportsAtomicCreateNewFile_shouldReturnTrueIfFlagIsSetInUserConfig() { - setAtomicCreateCreationFlag(mockUserConfig, "true"); + setAtomicCreateCreationFlag(userConfig, "true"); assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); } @Test public void supportsAtomicCreateNewFile_shouldReturnTrueIfFlagIsSetInSystemConfig() { - setAtomicCreateCreationFlag(mockSystemConfig, "true"); + setAtomicCreateCreationFlag(systemConfig, "true"); assertTrue(new FS_POSIX().supportsAtomicCreateNewFile()); } @Test public void supportsAtomicCreateNewFile_shouldReturnFalseIfFlagUnsetInUserConfig() { - setAtomicCreateCreationFlag(mockUserConfig, "false"); + setAtomicCreateCreationFlag(userConfig, "false"); assertFalse(new FS_POSIX().supportsAtomicCreateNewFile()); } @Test public void supportsAtomicCreateNewFile_shouldReturnFalseIfFlagUnsetInSystemConfig() { - setAtomicCreateCreationFlag(mockSystemConfig, "false"); + setAtomicCreateCreationFlag(systemConfig, "false"); assertFalse(new FS_POSIX().supportsAtomicCreateNewFile()); } private void setAtomicCreateCreationFlag(FileBasedConfig config, String value) { - when(config.getString(ConfigConstants.CONFIG_CORE_SECTION, null, - ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION)) - .thenReturn(value); + config.setString(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION, value); } } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 5874cd51f..f05afaf53 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -248,4 +248,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + 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 1579dc7a7..dc1db8140 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -17,6 +17,7 @@ applyingCommit=Applying {0} archiveFormatAlreadyAbsent=Archive format already absent: {0} archiveFormatAlreadyRegistered=Archive format already registered with different implementation: {0} argumentIsNotAValidCommentString=Invalid comment: {0} +assumeAtomicCreateNewFile=Reading option "core.supportsAtomicFileCreation" failed, fallback to default assuming atomic file creation is supported atLeastOnePathIsRequired=At least one path is required. atLeastOnePatternIsRequired=At least one pattern is required. atLeastTwoFiltersNeeded=At least two filters needed. @@ -562,6 +563,7 @@ pushNotPermitted=push not permitted pushOptionsNotSupported=Push options not supported; received {0} rawLogMessageDoesNotParseAsLogEntry=Raw log message does not parse as log entry readConfigFailed=Reading config file ''{0}'' failed +readFileStoreAttributesFailed=Reading FileStore attributes from user config failed readerIsRequired=Reader is required readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0} readLastModifiedFailed=Reading lastModified of {0} failed @@ -618,6 +620,7 @@ rewinding=Rewinding to commit {0} s3ActionDeletion=Deletion s3ActionReading=Reading s3ActionWriting=Writing +saveFileStoreAttributesFailed=Saving measured FileStore attributes to user config failed searchForReuse=Finding sources searchForSizes=Getting sizes secondsAgo={0} seconds ago @@ -777,7 +780,7 @@ uriNotConfigured=Submodule URI not configured uriNotFound={0} not found uriNotFoundWithMessage={0} not found: {1} URINotSupported=URI not supported: {0} -userConfigFileInvalid=User config file {0} invalid {1} +userConfigInvalid=Git config in the user's home directory {0} is invalid {1} walkFailure=Walk failure. wantNotValid=want {0} not valid weeksAgo={0} weeks ago 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 a6110e57d..4c6026624 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -78,6 +78,7 @@ public static JGitText get() { /***/ public String archiveFormatAlreadyAbsent; /***/ public String archiveFormatAlreadyRegistered; /***/ public String argumentIsNotAValidCommentString; + /***/ public String assumeAtomicCreateNewFile; /***/ public String atLeastOnePathIsRequired; /***/ public String atLeastOnePatternIsRequired; /***/ public String atLeastTwoFiltersNeeded; @@ -623,6 +624,7 @@ public static JGitText get() { /***/ public String pushOptionsNotSupported; /***/ public String rawLogMessageDoesNotParseAsLogEntry; /***/ public String readConfigFailed; + /***/ public String readFileStoreAttributesFailed; /***/ public String readerIsRequired; /***/ public String readingObjectsFromLocalRepositoryFailed; /***/ public String readLastModifiedFailed; @@ -679,6 +681,7 @@ public static JGitText get() { /***/ public String s3ActionDeletion; /***/ public String s3ActionReading; /***/ public String s3ActionWriting; + /***/ public String saveFileStoreAttributesFailed; /***/ public String searchForReuse; /***/ public String searchForSizes; /***/ public String secondsAgo; @@ -838,7 +841,7 @@ public static JGitText get() { /***/ public String uriNotFound; /***/ public String uriNotFoundWithMessage; /***/ public String URINotSupported; - /***/ public String userConfigFileInvalid; + /***/ public String userConfigInvalid; /***/ public String walkFailure; /***/ public String wantNotValid; /***/ public String weeksAgo; 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 5ced68646..356d64b56 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 @@ -81,15 +81,17 @@ import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.ReflogReader; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.storage.pack.PackConfig; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.SystemReader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Represents a Git repository. A repository holds all objects and refs used for @@ -116,10 +118,10 @@ * This implementation only handles a subtly undocumented subset of git features. */ public class FileRepository extends Repository { + private static final Logger LOG = LoggerFactory + .getLogger(FileRepository.class); private static final String UNNAMED = "Unnamed repository; edit this file to name it for gitweb."; //$NON-NLS-1$ - private final FileBasedConfig systemConfig; - private final FileBasedConfig userConfig; private final FileBasedConfig repoConfig; private final RefDatabase refs; private final ObjectDirectory objectDatabase; @@ -178,32 +180,16 @@ public FileRepository(String gitDir) throws IOException { */ public FileRepository(BaseRepositoryBuilder options) throws IOException { super(options); - - if (StringUtils.isEmptyOrNull(SystemReader.getInstance().getenv( - Constants.GIT_CONFIG_NOSYSTEM_KEY))) - systemConfig = SystemReader.getInstance().openSystemConfig(null, - getFS()); - else - systemConfig = new FileBasedConfig(null, FS.DETECTED) { - @Override - public void load() { - // empty, do not load - } - - @Override - public boolean isOutdated() { - // regular class would bomb here - return false; - } - }; - userConfig = SystemReader.getInstance().openUserConfig(systemConfig, - getFS()); + StoredConfig userConfig = null; + try { + userConfig = SystemReader.getInstance().getUserConfig(); + } catch (ConfigInvalidException e) { + LOG.error(e.getMessage(), e); + throw new IOException(e.getMessage(), e); + } repoConfig = new FileBasedConfig(userConfig, getFS().resolve( getDirectory(), Constants.CONFIG), getFS()); - - loadSystemConfig(); - loadUserConfig(); loadRepoConfig(); repoConfig.addChangeListener(new ConfigChangedListener() { @@ -247,28 +233,6 @@ public void onConfigChanged(ConfigChangedEvent event) { } } - private void loadSystemConfig() throws IOException { - try { - systemConfig.load(); - } catch (ConfigInvalidException e) { - throw new IOException(MessageFormat.format(JGitText - .get().systemConfigFileInvalid, systemConfig.getFile() - .getAbsolutePath(), - e), e); - } - } - - private void loadUserConfig() throws IOException { - try { - userConfig.load(); - } catch (ConfigInvalidException e) { - throw new IOException(MessageFormat.format(JGitText - .get().userConfigFileInvalid, userConfig.getFile() - .getAbsolutePath(), - e), e); - } - } - private void loadRepoConfig() throws IOException { try { repoConfig.load(); @@ -398,26 +362,13 @@ public RefDatabase getRefDatabase() { /** {@inheritDoc} */ @Override public FileBasedConfig getConfig() { - if (systemConfig.isOutdated()) { - try { - loadSystemConfig(); - } catch (IOException e) { - throw new RuntimeException(e); + try { + SystemReader.getInstance().getUserConfig(); + if (repoConfig.isOutdated()) { + loadRepoConfig(); } - } - if (userConfig.isOutdated()) { - try { - loadUserConfig(); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - if (repoConfig.isOutdated()) { - try { - loadRepoConfig(); - } catch (IOException e) { - throw new RuntimeException(e); - } + } catch (IOException | ConfigInvalidException e) { + throw new RuntimeException(e); } return repoConfig; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java index 101ce3568..ce9e1b3de 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java @@ -53,8 +53,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.storage.file.FileBasedConfig; -import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.util.StringUtils; import org.eclipse.jgit.util.SystemReader; import org.slf4j.Logger; @@ -210,14 +209,12 @@ public HttpConfig(Config config, URIish uri) { * to get the configuration values for */ public HttpConfig(URIish uri) { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = null; try { - userConfig.load(); + userConfig = SystemReader.getInstance().getUserConfig(); } catch (IOException | ConfigInvalidException e) { // Log it and then work with default values. - LOG.error(MessageFormat.format(JGitText.get().userConfigFileInvalid, - userConfig.getFile().getAbsolutePath(), e)); + LOG.error(e.getMessage(), e); init(new Config(), uri); return; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java index 8b41ab046..0df1b70cb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java @@ -108,11 +108,9 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.lib.SymbolicRef; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.transport.HttpAuthMethod.Type; import org.eclipse.jgit.transport.HttpConfig.HttpRedirectMode; import org.eclipse.jgit.transport.http.HttpConnection; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; @@ -715,15 +713,13 @@ private void updateSslVerify(StoredConfig config, boolean value) { } private void updateSslVerifyUser(boolean value) { - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig = null; try { - userConfig.load(); + userConfig = SystemReader.getInstance().getUserConfig(); updateSslVerify(userConfig, value); } catch (IOException | ConfigInvalidException e) { // Log it, but otherwise ignore here. - LOG.error(MessageFormat.format(JGitText.get().userConfigFileInvalid, - userConfig.getFile().getAbsolutePath(), e)); + LOG.error(e.getMessage(), e); } } 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 6efd02f47..7d37cfa65 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -510,18 +510,12 @@ private static void deleteProbe(Path probe) { private static Optional readFromConfig( FileStore s) { - StoredConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig; try { - userConfig.load(); - } catch (IOException e) { - LOG.error(MessageFormat.format(JGitText.get().readConfigFailed, - userConfig), e); - } catch (ConfigInvalidException e) { - LOG.error(MessageFormat.format( - JGitText.get().repositoryConfigFileInvalid, - userConfig, - e.getMessage())); + userConfig = SystemReader.getInstance().getUserConfig(); + } catch (IOException | ConfigInvalidException e) { + LOG.error(JGitText.get().readFileStoreAttributesFailed, e); + return Optional.empty(); } String key = getConfigKey(s); Duration resolution = Duration.ofNanos(userConfig.getTimeUnit( @@ -544,8 +538,13 @@ private static Optional readFromConfig( private static void saveToConfig(FileStore s, FileStoreAttributes c) { - StoredConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); + StoredConfig userConfig; + try { + userConfig = SystemReader.getInstance().getUserConfig(); + } catch (IOException | ConfigInvalidException e) { + LOG.error(JGitText.get().saveFileStoreAttributesFailed, e); + return; + } long resolution = c.getFsTimestampResolution().toNanos(); TimeUnit resolutionUnit = getUnit(resolution); long resolutionValue = resolutionUnit.convert(resolution, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java index 1c42f7857..eda8afb10 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java @@ -42,6 +42,9 @@ */ package org.eclipse.jgit.util; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_CORE_SECTION; +import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION; + import java.io.BufferedReader; import java.io.File; import java.io.IOException; @@ -67,10 +70,9 @@ import org.eclipse.jgit.errors.CommandFailedException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.lib.StoredConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,7 +89,7 @@ public class FS_POSIX extends FS { private volatile boolean supportsUnixNLink = true; - private volatile AtomicFileCreation supportsAtomicCreateNewFile = AtomicFileCreation.UNDEFINED; + private volatile AtomicFileCreation supportsAtomicFileCreation = AtomicFileCreation.UNDEFINED; private enum AtomicFileCreation { SUPPORTED, NOT_SUPPORTED, UNDEFINED @@ -112,42 +114,6 @@ protected FS_POSIX(FS src) { } } - private void determineAtomicFileCreationSupport() { - // @TODO: enhance SystemReader to support this without copying code - AtomicFileCreation ret = getAtomicFileCreationSupportOption( - SystemReader.getInstance().openUserConfig(null, this)); - if (ret == AtomicFileCreation.UNDEFINED - && StringUtils.isEmptyOrNull(SystemReader.getInstance() - .getenv(Constants.GIT_CONFIG_NOSYSTEM_KEY))) { - ret = getAtomicFileCreationSupportOption( - SystemReader.getInstance().openSystemConfig(null, this)); - } - - if (ret == AtomicFileCreation.UNDEFINED) { - ret = AtomicFileCreation.SUPPORTED; - } - supportsAtomicCreateNewFile = ret; - } - - private AtomicFileCreation getAtomicFileCreationSupportOption( - FileBasedConfig config) { - try { - config.load(); - String value = config.getString(ConfigConstants.CONFIG_CORE_SECTION, - null, - ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION); - if (value == null) { - return AtomicFileCreation.UNDEFINED; - } - return StringUtils.toBoolean(value) - ? AtomicFileCreation.SUPPORTED - : AtomicFileCreation.NOT_SUPPORTED; - } catch (IOException | ConfigInvalidException e) { - LOG.error(e.getMessage(), e); - return AtomicFileCreation.UNDEFINED; - } - } - /** {@inheritDoc} */ @Override public FS newInstance() { @@ -362,10 +328,24 @@ public File findHook(Repository repository, String hookName) { /** {@inheritDoc} */ @Override public boolean supportsAtomicCreateNewFile() { - if (supportsAtomicCreateNewFile == AtomicFileCreation.UNDEFINED) { - determineAtomicFileCreationSupport(); + if (supportsAtomicFileCreation == AtomicFileCreation.UNDEFINED) { + try { + StoredConfig config = SystemReader.getInstance().getUserConfig(); + String value = config.getString(CONFIG_CORE_SECTION, null, + CONFIG_KEY_SUPPORTSATOMICFILECREATION); + if (value != null) { + supportsAtomicFileCreation = StringUtils.toBoolean(value) + ? AtomicFileCreation.SUPPORTED + : AtomicFileCreation.NOT_SUPPORTED; + } else { + supportsAtomicFileCreation = AtomicFileCreation.SUPPORTED; + } + } catch (IOException | ConfigInvalidException e) { + LOG.warn(JGitText.get().assumeAtomicCreateNewFile, e); + supportsAtomicFileCreation = AtomicFileCreation.SUPPORTED; + } } - return supportsAtomicCreateNewFile == AtomicFileCreation.SUPPORTED; + return supportsAtomicFileCreation == AtomicFileCreation.SUPPORTED; } @Override 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 b61a6f191..d554562a7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SystemReader.java @@ -47,6 +47,7 @@ package org.eclipse.jgit.util; import java.io.File; +import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; import java.security.AccessController; @@ -56,12 +57,17 @@ import java.util.Locale; import java.util.TimeZone; +import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectChecker; +import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.time.MonotonicClock; import org.eclipse.jgit.util.time.MonotonicSystemClock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Interface to read values from the system. @@ -72,6 +78,10 @@ *

*/ public abstract class SystemReader { + + private final static Logger LOG = LoggerFactory + .getLogger(SystemReader.class); + private static final SystemReader DEFAULT; private static Boolean isMacOS; @@ -87,6 +97,10 @@ public abstract class SystemReader { private static class Default extends SystemReader { private volatile String hostname; + private volatile FileBasedConfig systemConfig; + + private volatile FileBasedConfig userConfig; + @Override public String getenv(String variable) { return System.getenv(variable); @@ -99,28 +113,69 @@ public String getProperty(String key) { @Override public FileBasedConfig openSystemConfig(Config parent, FS fs) { - File configFile = fs.getGitSystemConfig(); - if (configFile == null) { - return new FileBasedConfig(null, fs) { - @Override - public void load() { - // empty, do not load - } - - @Override - public boolean isOutdated() { - // regular class would bomb here - return false; - } - }; + if (systemConfig == null) { + systemConfig = createSystemConfig(parent, fs); } - return new FileBasedConfig(parent, configFile, fs); + return systemConfig; + } + + protected FileBasedConfig createSystemConfig(Config parent, FS fs) { + if (StringUtils.isEmptyOrNull(getenv(Constants.GIT_CONFIG_NOSYSTEM_KEY))) { + File configFile = fs.getGitSystemConfig(); + if (configFile != null) { + return new FileBasedConfig(parent, configFile, fs); + } + } + return new FileBasedConfig(null, fs) { + @Override + public void load() { + // empty, do not load + } + + @Override + public boolean isOutdated() { + // regular class would bomb here + return false; + } + }; } @Override public FileBasedConfig openUserConfig(Config parent, FS fs) { - final File home = fs.userHome(); - return new FileBasedConfig(parent, new File(home, ".gitconfig"), fs); //$NON-NLS-1$ + if (userConfig == null) { + File home = fs.userHome(); + userConfig = new FileBasedConfig(parent, + new File(home, ".gitconfig"), fs); //$NON-NLS-1$ + } + return userConfig; + } + + @Override + public StoredConfig getSystemConfig() + throws IOException, ConfigInvalidException { + if (systemConfig == null) { + systemConfig = createSystemConfig(null, FS.DETECTED); + } + if (systemConfig.isOutdated()) { + LOG.debug("loading system config {}", systemConfig); //$NON-NLS-1$ + systemConfig.load(); + } + return systemConfig; + } + + @Override + public StoredConfig getUserConfig() + throws IOException, ConfigInvalidException { + if (userConfig == null) { + userConfig = openUserConfig(getSystemConfig(), FS.DETECTED); + } else { + getSystemConfig(); + } + if (userConfig.isOutdated()) { + LOG.debug("loading user config {}", userConfig); //$NON-NLS-1$ + userConfig.load(); + } + return userConfig; } @Override @@ -149,7 +204,7 @@ public int getTimezone(long when) { } } - private static SystemReader INSTANCE = DEFAULT; + private static volatile SystemReader INSTANCE = DEFAULT; /** * Get the current SystemReader instance @@ -225,7 +280,10 @@ protected final void setPlatformChecker() { public abstract String getProperty(String key); /** - * Open the git configuration found in the user home + * Open the git configuration found in the user home. Use + * {@link #getUserConfig()} to get the current git configuration in the user + * home since it manages automatic reloading when the gitconfig file was + * modified and avoids unnecessary reloads. * * @param parent * a config with values not found directly in the returned config @@ -237,7 +295,10 @@ protected final void setPlatformChecker() { public abstract FileBasedConfig openUserConfig(Config parent, FS fs); /** - * Open the gitconfig configuration found in the system-wide "etc" directory + * Open the gitconfig configuration found in the system-wide "etc" + * directory. Use {@link #getSystemConfig()} to get the current system-wide + * git configuration since it manages automatic reloading when the gitconfig + * file was modified and avoids unnecessary reloads. * * @param parent * a config with values not found directly in the returned @@ -250,6 +311,38 @@ protected final void setPlatformChecker() { */ public abstract FileBasedConfig openSystemConfig(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 + * reloads the system config if the system config file was modified. If the + * configuration file wasn't modified returns the cached configuration. + * + * @return the git configuration found in the user home + * @throws ConfigInvalidException + * if configuration is invalid + * @throws IOException + * if something went wrong when reading files + * @since 5.1.9 + */ + public abstract StoredConfig getUserConfig() + throws IOException, ConfigInvalidException; + + /** + * Get the gitconfig configuration found in the system-wide "etc" directory. + * The configuration will be reloaded automatically if the configuration + * file was modified otherwise returns the cached system level config. + * + * @return the gitconfig configuration found in the system-wide "etc" + * directory + * @throws ConfigInvalidException + * if configuration is invalid + * @throws IOException + * if something went wrong when reading files + * @since 5.1.9 + */ + public abstract StoredConfig getSystemConfig() + throws IOException, ConfigInvalidException; + /** * Get the current system time *