Reduce calls to Repository.getConfig

Each time getConfig() is called on FileRepository, it checks the
last modified time of both ~/.gitconfig and $GIT_DIR?config.  If
$GIT_DIR/config appears to have been modified, it is read back in
from disk and the current config is wiped out.

When mutating a configuration file, this may cause in-memory edits
to disappear.  To avoid that callers need to avoid calling getConfig
until after the configuration has been saved to disk.

Unfortunately the API is still horribly broken.  Configuration should
be modified only while a lock is held on the configuration file, very
similar to the way a ref is updated via its locking protocol.  But our
existing API is really broken for that so we'll have to defer cleaning
up the edit path for a future change.

Change-Id: I5888dd97bac20ddf60456c81ffc1eb8df04ef410
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

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 491094ca6ddc6858b69635c9255db6163311c1bd..f73f54ffbaa1ed952acaab0f68783b717cb626f3 100644 (file)
--- 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
@@ -500,9 +500,10 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 
                enableReceivePack();
 
-               db.getConfig().setInt("core", null, "compression", 0);
-               db.getConfig().setInt("http", null, "postbuffer", 8 * 1024);
-               db.getConfig().save();
+               final FileBasedConfig cfg = db.getConfig();
+               cfg.setInt("core", null, "compression", 0);
+               cfg.setInt("http", null, "postbuffer", 8 * 1024);
+               cfg.save();
 
                t = Transport.open(db, remoteURI);
                try {

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
index 0bc72a71e1f485a91abfebeb8362859f74e6ee5c..8eccdedc3f4c8210d5a24ba5fb3f218d78ad6298 100644 (file)
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
@@ -65,6 +65,7 @@ import org.eclipse.jgit.lib.Tree;
 import org.eclipse.jgit.lib.WorkDirCheckout;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.storage.file.FileRepository;
 import org.eclipse.jgit.transport.FetchResult;
 import org.eclipse.jgit.transport.RefSpec;
@@ -110,8 +111,9 @@ class Clone extends AbstractFetchCommand {
 
                dst = new FileRepository(gitdir);
                dst.create();
-               dst.getConfig().setBoolean("core", null, "bare", false);
-               dst.getConfig().save();
+               final FileBasedConfig dstcfg = dst.getConfig();
+               dstcfg.setBoolean("core", null, "bare", false);
+               dstcfg.save();
                db = dst;
 
                out.print(MessageFormat.format(
@@ -128,13 +130,14 @@ class Clone extends AbstractFetchCommand {
 
        private void saveRemote(final URIish uri) throws URISyntaxException,
                        IOException {
-               final RemoteConfig rc = new RemoteConfig(dst.getConfig(), remoteName);
+               final FileBasedConfig dstcfg = dst.getConfig();
+               final RemoteConfig rc = new RemoteConfig(dstcfg, remoteName);
                rc.addURI(uri);
                rc.addFetchRefSpec(new RefSpec().setForceUpdate(true)
                                .setSourceDestination(Constants.R_HEADS + "*",
                                                Constants.R_REMOTES + remoteName + "/*"));
-               rc.update(dst.getConfig());
-               dst.getConfig().save();
+               rc.update(dstcfg);
+               dstcfg.save();
        }
 
        private FetchResult runFetch() throws NotSupportedException,

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java
index 2057020cdf6eb06bcec94b2d91500705a35c3d57..6044f2599a95ba6fbf7af21ca4b36879f97b6f35 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/T0003_Basic.java
@@ -182,9 +182,9 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
                workdir.mkdir();
                FileRepository repo1initial = new FileRepository(new File(repo1Parent, Constants.DOT_GIT));
                repo1initial.create();
-               repo1initial.getConfig().setString("core", null, "worktree",
-                               workdir.getAbsolutePath());
-               repo1initial.getConfig().save();
+               final FileBasedConfig cfg = repo1initial.getConfig();
+               cfg.setString("core", null, "worktree", workdir.getAbsolutePath());
+               cfg.save();
                repo1initial.close();
 
                File theDir = new File(repo1Parent, Constants.DOT_GIT);
@@ -207,9 +207,9 @@ public class T0003_Basic extends SampleDataRepositoryTestCase {
                workdir.mkdir();
                FileRepository repo1initial = new FileRepository(new File(repo1Parent, Constants.DOT_GIT));
                repo1initial.create();
-               repo1initial.getConfig()
-                               .setString("core", null, "worktree", "../../rw");
-               repo1initial.getConfig().save();
+               final FileBasedConfig cfg = repo1initial.getConfig();
+               cfg.setString("core", null, "worktree", "../../rw");
+               cfg.save();
                repo1initial.close();
 
                File theDir = new File(repo1Parent, Constants.DOT_GIT);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java
index fa00581d070545051e46f5b6df1cc7324f213c49..a88dd2a7e00d71af6e59a8968691b48e302ee6d9 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/DeleteBranchCommand.java
@@ -59,6 +59,7 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -156,10 +157,11 @@ public class DeleteBranchCommand extends GitCommand<List<String>> {
                                                String shortenedName = fullName
                                                                .substring(Constants.R_HEADS.length());
                                                // remove upstream configuration if any
-                                               repo.getConfig().unsetSection(
+                                               final StoredConfig cfg = repo.getConfig();
+                                               cfg.unsetSection(
                                                                ConfigConstants.CONFIG_BRANCH_SECTION,
                                                                shortenedName);
-                                               repo.getConfig().save();
+                                               cfg.save();
                                        }
                                } else
                                        throw new JGitInternalException(MessageFormat.format(

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java
index 4fe050b2a30dc24510172ac8167207848f788a2c..adcea90f560cb69a2bacc2308b36e182bfb1d8da 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java
@@ -186,7 +186,7 @@ public class PullCommand extends GitCommand<PullResult> {
                String remoteUri;
                FetchResult fetchRes;
                if (isRemote) {
-                       remoteUri = repo.getConfig().getString("remote", remote,
+                       remoteUri = repoConfig.getString("remote", remote,
                                        ConfigConstants.CONFIG_KEY_URL);
                        if (remoteUri == null) {
                                String missingKey = ConfigConstants.CONFIG_REMOTE_SECTION + DOT

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java
index 4654981329a899dba85d5f699ff0a953253eb728..04b7791d6657d1e9df5a655a7bfcbff5227137c5 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java
@@ -58,6 +58,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefRename;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.lib.RefUpdate.Result;
 
 /**
@@ -144,27 +145,28 @@ public class RenameBranchCommand extends GitCommand<Ref> {
                                        // move the upstream configuration over to the new branch
                                        String shortOldName = fullOldName
                                                        .substring(Constants.R_HEADS.length());
-                                       String oldRemote = repo.getConfig().getString(
+                                       final StoredConfig repoConfig = repo.getConfig();
+                                       String oldRemote = repoConfig.getString(
                                                        ConfigConstants.CONFIG_BRANCH_SECTION,
                                                        shortOldName, ConfigConstants.CONFIG_KEY_REMOTE);
                                        if (oldRemote != null) {
-                                               repo.getConfig().setString(
+                                               repoConfig.setString(
                                                                ConfigConstants.CONFIG_BRANCH_SECTION, newName,
                                                                ConfigConstants.CONFIG_KEY_REMOTE, oldRemote);
                                        }
-                                       String oldMerge = repo.getConfig().getString(
+                                       String oldMerge = repoConfig.getString(
                                                        ConfigConstants.CONFIG_BRANCH_SECTION,
                                                        shortOldName, ConfigConstants.CONFIG_KEY_MERGE);
                                        if (oldMerge != null) {
-                                               repo.getConfig().setString(
+                                               repoConfig.setString(
                                                                ConfigConstants.CONFIG_BRANCH_SECTION, newName,
                                                                ConfigConstants.CONFIG_KEY_MERGE, oldMerge);
                                        }
-                                       repo.getConfig()
+                                       repoConfig
                                                        .unsetSection(
                                                                        ConfigConstants.CONFIG_BRANCH_SECTION,
                                                                        shortOldName);
-                                       repo.getConfig().save();
+                                       repoConfig.save();
                                }
 
                        } else

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java
index a145e7725cceeea46e216c0b57149008372a0ffa..417fa4b53ee417c5c718867bf52ddde85982175d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java
@@ -160,7 +160,7 @@ public class FileRepository extends Repository {
                loadUserConfig();
                loadRepoConfig();
 
-               getConfig().addChangeListener(new ConfigChangedListener() {
+               repoConfig.addChangeListener(new ConfigChangedListener() {
                        public void onConfigChanged(ConfigChangedEvent event) {
                                fireEvent(event);
                        }