* [JGIT PATCH 00/19] More Config class cleanup work @ 2009-07-25 18:52 Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 01/19] Cleanup nonstandard references to encoding strings to bytes Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git, Constantine Plotnikov Yet another series to cleanup the Config class and its implementations to be slightly more useful. The later part of the series makes Config thread safe and introduces caching of application-specific model instances as part of the Config, making it more efficient for code to query for multiple values at once. This new caching feature is especially useful for ReceivePack on the server side, where the values aren't expected to change between connections, but connections come in at a steady enough rate that reparsing the configuration each time is just a waste of time. Later I plan to extend the caching by using it RemoteConfig and also for a new SubmoduleConfig. The entire series applies on top of my 1 patch from yesterday that fixes the Turkish locale problems in Config. Shawn O. Pearce (19): Cleanup nonstandard references to encoding strings to bytes Delete incorrect Javadoc from Config's getRawString method Make Config.escapeValue a private method Allow a RemoteConfig to use the more generic Config class Use type specific sets when creating a new RepositoryConfig Move SystemReader out of RepositoryConfig Correct user config to be of type FileBasedConfig Extract the test specific SystemReader out of RepositoryTestCase Refactor Config hierarchy to make IO more explicit Test for the config file when creating a new repository Remove the map lookup for values in Config Return base values first from Config.getStringList() Make Config thread safe by using copy-on-write semantics Support cached application models in a Config Cache Config subsection names when requested by application code Refactor author/committer lookup to use cached data Move repository config creation fully into Repository class Use Config SectionParser cache to store daemon enable states Use Config cache for fetch and receive configuration parsing .../org/spearce/jgit/lib/ConcurrentRepackTest.java | 2 +- .../tst/org/spearce/jgit/lib/MockSystemReader.java | 78 ++ .../org/spearce/jgit/lib/RepositoryConfigTest.java | 193 +++--- .../org/spearce/jgit/lib/RepositoryTestCase.java | 51 +-- .../tst/org/spearce/jgit/lib/T0003_Basic.java | 16 +- .../org/spearce/jgit/revwalk/RevWalkTestCase.java | 3 +- .../spearce/jgit/transport/RemoteConfigTest.java | 166 ++--- .../jgit/errors/ConfigInvalidException.java | 53 ++ .../src/org/spearce/jgit/lib/BlobBasedConfig.java | 110 ++-- .../src/org/spearce/jgit/lib/Config.java | 743 +++++++++++--------- .../src/org/spearce/jgit/lib/CoreConfig.java | 16 +- .../src/org/spearce/jgit/lib/FileBasedConfig.java | 83 ++- .../src/org/spearce/jgit/lib/ObjectWriter.java | 2 +- .../src/org/spearce/jgit/lib/Repository.java | 38 +- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 144 +---- .../src/org/spearce/jgit/lib/TransferConfig.java | 11 +- .../src/org/spearce/jgit/lib/UserConfig.java | 149 ++++ .../jgit/revwalk/filter/PatternMatchRevFilter.java | 10 +- .../jgit/transport/BasePackFetchConnection.java | 21 +- .../org/spearce/jgit/transport/DaemonService.java | 30 +- .../org/spearce/jgit/transport/ReceivePack.java | 45 +- .../org/spearce/jgit/transport/RemoteConfig.java | 15 +- .../org/spearce/jgit/util/RawSubStringPattern.java | 10 +- .../src/org/spearce/jgit/util/SystemReader.java | 65 ++- 24 files changed, 1172 insertions(+), 882 deletions(-) create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/UserConfig.java ^ permalink raw reply [flat|nested] 26+ messages in thread
* [JGIT PATCH 01/19] Cleanup nonstandard references to encoding strings to bytes 2009-07-25 18:52 [JGIT PATCH 00/19] More Config class cleanup work Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/lib/ConcurrentRepackTest.java | 2 +- .../org/spearce/jgit/revwalk/RevWalkTestCase.java | 3 +-- .../src/org/spearce/jgit/lib/ObjectWriter.java | 2 +- .../jgit/revwalk/filter/PatternMatchRevFilter.java | 10 ++-------- .../org/spearce/jgit/util/RawSubStringPattern.java | 10 ++-------- 5 files changed, 7 insertions(+), 20 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java index fa6345e..bf155cf 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java @@ -244,7 +244,7 @@ private File fullPackFileName(final ObjectId name, final String suffix) { private RevObject writeBlob(final Repository repo, final String data) throws IOException { final RevWalk revWalk = new RevWalk(repo); - final byte[] bytes = data.getBytes(Constants.CHARACTER_ENCODING); + final byte[] bytes = Constants.encode(data); final ObjectWriter ow = new ObjectWriter(repo); final ObjectId id = ow.writeBlob(bytes); try { diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java index befc3d5..9d5a44c 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevWalkTestCase.java @@ -82,8 +82,7 @@ protected void tick(final int secDelta) { } protected RevBlob blob(final String content) throws Exception { - return rw.lookupBlob(ow.writeBlob(content - .getBytes(Constants.CHARACTER_ENCODING))); + return rw.lookupBlob(ow.writeBlob(Constants.encode(content))); } protected DirCacheEntry file(final String path, final RevBlob blob) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectWriter.java index 546cc68..86b5b09 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectWriter.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectWriter.java @@ -215,7 +215,7 @@ public ObjectId writeCommit(final Commit c) throws IOException { w.flush(); os.write('\n'); - if (!encoding.equals("UTF-8")) { + if (!encoding.equals(Constants.CHARACTER_ENCODING)) { os.write(hencoding); os.write(' '); os.write(Constants.encodeASCII(encoding)); diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/PatternMatchRevFilter.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/PatternMatchRevFilter.java index e0bccf7..f9e7e4a 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/PatternMatchRevFilter.java +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/filter/PatternMatchRevFilter.java @@ -38,12 +38,12 @@ package org.spearce.jgit.revwalk.filter; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.spearce.jgit.errors.IncorrectObjectTypeException; import org.spearce.jgit.errors.MissingObjectException; +import org.spearce.jgit.lib.Constants; import org.spearce.jgit.revwalk.RevCommit; import org.spearce.jgit.revwalk.RevWalk; import org.spearce.jgit.util.RawCharSequence; @@ -64,13 +64,7 @@ * character sequence {@link RawCharSequence}. */ protected static final String forceToRaw(final String patternText) { - final byte[] b; - try { - b = patternText.getBytes("UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new IllegalStateException("JVM lacks UTF-8 support.", e); - } - + final byte[] b = Constants.encode(patternText); final StringBuilder needle = new StringBuilder(b.length); for (int i = 0; i < b.length; i++) needle.append((char) (b[i] & 0xff)); diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java index 5ed071c..1628deb 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java +++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java @@ -37,7 +37,7 @@ package org.spearce.jgit.util; -import java.io.UnsupportedEncodingException; +import org.spearce.jgit.lib.Constants; /** * Searches text using only substring search. @@ -63,13 +63,7 @@ public RawSubStringPattern(final String patternText) { throw new IllegalArgumentException("Cannot match on empty string."); needleString = patternText; - final byte[] b; - try { - b = patternText.getBytes("UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new IllegalStateException("JVM lacks UTF-8 support.", e); - } - + final byte[] b = Constants.encode(patternText); needle = new byte[b.length]; for (int i = 0; i < b.length; i++) needle[i] = lc(b[i]); -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method 2009-07-25 18:52 ` [JGIT PATCH 01/19] Cleanup nonstandard references to encoding strings to bytes Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 03/19] Make Config.escapeValue a private method Shawn O. Pearce 2009-07-25 20:32 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Robin Rosenberg 0 siblings, 2 replies; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git I don't know how this Javadoc got here, but it predates the code refactor done by Constantine Plotnikov in 2564768e63. The documentation is incorrect, as the method returns a single string but the summary line says a list. Since we usually don't document a private method I'm removing the documentation block rather than correcting it. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Config.java | 11 ----------- 1 files changed, 0 insertions(+), 11 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index 258dba5..76f780f 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -385,17 +385,6 @@ public String getString(final String section, String subsection, return result; } - /** - * Get a list of string values - * - * @param section - * the section - * @param subsection - * the subsection for the value - * @param name - * the key name - * @return a raw string value as it is stored in configuration file - */ private String getRawString(final String section, final String subsection, final String name) { final Object o = getRawEntry(section, subsection, name); -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 03/19] Make Config.escapeValue a private method 2009-07-25 18:52 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 04/19] Allow a RemoteConfig to use the more generic Config class Shawn O. Pearce 2009-07-25 20:32 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Robin Rosenberg 1 sibling, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git This method is only used by toText() as it generates the value for an entry in the file. We don't want (or need) to export it to our subclasses. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Config.java | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index 76f780f..a2f5c6a 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -115,7 +115,7 @@ protected void setFileRead(boolean ok) { * the value to escape * @return the escaped value */ - protected static String escapeValue(final String x) { + private static String escapeValue(final String x) { boolean inquote = false; int lineStart = 0; final StringBuffer r = new StringBuffer(x.length()); -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 04/19] Allow a RemoteConfig to use the more generic Config class 2009-07-25 18:52 ` [JGIT PATCH 03/19] Make Config.escapeValue a private method Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 05/19] Use type specific sets when creating a new RepositoryConfig Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git A RemoteConfig can be parsed from any Config type object, not just the RepositoryConfig object. This change makes it easier to use other types of Config storage, such as in unit tests. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/transport/RemoteConfig.java | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java index 93a5873..ca84acf 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java @@ -43,6 +43,7 @@ import java.util.Collections; import java.util.List; +import org.spearce.jgit.lib.Config; import org.spearce.jgit.lib.RepositoryConfig; /** @@ -143,7 +144,7 @@ * @throws URISyntaxException * one of the URIs within the remote's configuration is invalid. */ - public RemoteConfig(final RepositoryConfig rc, final String remoteName) + public RemoteConfig(final Config rc, final String remoteName) throws URISyntaxException { name = remoteName; @@ -192,7 +193,7 @@ public RemoteConfig(final RepositoryConfig rc, final String remoteName) * @param rc * the configuration file to store ourselves into. */ - public void update(final RepositoryConfig rc) { + public void update(final Config rc) { final List<String> vlst = new ArrayList<String>(); vlst.clear(); @@ -222,7 +223,7 @@ public void update(final RepositoryConfig rc) { set(rc, KEY_TIMEOUT, timeout, 0); } - private void set(final RepositoryConfig rc, final String key, + private void set(final Config rc, final String key, final String currentValue, final String defaultValue) { if (defaultValue.equals(currentValue)) unset(rc, key); @@ -230,7 +231,7 @@ private void set(final RepositoryConfig rc, final String key, rc.setString(SECTION, getName(), key, currentValue); } - private void set(final RepositoryConfig rc, final String key, + private void set(final Config rc, final String key, final boolean currentValue, final boolean defaultValue) { if (defaultValue == currentValue) unset(rc, key); @@ -238,15 +239,15 @@ private void set(final RepositoryConfig rc, final String key, rc.setBoolean(SECTION, getName(), key, currentValue); } - private void set(final RepositoryConfig rc, final String key, - final int currentValue, final int defaultValue) { + private void set(final Config rc, final String key, final int currentValue, + final int defaultValue) { if (defaultValue == currentValue) unset(rc, key); else rc.setInt(SECTION, getName(), key, currentValue); } - private void unset(final RepositoryConfig rc, final String key) { + private void unset(final Config rc, final String key) { rc.unset(SECTION, getName(), key); } -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 05/19] Use type specific sets when creating a new RepositoryConfig 2009-07-25 18:52 ` [JGIT PATCH 04/19] Allow a RemoteConfig to use the more generic Config class Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 06/19] Move SystemReader out of RepositoryConfig Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Repository.java | 5 ++--- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java index 1076fe1..98a276b 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java @@ -166,9 +166,8 @@ public void create(boolean bare) throws IOException { RepositoryConfig cfg = getConfig(); cfg.create(); - if (bare) { - cfg.setString("core", null, "bare", "true"); - } + if (bare) + cfg.setBoolean("core", null, "bare", true); cfg.save(); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index 85e8738..c80db00 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -203,8 +203,8 @@ private String getUserEmailInternal(String gitVariableKey) { public void create() { clear(); setFileRead(true); - setString("core", null, "repositoryformatversion", "0"); - setString("core", null, "filemode", "true"); + setInt("core", null, "repositoryformatversion", 0); + setBoolean("core", null, "filemode", true); core = new CoreConfig(this); transfer = new TransferConfig(this); -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 06/19] Move SystemReader out of RepositoryConfig 2009-07-25 18:52 ` [JGIT PATCH 05/19] Use type specific sets when creating a new RepositoryConfig Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 07/19] Correct user config to be of type FileBasedConfig Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Reading basic properties of the JVM has nothing to do with reading a Git style configuration file for a repository, or for the current user account. Instead pull all of that logic into its own abstract class, and provide a default implementation available through a singleton pattern. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/lib/RepositoryConfigTest.java | 11 +--- .../org/spearce/jgit/lib/RepositoryTestCase.java | 9 ++- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 50 +-------------- .../src/org/spearce/jgit/util/SystemReader.java | 64 ++++++++++++++++++- 4 files changed, 72 insertions(+), 62 deletions(-) diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java index 5e2328b..5bb9afb 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java @@ -42,8 +42,6 @@ import java.io.File; import java.io.IOException; -import java.net.InetAddress; -import java.net.UnknownHostException; import java.util.Arrays; import java.util.LinkedList; @@ -116,14 +114,7 @@ public void test006_readCaseInsensitive() throws IOException { } public void test007_readUserInfos() throws IOException { - String hostname; - try { - InetAddress localMachine = InetAddress.getLocalHost(); - hostname = localMachine.getCanonicalHostName(); - } catch (UnknownHostException e) { - hostname = "localhost"; - } - + final String hostname = FAKE_HOSTNAME; final File localConfig = writeTrashFile("local.config", ""); System.clearProperty(Constants.OS_USER_NAME_KEY); diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java index 2783180..9dfaeef 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java @@ -81,6 +81,8 @@ protected static final PersonIdent jcommitter; + protected static final String FAKE_HOSTNAME = "fake.host.example.com"; + static { jauthor = new PersonIdent("J. Author", "jauthor@example.com"); jcommitter = new PersonIdent("J. Committer", "jcommitter@example.com"); @@ -88,7 +90,7 @@ protected boolean packedGitMMAP; - protected static class FakeSystemReader implements SystemReader { + protected static class FakeSystemReader extends SystemReader { Map<String, String> values = new HashMap<String, String>(); RepositoryConfig userGitConfig; public String getenv(String variable) { @@ -103,6 +105,9 @@ public RepositoryConfig openUserConfig() { public void setUserGitConfig(RepositoryConfig userGitConfig) { this.userGitConfig = userGitConfig; } + public String getHostname() { + return FAKE_HOSTNAME; + } } /** @@ -114,7 +119,7 @@ public void setUserGitConfig(RepositoryConfig userGitConfig) { static { fakeSystemReader = new FakeSystemReader(); - RepositoryConfig.setSystemReader(fakeSystemReader); + SystemReader.setInstance(fakeSystemReader); } /** diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index c80db00..9be7c1b 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -43,8 +43,6 @@ import java.io.File; import java.io.IOException; -import java.net.InetAddress; -import java.net.UnknownHostException; import org.spearce.jgit.util.FS; import org.spearce.jgit.util.SystemReader; @@ -63,7 +61,7 @@ * configuration file from their home directory. */ public static RepositoryConfig openUserConfig() { - return systemReader.openUserConfig(); + return SystemReader.getInstance().openUserConfig(); } /** Section name for a branch configuration. */ @@ -73,21 +71,6 @@ public static RepositoryConfig openUserConfig() { TransferConfig transfer; - private static String hostname; - - // default system reader gets the value from the system - private static SystemReader systemReader = new SystemReader() { - public String getenv(String variable) { - return System.getenv(variable); - } - public String getProperty(String key) { - return System.getProperty(key); - } - public RepositoryConfig openUserConfig() { - return new RepositoryConfig(null, new File(FS.userHome(), ".gitconfig")); - } - }; - RepositoryConfig(final Repository repo) { this(openUserConfig(), FS.resolve(repo.getDirectory(), "config")); } @@ -139,6 +122,7 @@ public String getCommitterName() { } private String getUsernameInternal(String gitVariableKey) { + SystemReader systemReader = SystemReader.getInstance(); // try to get the user name from the local and global configurations. String username = getString("user", null, "name"); @@ -177,6 +161,7 @@ public String getCommitterEmail() { } private String getUserEmailInternal(String gitVariableKey) { + SystemReader systemReader = SystemReader.getInstance(); // try to get the email from the local and global configurations. String email = getString("user", null, "email"); @@ -191,7 +176,7 @@ private String getUserEmailInternal(String gitVariableKey) { if (username == null){ username = Constants.UNKNOWN_USER_DEFAULT; } - email = username + "@" + getHostname(); + email = username + "@" + systemReader.getHostname(); } return email; @@ -216,31 +201,4 @@ public void load() throws IOException { core = new CoreConfig(this); transfer = new TransferConfig(this); } - - /** - * Gets the hostname of the local host. - * If no hostname can be found, the hostname is set to the default value "localhost". - * @return the canonical hostname - */ - private static String getHostname() { - if (hostname == null) { - try { - InetAddress localMachine = InetAddress.getLocalHost(); - hostname = localMachine.getCanonicalHostName(); - } catch (UnknownHostException e) { - // we do nothing - hostname = "localhost"; - } - assert hostname != null; - } - return hostname; - } - - /** - * Overrides the default system reader by a custom one. - * @param newSystemReader new system reader - */ - public static void setSystemReader(SystemReader newSystemReader) { - systemReader = newSystemReader; - } } diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java b/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java index 32c2e20..36c188c 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java +++ b/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java @@ -37,6 +37,10 @@ package org.spearce.jgit.util; +import java.io.File; +import java.net.InetAddress; +import java.net.UnknownHostException; + import org.spearce.jgit.lib.RepositoryConfig; /** @@ -47,21 +51,73 @@ * permits to control the user's global configuration. * </p> */ -public interface SystemReader { +public abstract class SystemReader { + private static SystemReader INSTANCE = new SystemReader() { + private volatile String hostname; + + public String getenv(String variable) { + return System.getenv(variable); + } + + public String getProperty(String key) { + return System.getProperty(key); + } + + public RepositoryConfig openUserConfig() { + final File home = FS.userHome(); + return new RepositoryConfig(null, new File(home, ".gitconfig")); + } + + public String getHostname() { + if (hostname == null) { + try { + InetAddress localMachine = InetAddress.getLocalHost(); + hostname = localMachine.getCanonicalHostName(); + } catch (UnknownHostException e) { + // we do nothing + hostname = "localhost"; + } + assert hostname != null; + } + return hostname; + } + }; + + /** @return the live instance to read system properties. */ + public static SystemReader getInstance() { + return INSTANCE; + } + + /** + * @param newReader + * the new instance to use when accessing properties. + */ + public static void setInstance(SystemReader newReader) { + INSTANCE = newReader; + } + + /** + * Gets the hostname of the local host. If no hostname can be found, the + * hostname is set to the default value "localhost". + * + * @return the canonical hostname + */ + public abstract String getHostname(); + /** * @param variable system variable to read * @return value of the system variable */ - String getenv(String variable); + public abstract String getenv(String variable); /** * @param key of the system property to read * @return value of the system property */ - String getProperty(String key); + public abstract String getProperty(String key); /** * @return the git configuration found in the user home */ - RepositoryConfig openUserConfig(); + public abstract RepositoryConfig openUserConfig(); } -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 07/19] Correct user config to be of type FileBasedConfig 2009-07-25 18:52 ` [JGIT PATCH 06/19] Move SystemReader out of RepositoryConfig Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 08/19] Extract the test specific SystemReader out of RepositoryTestCase Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git The user's ~/.gitconfig file is a file, but not a repository specific file. Since RepositoryConfig has been refactored into base classes that handle file based configuration, we no longer need to use the RepositoryConfig type to access ~/.gitconfig. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 4 ++-- .../src/org/spearce/jgit/util/SystemReader.java | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index 9be7c1b..5c912b7 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -60,7 +60,7 @@ * @return a new configuration instance to read the user's global * configuration file from their home directory. */ - public static RepositoryConfig openUserConfig() { + public static FileBasedConfig openUserConfig() { return SystemReader.getInstance().openUserConfig(); } @@ -85,7 +85,7 @@ public static RepositoryConfig openUserConfig() { * @param cfgLocation * path of the file to load (or save). */ - public RepositoryConfig(final RepositoryConfig base, final File cfgLocation) { + public RepositoryConfig(final Config base, final File cfgLocation) { super(base, cfgLocation); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java b/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java index 36c188c..a30dfef 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java +++ b/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java @@ -41,6 +41,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; +import org.spearce.jgit.lib.FileBasedConfig; import org.spearce.jgit.lib.RepositoryConfig; /** @@ -63,7 +64,7 @@ public String getProperty(String key) { return System.getProperty(key); } - public RepositoryConfig openUserConfig() { + public FileBasedConfig openUserConfig() { final File home = FS.userHome(); return new RepositoryConfig(null, new File(home, ".gitconfig")); } @@ -119,5 +120,5 @@ public static void setInstance(SystemReader newReader) { /** * @return the git configuration found in the user home */ - public abstract RepositoryConfig openUserConfig(); + public abstract FileBasedConfig openUserConfig(); } -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 08/19] Extract the test specific SystemReader out of RepositoryTestCase 2009-07-25 18:52 ` [JGIT PATCH 07/19] Correct user config to be of type FileBasedConfig Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git We may need this in tests that don't extend off RepositoryTestCase, as not all tests require a local Git repository to be created in the host filesystem. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../tst/org/spearce/jgit/lib/MockSystemReader.java | 78 ++++++++++++++++++++ .../org/spearce/jgit/lib/RepositoryConfigTest.java | 49 ++++++------ .../org/spearce/jgit/lib/RepositoryTestCase.java | 56 +------------- 3 files changed, 107 insertions(+), 76 deletions(-) create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java new file mode 100644 index 0000000..62862d1 --- /dev/null +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2009, Yann Simon <yann.simon.fr@gmail.com> + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Git Development Community nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.spearce.jgit.lib; + +import java.util.HashMap; +import java.util.Map; + +import org.spearce.jgit.util.SystemReader; + +class MockSystemReader extends SystemReader { + final Map<String, String> values = new HashMap<String, String>(); + + FileBasedConfig userGitConfig; + + MockSystemReader() { + init(Constants.OS_USER_NAME_KEY); + init(Constants.GIT_AUTHOR_NAME_KEY); + init(Constants.GIT_AUTHOR_EMAIL_KEY); + init(Constants.GIT_COMMITTER_NAME_KEY); + init(Constants.GIT_COMMITTER_EMAIL_KEY); + userGitConfig = new FileBasedConfig(null, null); + } + + private void init(final String n) { + values.put(n, n); + } + + public String getenv(String variable) { + return values.get(variable); + } + + public String getProperty(String key) { + return values.get(key); + } + + public FileBasedConfig openUserConfig() { + return userGitConfig; + } + + public String getHostname() { + return "fake.host.example.com"; + } +} diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java index 5bb9afb..e320679 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java @@ -45,6 +45,8 @@ import java.util.Arrays; import java.util.LinkedList; +import org.spearce.jgit.util.SystemReader; + /** * Test reading of git config */ @@ -113,59 +115,58 @@ public void test006_readCaseInsensitive() throws IOException { assertEquals("", repositoryConfig.getString("foo", null, "bar")); } - public void test007_readUserInfos() throws IOException { - final String hostname = FAKE_HOSTNAME; - final File localConfig = writeTrashFile("local.config", ""); - System.clearProperty(Constants.OS_USER_NAME_KEY); - - RepositoryConfig localRepositoryConfig = new RepositoryConfig(userGitConfig, localConfig); - fakeSystemReader.values.clear(); + public void test007_readUserConfig() { + final MockSystemReader mockSystemReader = (MockSystemReader)SystemReader.getInstance(); + final String hostname = mockSystemReader.getHostname(); + final Config userGitConfig = mockSystemReader.userGitConfig; + final RepositoryConfig localConfig = db.getConfig(); + mockSystemReader.values.clear(); String authorName; String authorEmail; // no values defined nowhere - authorName = localRepositoryConfig.getAuthorName(); - authorEmail = localRepositoryConfig.getAuthorEmail(); + authorName = localConfig.getAuthorName(); + authorEmail = localConfig.getAuthorEmail(); assertEquals(Constants.UNKNOWN_USER_DEFAULT, authorName); assertEquals(Constants.UNKNOWN_USER_DEFAULT + "@" + hostname, authorEmail); // the system user name is defined - fakeSystemReader.values.put(Constants.OS_USER_NAME_KEY, "os user name"); - authorName = localRepositoryConfig.getAuthorName(); + mockSystemReader.values.put(Constants.OS_USER_NAME_KEY, "os user name"); + authorName = localConfig.getAuthorName(); assertEquals("os user name", authorName); if (hostname != null && hostname.length() != 0) { - authorEmail = localRepositoryConfig.getAuthorEmail(); + authorEmail = localConfig.getAuthorEmail(); assertEquals("os user name@" + hostname, authorEmail); } // the git environment variables are defined - fakeSystemReader.values.put(Constants.GIT_AUTHOR_NAME_KEY, "git author name"); - fakeSystemReader.values.put(Constants.GIT_AUTHOR_EMAIL_KEY, "author@email"); - authorName = localRepositoryConfig.getAuthorName(); - authorEmail = localRepositoryConfig.getAuthorEmail(); + mockSystemReader.values.put(Constants.GIT_AUTHOR_NAME_KEY, "git author name"); + mockSystemReader.values.put(Constants.GIT_AUTHOR_EMAIL_KEY, "author@email"); + authorName = localConfig.getAuthorName(); + authorEmail = localConfig.getAuthorEmail(); assertEquals("git author name", authorName); assertEquals("author@email", authorEmail); // the values are defined in the global configuration userGitConfig.setString("user", null, "name", "global username"); userGitConfig.setString("user", null, "email", "author@globalemail"); - authorName = localRepositoryConfig.getAuthorName(); - authorEmail = localRepositoryConfig.getAuthorEmail(); + authorName = localConfig.getAuthorName(); + authorEmail = localConfig.getAuthorEmail(); assertEquals("global username", authorName); assertEquals("author@globalemail", authorEmail); // the values are defined in the local configuration - localRepositoryConfig.setString("user", null, "name", "local username"); - localRepositoryConfig.setString("user", null, "email", "author@localemail"); - authorName = localRepositoryConfig.getAuthorName(); - authorEmail = localRepositoryConfig.getAuthorEmail(); + localConfig.setString("user", null, "name", "local username"); + localConfig.setString("user", null, "email", "author@localemail"); + authorName = localConfig.getAuthorName(); + authorEmail = localConfig.getAuthorEmail(); assertEquals("local username", authorName); assertEquals("author@localemail", authorEmail); - authorName = localRepositoryConfig.getCommitterName(); - authorEmail = localRepositoryConfig.getCommitterEmail(); + authorName = localConfig.getCommitterName(); + authorEmail = localConfig.getCommitterEmail(); assertEquals("local username", authorName); assertEquals("author@localemail", authorEmail); } diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java index 9dfaeef..24a99ca 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java @@ -47,9 +47,7 @@ import java.io.OutputStreamWriter; import java.io.Reader; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import junit.framework.TestCase; @@ -81,8 +79,6 @@ protected static final PersonIdent jcommitter; - protected static final String FAKE_HOSTNAME = "fake.host.example.com"; - static { jauthor = new PersonIdent("J. Author", "jauthor@example.com"); jcommitter = new PersonIdent("J. Committer", "jcommitter@example.com"); @@ -90,38 +86,6 @@ protected boolean packedGitMMAP; - protected static class FakeSystemReader extends SystemReader { - Map<String, String> values = new HashMap<String, String>(); - RepositoryConfig userGitConfig; - public String getenv(String variable) { - return values.get(variable); - } - public String getProperty(String key) { - return values.get(key); - } - public RepositoryConfig openUserConfig() { - return userGitConfig; - } - public void setUserGitConfig(RepositoryConfig userGitConfig) { - this.userGitConfig = userGitConfig; - } - public String getHostname() { - return FAKE_HOSTNAME; - } - } - - /** - * Simulates the reading of system variables and properties. - * Unit test can control the returned values by manipulating - * {@link FakeSystemReader#values}. - */ - protected static FakeSystemReader fakeSystemReader; - - static { - fakeSystemReader = new FakeSystemReader(); - SystemReader.setInstance(fakeSystemReader); - } - /** * Configure JGit before setting up test repositories. */ @@ -241,12 +205,6 @@ protected static void checkFile(File f, final String checkData) protected Repository db; - /** - * mock user's global configuration used instead ~/.gitconfig. - * This configuration can be modified by the tests without any - * effect for ~/.gitconfig. - */ - protected RepositoryConfig userGitConfig; private static Thread shutdownhook; private static List<Runnable> shutDownCleanups = new ArrayList<Runnable>(); private static int testcount; @@ -278,9 +236,10 @@ public void run() { Runtime.getRuntime().addShutdownHook(shutdownhook); } - final File userGitConfigFile = new File(trash_git, "usergitconfig").getAbsoluteFile(); - userGitConfig = new RepositoryConfig(null, userGitConfigFile); - fakeSystemReader.setUserGitConfig(userGitConfig); + final MockSystemReader mockSystemReader = new MockSystemReader(); + mockSystemReader.userGitConfig = new FileBasedConfig(null, new File( + trash_git, "usergitconfig")); + SystemReader.setInstance(mockSystemReader); db = new Repository(trash_git); db.create(); @@ -302,13 +261,6 @@ public void run() { } copyFile(JGitTestUtil.getTestResourceFile("packed-refs"), new File(trash_git,"packed-refs")); - - fakeSystemReader.values.clear(); - fakeSystemReader.values.put(Constants.OS_USER_NAME_KEY, Constants.OS_USER_NAME_KEY); - fakeSystemReader.values.put(Constants.GIT_AUTHOR_NAME_KEY, Constants.GIT_AUTHOR_NAME_KEY); - fakeSystemReader.values.put(Constants.GIT_AUTHOR_EMAIL_KEY, Constants.GIT_AUTHOR_EMAIL_KEY); - fakeSystemReader.values.put(Constants.GIT_COMMITTER_NAME_KEY, Constants.GIT_COMMITTER_NAME_KEY); - fakeSystemReader.values.put(Constants.GIT_COMMITTER_EMAIL_KEY, Constants.GIT_COMMITTER_EMAIL_KEY); } protected void tearDown() throws Exception { -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit 2009-07-25 18:52 ` [JGIT PATCH 08/19] Extract the test specific SystemReader out of RepositoryTestCase Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 10/19] Test for the config file when creating a new repository Shawn O. Pearce 2009-07-25 22:54 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Robin Rosenberg 0 siblings, 2 replies; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git, Constantine Plotnikov Accessing a config file may or may not require IO, e.g. if we are reading a file that was already loaded from a blob in the repository then there is no additional IO required. Moving the IO portions out of the base Config class helps to isolate it to only where we really know we have to do IO, making it easier to handle the IO conditions. This makes it easier to deal with test cases for config files, as we no longer need to perform local disk IO just to test the parse and formatting logic. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> CC: Constantine Plotnikov <constantine.plotnikov@gmail.com> --- .../org/spearce/jgit/lib/RepositoryConfigTest.java | 138 +++---- .../tst/org/spearce/jgit/lib/T0003_Basic.java | 16 +- .../spearce/jgit/transport/RemoteConfigTest.java | 166 ++++----- .../jgit/errors/ConfigInvalidException.java | 53 +++ .../src/org/spearce/jgit/lib/BlobBasedConfig.java | 110 ++---- .../src/org/spearce/jgit/lib/Config.java | 410 ++++++++++---------- .../src/org/spearce/jgit/lib/FileBasedConfig.java | 83 +++-- .../src/org/spearce/jgit/lib/Repository.java | 24 +- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 19 +- 9 files changed, 507 insertions(+), 512 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java index e320679..d4a6dd2 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java @@ -40,86 +40,77 @@ package org.spearce.jgit.lib; -import java.io.File; -import java.io.IOException; import java.util.Arrays; import java.util.LinkedList; +import junit.framework.TestCase; + +import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.util.SystemReader; /** * Test reading of git config */ -public class RepositoryConfigTest extends RepositoryTestCase { - /** - * Read config item with no value from a section without a subsection. - * - * @throws IOException - */ - public void test001_ReadBareKey() throws IOException { - final RepositoryConfig repositoryConfig = read("[foo]\nbar\n"); - assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false)); - assertEquals("", repositoryConfig.getString("foo", null, "bar")); +public class RepositoryConfigTest extends TestCase { + public void test001_ReadBareKey() throws ConfigInvalidException { + final Config c = parse("[foo]\nbar\n"); + assertEquals(true, c.getBoolean("foo", null, "bar", false)); + assertEquals("", c.getString("foo", null, "bar")); } - /** - * Read various data from a subsection. - * - * @throws IOException - */ - public void test002_ReadWithSubsection() throws IOException { - final RepositoryConfig repositoryConfig = read("[foo \"zip\"]\nbar\n[foo \"zap\"]\nbar=false\nn=3\n"); - assertEquals(true, repositoryConfig.getBoolean("foo", "zip", "bar", false)); - assertEquals("", repositoryConfig.getString("foo","zip", "bar")); - assertEquals(false, repositoryConfig.getBoolean("foo", "zap", "bar", true)); - assertEquals("false", repositoryConfig.getString("foo", "zap", "bar")); - assertEquals(3, repositoryConfig.getInt("foo", "zap", "n", 4)); - assertEquals(4, repositoryConfig.getInt("foo", "zap","m", 4)); + public void test002_ReadWithSubsection() throws ConfigInvalidException { + final Config c = parse("[foo \"zip\"]\nbar\n[foo \"zap\"]\nbar=false\nn=3\n"); + assertEquals(true, c.getBoolean("foo", "zip", "bar", false)); + assertEquals("", c.getString("foo","zip", "bar")); + assertEquals(false, c.getBoolean("foo", "zap", "bar", true)); + assertEquals("false", c.getString("foo", "zap", "bar")); + assertEquals(3, c.getInt("foo", "zap", "n", 4)); + assertEquals(4, c.getInt("foo", "zap","m", 4)); } - public void test003_PutRemote() throws IOException { - File cfgFile = writeTrashFile("config_003", ""); - RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile); - repositoryConfig.setString("sec", "ext", "name", "value"); - repositoryConfig.setString("sec", "ext", "name2", "value2"); - repositoryConfig.save(); - checkFile(cfgFile, "[sec \"ext\"]\n\tname = value\n\tname2 = value2\n"); + public void test003_PutRemote() { + final Config c = new Config(); + c.setString("sec", "ext", "name", "value"); + c.setString("sec", "ext", "name2", "value2"); + final String expText = "[sec \"ext\"]\n\tname = value\n\tname2 = value2\n"; + assertEquals(expText, c.toText()); } - public void test004_PutGetSimple() throws IOException { - File cfgFile = writeTrashFile("config_004", ""); - RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile); - repositoryConfig.setString("my", null, "somename", "false"); - repositoryConfig.save(); - checkFile(cfgFile, "[my]\n\tsomename = false\n"); - assertEquals("false", repositoryConfig - .getString("my", null, "somename")); + public void test004_PutGetSimple() { + Config c = new Config(); + c.setString("my", null, "somename", "false"); + assertEquals("false", c.getString("my", null, "somename")); + assertEquals("[my]\n\tsomename = false\n", c.toText()); } - public void test005_PutGetStringList() throws IOException { - File cfgFile = writeTrashFile("config_005", ""); - RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile); + public void test005_PutGetStringList() { + Config c = new Config(); final LinkedList<String> values = new LinkedList<String>(); values.add("value1"); values.add("value2"); - repositoryConfig.setStringList("my", null, "somename", values); - repositoryConfig.save(); - assertTrue(Arrays.equals(values.toArray(), repositoryConfig - .getStringList("my", null, "somename"))); - checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n"); + c.setStringList("my", null, "somename", values); + + final Object[] expArr = values.toArray(); + final String[] actArr = c.getStringList("my", null, "somename"); + assertTrue(Arrays.equals(expArr, actArr)); + + final String expText = "[my]\n\tsomename = value1\n\tsomename = value2\n"; + assertEquals(expText, c.toText()); } - public void test006_readCaseInsensitive() throws IOException { - final RepositoryConfig repositoryConfig = read("[Foo]\nBar\n"); - assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false)); - assertEquals("", repositoryConfig.getString("foo", null, "bar")); + public void test006_readCaseInsensitive() throws ConfigInvalidException { + final Config c = parse("[Foo]\nBar\n"); + assertEquals(true, c.getBoolean("foo", null, "bar", false)); + assertEquals("", c.getString("foo", null, "bar")); } public void test007_readUserConfig() { - final MockSystemReader mockSystemReader = (MockSystemReader)SystemReader.getInstance(); + final MockSystemReader mockSystemReader = new MockSystemReader(); + SystemReader.setInstance(mockSystemReader); final String hostname = mockSystemReader.getHostname(); final Config userGitConfig = mockSystemReader.userGitConfig; - final RepositoryConfig localConfig = db.getConfig(); + final RepositoryConfig localConfig = new RepositoryConfig(userGitConfig, null); + localConfig.create(); mockSystemReader.values.clear(); String authorName; @@ -171,8 +162,8 @@ public void test007_readUserConfig() { assertEquals("author@localemail", authorEmail); } - public void testReadBoolean_TrueFalse1() throws IOException { - final RepositoryConfig c = read("[s]\na = true\nb = false\n"); + public void testReadBoolean_TrueFalse1() throws ConfigInvalidException { + final Config c = parse("[s]\na = true\nb = false\n"); assertEquals("true", c.getString("s", null, "a")); assertEquals("false", c.getString("s", null, "b")); @@ -180,8 +171,8 @@ public void testReadBoolean_TrueFalse1() throws IOException { assertFalse(c.getBoolean("s", "b", true)); } - public void testReadBoolean_TrueFalse2() throws IOException { - final RepositoryConfig c = read("[s]\na = TrUe\nb = fAlSe\n"); + public void testReadBoolean_TrueFalse2() throws ConfigInvalidException { + final Config c = parse("[s]\na = TrUe\nb = fAlSe\n"); assertEquals("TrUe", c.getString("s", null, "a")); assertEquals("fAlSe", c.getString("s", null, "b")); @@ -189,8 +180,8 @@ public void testReadBoolean_TrueFalse2() throws IOException { assertFalse(c.getBoolean("s", "b", true)); } - public void testReadBoolean_YesNo1() throws IOException { - final RepositoryConfig c = read("[s]\na = yes\nb = no\n"); + public void testReadBoolean_YesNo1() throws ConfigInvalidException { + final Config c = parse("[s]\na = yes\nb = no\n"); assertEquals("yes", c.getString("s", null, "a")); assertEquals("no", c.getString("s", null, "b")); @@ -198,8 +189,8 @@ public void testReadBoolean_YesNo1() throws IOException { assertFalse(c.getBoolean("s", "b", true)); } - public void testReadBoolean_YesNo2() throws IOException { - final RepositoryConfig c = read("[s]\na = yEs\nb = NO\n"); + public void testReadBoolean_YesNo2() throws ConfigInvalidException { + final Config c = parse("[s]\na = yEs\nb = NO\n"); assertEquals("yEs", c.getString("s", null, "a")); assertEquals("NO", c.getString("s", null, "b")); @@ -207,8 +198,8 @@ public void testReadBoolean_YesNo2() throws IOException { assertFalse(c.getBoolean("s", "b", true)); } - public void testReadBoolean_OnOff1() throws IOException { - final RepositoryConfig c = read("[s]\na = on\nb = off\n"); + public void testReadBoolean_OnOff1() throws ConfigInvalidException { + final Config c = parse("[s]\na = on\nb = off\n"); assertEquals("on", c.getString("s", null, "a")); assertEquals("off", c.getString("s", null, "b")); @@ -216,8 +207,8 @@ public void testReadBoolean_OnOff1() throws IOException { assertFalse(c.getBoolean("s", "b", true)); } - public void testReadBoolean_OnOff2() throws IOException { - final RepositoryConfig c = read("[s]\na = ON\nb = OFF\n"); + public void testReadBoolean_OnOff2() throws ConfigInvalidException { + final Config c = parse("[s]\na = ON\nb = OFF\n"); assertEquals("ON", c.getString("s", null, "a")); assertEquals("OFF", c.getString("s", null, "b")); @@ -225,7 +216,7 @@ public void testReadBoolean_OnOff2() throws IOException { assertFalse(c.getBoolean("s", "b", true)); } - public void testReadLong() throws IOException { + public void testReadLong() throws ConfigInvalidException { assertReadLong(1L); assertReadLong(-1L); assertReadLong(Long.MIN_VALUE); @@ -242,18 +233,19 @@ public void testReadLong() throws IOException { } } - private void assertReadLong(long exp) throws IOException { + private void assertReadLong(long exp) throws ConfigInvalidException { assertReadLong(exp, String.valueOf(exp)); } - private void assertReadLong(long exp, String act) throws IOException { - final RepositoryConfig c = read("[s]\na = " + act + "\n"); + private void assertReadLong(long exp, String act) + throws ConfigInvalidException { + final Config c = parse("[s]\na = " + act + "\n"); assertEquals(exp, c.getLong("s", null, "a", 0L)); } - private RepositoryConfig read(final String content) throws IOException { - final File p = writeTrashFile(getName() + ".config", content); - final RepositoryConfig c = new RepositoryConfig(null, p); + private Config parse(final String content) throws ConfigInvalidException { + final Config c = new Config(null); + c.fromText(content); return c; } } diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java index df55b4f..3660b45 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java @@ -47,6 +47,8 @@ import java.io.IOException; import java.io.PrintWriter; +import org.spearce.jgit.errors.ConfigInvalidException; + public class T0003_Basic extends RepositoryTestCase { public void test001_Initalize() { final File gitdir = new File(trash, ".git"); @@ -118,7 +120,7 @@ public void test003_WriteShouldBeEmptyTree() throws IOException { assertTrue("Read-only " + o, !o.canWrite()); } - public void test004_CheckNewConfig() throws IOException { + public void test004_CheckNewConfig() { final RepositoryConfig c = db.getConfig(); assertNotNull(c); assertEquals("0", c.getString("core", null, "repositoryformatversion")); @@ -126,13 +128,11 @@ public void test004_CheckNewConfig() throws IOException { assertEquals("true", c.getString("core", null, "filemode")); assertEquals("true", c.getString("cOrE", null, "fIlEModE")); assertNull(c.getString("notavalue", null, "reallyNotAValue")); - c.load(); } - public void test005_ReadSimpleConfig() throws IOException { + public void test005_ReadSimpleConfig() { final RepositoryConfig c = db.getConfig(); assertNotNull(c); - c.load(); assertEquals("0", c.getString("core", null, "repositoryformatversion")); assertEquals("0", c.getString("CoRe", null, "REPOSITORYFoRmAtVeRsIoN")); assertEquals("true", c.getString("core", null, "filemode")); @@ -140,7 +140,8 @@ public void test005_ReadSimpleConfig() throws IOException { assertNull(c.getString("notavalue", null, "reallyNotAValue")); } - public void test006_ReadUglyConfig() throws IOException { + public void test006_ReadUglyConfig() throws IOException, + ConfigInvalidException { final RepositoryConfig c = db.getConfig(); final File cfg = new File(db.getDirectory(), "config"); final FileWriter pw = new FileWriter(cfg); @@ -192,7 +193,8 @@ public void test008_FailOnWrongVersion() throws IOException { } } - public void test009_CreateCommitOldFormat() throws IOException { + public void test009_CreateCommitOldFormat() throws IOException, + ConfigInvalidException { writeTrashFile(".git/config", "[core]\n" + "legacyHeaders=1\n"); db.getConfig().load(); @@ -417,8 +419,6 @@ public void test025_computeSha1NoStore() throws IOException { } public void test026_CreateCommitMultipleparents() throws IOException { - db.getConfig().load(); - final Tree t = new Tree(db); final FileTreeEntry f = t.addFile("i-am-a-file"); writeTrashFile(f.getName(), "and this is the data in me\n"); diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/RemoteConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/RemoteConfigTest.java index 3965bdb..752019d 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/RemoteConfigTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/RemoteConfigTest.java @@ -38,26 +38,28 @@ package org.spearce.jgit.transport; -import java.io.File; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.util.List; -import org.spearce.jgit.lib.RepositoryTestCase; - -public class RemoteConfigTest extends RepositoryTestCase { - private void writeConfig(final String dat) throws FileNotFoundException, - IOException, UnsupportedEncodingException { - final File file = new File(db.getDirectory(), "config"); - final FileOutputStream stream = new FileOutputStream(file, true); - try { - stream.write(dat.getBytes("UTF-8")); - } finally { - stream.close(); - } - db.getConfig().load(); +import junit.framework.TestCase; + +import org.spearce.jgit.errors.ConfigInvalidException; +import org.spearce.jgit.lib.Config; + +public class RemoteConfigTest extends TestCase { + private Config config; + + protected void setUp() throws Exception { + super.setUp(); + config = new Config(); + } + + private void readConfig(final String dat) throws ConfigInvalidException { + config = new Config(); + config.fromText(dat); + } + + private void checkConfig(final String exp) { + assertEquals(exp, config.toText()); } private static void assertEquals(final String exp, final URIish act) { @@ -65,11 +67,11 @@ private static void assertEquals(final String exp, final URIish act) { } public void testSimple() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); final List<URIish> allURIs = rc.getURIs(); RefSpec spec; @@ -95,30 +97,30 @@ public void testSimple() throws Exception { } public void testSimpleNoTags() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n" + "tagopt = --no-tags\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); assertSame(TagOpt.NO_TAGS, rc.getTagOpt()); } public void testSimpleAlwaysTags() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n" + "tagopt = --tags\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); assertSame(TagOpt.FETCH_TAGS, rc.getTagOpt()); } public void testMirror() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "fetch = +refs/heads/*:refs/heads/*\n" + "fetch = refs/tags/*:refs/tags/*\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); final List<URIish> allURIs = rc.getURIs(); RefSpec spec; @@ -148,13 +150,13 @@ public void testMirror() throws Exception { } public void testBackup() throws Exception { - writeConfig("[remote \"backup\"]\n" + readConfig("[remote \"backup\"]\n" + "url = http://www.spearce.org/egit.git\n" + "url = user@repo.or.cz:/srv/git/egit.git\n" + "push = +refs/heads/*:refs/heads/*\n" + "push = refs/tags/*:refs/tags/*\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); final List<URIish> allURIs = rc.getURIs(); RefSpec spec; @@ -184,13 +186,13 @@ public void testBackup() throws Exception { } public void testUploadPack() throws Exception { - writeConfig("[remote \"example\"]\n" + readConfig("[remote \"example\"]\n" + "url = user@example.com:egit.git\n" + "fetch = +refs/heads/*:refs/remotes/example/*\n" + "uploadpack = /path/to/git/git-upload-pack\n" + "receivepack = /path/to/git/git-receive-pack\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "example"); + final RemoteConfig rc = new RemoteConfig(config, "example"); final List<URIish> allURIs = rc.getURIs(); RefSpec spec; @@ -216,9 +218,9 @@ public void testUploadPack() throws Exception { } public void testUnknown() throws Exception { - writeConfig(""); + readConfig(""); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); assertEquals(0, rc.getURIs().size()); assertEquals(0, rc.getFetchRefSpecs().size()); assertEquals(0, rc.getPushRefSpecs().size()); @@ -227,10 +229,10 @@ public void testUnknown() throws Exception { } public void testAddURI() throws Exception { - writeConfig(""); + readConfig(""); final URIish uri = new URIish("/some/dir"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); assertEquals(0, rc.getURIs().size()); assertTrue(rc.addURI(uri)); @@ -242,12 +244,12 @@ public void testAddURI() throws Exception { } public void testRemoveFirstURI() throws Exception { - writeConfig(""); + readConfig(""); final URIish a = new URIish("/some/dir"); final URIish b = new URIish("/another/dir"); final URIish c = new URIish("/more/dirs"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); assertTrue(rc.addURI(a)); assertTrue(rc.addURI(b)); assertTrue(rc.addURI(c)); @@ -264,12 +266,12 @@ public void testRemoveFirstURI() throws Exception { } public void testRemoveMiddleURI() throws Exception { - writeConfig(""); + readConfig(""); final URIish a = new URIish("/some/dir"); final URIish b = new URIish("/another/dir"); final URIish c = new URIish("/more/dirs"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); assertTrue(rc.addURI(a)); assertTrue(rc.addURI(b)); assertTrue(rc.addURI(c)); @@ -286,12 +288,12 @@ public void testRemoveMiddleURI() throws Exception { } public void testRemoveLastURI() throws Exception { - writeConfig(""); + readConfig(""); final URIish a = new URIish("/some/dir"); final URIish b = new URIish("/another/dir"); final URIish c = new URIish("/more/dirs"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); assertTrue(rc.addURI(a)); assertTrue(rc.addURI(b)); assertTrue(rc.addURI(c)); @@ -308,10 +310,10 @@ public void testRemoveLastURI() throws Exception { } public void testRemoveOnlyURI() throws Exception { - writeConfig(""); + readConfig(""); final URIish a = new URIish("/some/dir"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "backup"); + final RemoteConfig rc = new RemoteConfig(config, "backup"); assertTrue(rc.addURI(a)); assertEquals(1, rc.getURIs().size()); @@ -322,130 +324,102 @@ public void testRemoveOnlyURI() throws Exception { } public void testCreateOrigin() throws Exception { - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "origin"); + final RemoteConfig rc = new RemoteConfig(config, "origin"); rc.addURI(new URIish("/some/dir")); rc.addFetchRefSpec(new RefSpec("+refs/heads/*:refs/remotes/" + rc.getName() + "/*")); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"origin\"]\n" + "\turl = /some/dir\n" + rc.update(config); + checkConfig("[remote \"origin\"]\n" + "\turl = /some/dir\n" + "\tfetch = +refs/heads/*:refs/remotes/origin/*\n"); } public void testSaveAddURI() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); rc.addURI(new URIish("/some/dir")); assertEquals(2, rc.getURIs().size()); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"spearce\"]\n" + rc.update(config); + checkConfig("[remote \"spearce\"]\n" + "\turl = http://www.spearce.org/egit.git\n" + "\turl = /some/dir\n" + "\tfetch = +refs/heads/*:refs/remotes/spearce/*\n"); } public void testSaveRemoveLastURI() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "url = /some/dir\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); assertEquals(2, rc.getURIs().size()); rc.removeURI(new URIish("/some/dir")); assertEquals(1, rc.getURIs().size()); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"spearce\"]\n" + rc.update(config); + checkConfig("[remote \"spearce\"]\n" + "\turl = http://www.spearce.org/egit.git\n" + "\tfetch = +refs/heads/*:refs/remotes/spearce/*\n"); } public void testSaveRemoveFirstURI() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "url = /some/dir\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); assertEquals(2, rc.getURIs().size()); rc.removeURI(new URIish("http://www.spearce.org/egit.git")); assertEquals(1, rc.getURIs().size()); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"spearce\"]\n" + "\turl = /some/dir\n" + rc.update(config); + checkConfig("[remote \"spearce\"]\n" + "\turl = /some/dir\n" + "\tfetch = +refs/heads/*:refs/remotes/spearce/*\n"); } public void testSaveNoTags() throws Exception { - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "origin"); + final RemoteConfig rc = new RemoteConfig(config, "origin"); rc.addURI(new URIish("/some/dir")); rc.addFetchRefSpec(new RefSpec("+refs/heads/*:refs/remotes/" + rc.getName() + "/*")); rc.setTagOpt(TagOpt.NO_TAGS); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"origin\"]\n" + "\turl = /some/dir\n" + rc.update(config); + checkConfig("[remote \"origin\"]\n" + "\turl = /some/dir\n" + "\tfetch = +refs/heads/*:refs/remotes/origin/*\n" + "\ttagopt = --no-tags\n"); } public void testSaveAllTags() throws Exception { - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "origin"); + final RemoteConfig rc = new RemoteConfig(config, "origin"); rc.addURI(new URIish("/some/dir")); rc.addFetchRefSpec(new RefSpec("+refs/heads/*:refs/remotes/" + rc.getName() + "/*")); rc.setTagOpt(TagOpt.FETCH_TAGS); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"origin\"]\n" + "\turl = /some/dir\n" + rc.update(config); + checkConfig("[remote \"origin\"]\n" + "\turl = /some/dir\n" + "\tfetch = +refs/heads/*:refs/remotes/origin/*\n" + "\ttagopt = --tags\n"); } public void testSimpleTimeout() throws Exception { - writeConfig("[remote \"spearce\"]\n" + readConfig("[remote \"spearce\"]\n" + "url = http://www.spearce.org/egit.git\n" + "fetch = +refs/heads/*:refs/remotes/spearce/*\n" + "timeout = 12\n"); - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "spearce"); + final RemoteConfig rc = new RemoteConfig(config, "spearce"); assertEquals(12, rc.getTimeout()); } public void testSaveTimeout() throws Exception { - final RemoteConfig rc = new RemoteConfig(db.getConfig(), "origin"); + final RemoteConfig rc = new RemoteConfig(config, "origin"); rc.addURI(new URIish("/some/dir")); rc.addFetchRefSpec(new RefSpec("+refs/heads/*:refs/remotes/" + rc.getName() + "/*")); rc.setTimeout(60); - rc.update(db.getConfig()); - db.getConfig().save(); - - checkFile(new File(db.getDirectory(), "config"), "[core]\n" - + "\trepositoryformatversion = 0\n" + "\tfilemode = true\n" - + "[remote \"origin\"]\n" + "\turl = /some/dir\n" + rc.update(config); + checkConfig("[remote \"origin\"]\n" + "\turl = /some/dir\n" + "\tfetch = +refs/heads/*:refs/remotes/origin/*\n" + "\ttimeout = 60\n"); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java new file mode 100644 index 0000000..ae6ad58 --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2009, Google Inc. + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Git Development Community nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.spearce.jgit.errors; + +/** Indicates a text string is not a valid Git style configuration. */ +public class ConfigInvalidException extends Exception { + private static final long serialVersionUID = 1L; + + /** + * Construct an invalid configuration error. + * + * @param message + * why the configuration is invalid. + */ + public ConfigInvalidException(final String message) { + super(message); + } +} diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/BlobBasedConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/BlobBasedConfig.java index 8763c6c..e97d797 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/BlobBasedConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/BlobBasedConfig.java @@ -36,111 +36,85 @@ */ package org.spearce.jgit.lib; -import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStream; -import java.util.concurrent.Callable; +import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.treewalk.TreeWalk; +import org.spearce.jgit.util.RawParseUtils; /** * The configuration file based on the blobs stored in the repository */ public class BlobBasedConfig extends Config { - private Callable<byte[]> blobProvider; - - /** - * The constructor for blob based config - * - * @param base - * the base configuration file - * @param blob - * the provider for blobs - */ - public BlobBasedConfig(Config base, Callable<byte[]> blob) { - super(base); - blobProvider = blob; - } - /** * The constructor from a byte array - * + * * @param base * the base configuration file * @param blob - * the byte array + * the byte array, should be UTF-8 encoded text. + * @throws ConfigInvalidException + * the byte array is not a valid configuration format. */ - public BlobBasedConfig(Config base, final byte[] blob) { - this(base, new Callable<byte[]>() { - public byte[] call() throws Exception { - return blob; - } - }); + public BlobBasedConfig(Config base, final byte[] blob) + throws ConfigInvalidException { + super(base); + fromText(RawParseUtils.decode(blob)); } /** * The constructor from object identifier - * + * * @param base * the base configuration file * @param r * the repository * @param objectId * the object identifier + * @throws IOException + * the blob cannot be read from the repository. + * @throws ConfigInvalidException + * the blob is not a valid configuration format. */ public BlobBasedConfig(Config base, final Repository r, - final ObjectId objectId) { - this(base, new Callable<byte[]>() { - public byte[] call() throws Exception { - ObjectLoader loader = r.openBlob(objectId); - if (loader == null) { - throw new IOException("Blob not found: " + objectId); - } - return loader.getBytes(); - } - }); + final ObjectId objectId) throws IOException, ConfigInvalidException { + super(base); + final ObjectLoader loader = r.openBlob(objectId); + if (loader == null) + throw new IOException("Blob not found: " + objectId); + fromText(RawParseUtils.decode(loader.getBytes())); } /** * The constructor from commit and path - * + * * @param base * the base configuration file * @param commit * the commit that contains the object * @param path * the path within the tree of the commit + * @throws FileNotFoundException + * the path does not exist in the commit's tree. + * @throws IOException + * the tree and/or blob cannot be accessed. + * @throws ConfigInvalidException + * the blob is not a valid configuration format. */ - public BlobBasedConfig(Config base, final Commit commit, final String path) { - this(base, new Callable<byte[]>() { - public byte[] call() throws Exception { - final ObjectId treeId = commit.getTreeId(); - final Repository r = commit.getRepository(); - final TreeWalk tree = TreeWalk.forPath(r, path, treeId); - if (tree == null) { - throw new FileNotFoundException("Entry not found by path: " + path); - } - ObjectId blobId = tree.getObjectId(0); - ObjectLoader loader = tree.getRepository().openBlob(blobId); - if (loader == null) { - throw new IOException("Blob not found: " + blobId + " for path: " + path); - } - return loader.getBytes(); - } - }); - } - - @Override - protected InputStream openInputStream() throws IOException { - try { - return new ByteArrayInputStream(blobProvider.call()); - } catch (IOException e) { - throw e; - } catch (Exception e) { - final IOException e2 = new IOException("Unable to read config"); - e2.initCause(e); - throw e2; - } + public BlobBasedConfig(Config base, final Commit commit, final String path) + throws FileNotFoundException, IOException, ConfigInvalidException { + super(base); + final ObjectId treeId = commit.getTreeId(); + final Repository r = commit.getRepository(); + final TreeWalk tree = TreeWalk.forPath(r, path, treeId); + if (tree == null) + throw new FileNotFoundException("Entry not found by path: " + path); + final ObjectId blobId = tree.getObjectId(0); + final ObjectLoader loader = tree.getRepository().openBlob(blobId); + if (loader == null) + throw new IOException("Blob not found: " + blobId + " for path: " + + path); + fromText(RawParseUtils.decode(loader.getBytes())); } } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index a2f5c6a..d63e926 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -41,12 +41,6 @@ */ package org.spearce.jgit.lib; -import java.io.BufferedReader; -import java.io.FileNotFoundException; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.PrintWriter; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -55,19 +49,18 @@ import java.util.Map; import java.util.Set; +import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.util.StringUtils; + /** - * The configuration file stored in the format similar to the ".git/config" - * file. + * Git style {@code .config}, {@code .gitconfig}, {@code .gitmodules} file. */ -public abstract class Config { +public class Config { private static final long KiB = 1024; private static final long MiB = 1024 * KiB; private static final long GiB = 1024 * MiB; - private boolean fileRead; - private List<Entry> entries; final Config baseConfig; @@ -86,26 +79,21 @@ */ private static final String MAGIC_EMPTY_VALUE = new String(); - /** - * The constructor for configuration file - * - * @param base - * the base configuration file to be consulted when value is - * missing from this file - */ - protected Config(Config base) { - baseConfig = base; - clear(); + /** Create a configuration with no default fallback. */ + public Config() { + this(null); } /** - * Set file read indicator + * Create an empty configuration with a fallback for missing keys. * - * @param ok - * true if file does not need loading + * @param defaultConfig + * the base configuration to be consulted when a key is missing + * from this configuration instance. */ - protected void setFileRead(boolean ok) { - fileRead = ok; + public Config(Config defaultConfig) { + baseConfig = defaultConfig; + clear(); } /** @@ -373,8 +361,6 @@ public String getString(final String section, String subsection, */ public Set<String> getSubsections(final String section) { final Set<String> result = new HashSet<String>(); - ensureLoaded(); - for (final Entry e : entries) { if (StringUtils.equalsIgnoreCase(section, e.section) && e.subsection != null) @@ -398,22 +384,8 @@ private String getRawString(final String section, final String subsection, return null; } - private void ensureLoaded() { - if (!fileRead) { - try { - load(); - } catch (FileNotFoundException err) { - // Oh well. No sense in complaining about it. - // - } catch (IOException err) { - err.printStackTrace(); - } - } - } - private Object getRawEntry(final String section, final String subsection, final String name) { - ensureLoaded(); return byName.get(concatenateKey(section, subsection, name)); } @@ -678,129 +650,116 @@ private int findSectionEnd(final String section, final String subsection) { } /** - * Print configuration file to the PrintWriter - * - * @param r - * stream to write the configuration to. + * @return this configuration, formatted as a Git style text file. */ - protected void printConfig(final PrintWriter r) { + public String toText() { + final StringBuilder out = new StringBuilder(); for (final Entry e : entries) { - if (e.prefix != null) { - r.print(e.prefix); - } + if (e.prefix != null) + out.append(e.prefix); if (e.section != null && e.name == null) { - r.print('['); - r.print(e.section); + out.append('['); + out.append(e.section); if (e.subsection != null) { - r.print(' '); - r.print('"'); - r.print(escapeValue(e.subsection)); - r.print('"'); + out.append(' '); + out.append('"'); + out.append(escapeValue(e.subsection)); + out.append('"'); } - r.print(']'); + out.append(']'); } else if (e.section != null && e.name != null) { - if (e.prefix == null || "".equals(e.prefix)) { - r.print('\t'); - } - r.print(e.name); + if (e.prefix == null || "".equals(e.prefix)) + out.append('\t'); + out.append(e.name); if (e.value != null) { if (MAGIC_EMPTY_VALUE != e.value) { - r.print(" = "); - r.print(escapeValue(e.value)); + out.append(" = "); + out.append(escapeValue(e.value)); } } - if (e.suffix != null) { - r.print(' '); - } + if (e.suffix != null) + out.append(' '); } - if (e.suffix != null) { - r.print(e.suffix); - } - r.print('\n'); + if (e.suffix != null) + out.append(e.suffix); + out.append('\n'); } + return out.toString(); } /** - * Read the config file + * Clear this configuration and reset to the contents of the parsed string. * - * @throws IOException in case of IO error + * @param text + * Git style text file listing configuration properties. + * @throws ConfigInvalidException + * the text supplied is not formatted correctly. No changes were + * made to {@code this}. */ - public void load() throws IOException { - clear(); - fileRead = true; - final BufferedReader r = new BufferedReader(new InputStreamReader( - openInputStream(), Constants.CHARSET)); - try { - Entry last = null; - Entry e = new Entry(); - for (;;) { - r.mark(1); - int input = r.read(); - final char in = (char) input; - if (-1 == input) { - break; - } else if ('\n' == in) { - // End of this entry. - add(e); - if (e.section != null) { - last = e; - } - e = new Entry(); - } else if (e.suffix != null) { - // Everything up until the end-of-line is in the suffix. - e.suffix += in; - } else if (';' == in || '#' == in) { - // The rest of this line is a comment; put into suffix. - e.suffix = String.valueOf(in); - } else if (e.section == null && Character.isWhitespace(in)) { - // Save the leading whitespace (if any). - if (e.prefix == null) { - e.prefix = ""; - } - e.prefix += in; - } else if ('[' == in) { - // This is a section header. - e.section = readSectionName(r); - input = r.read(); - if ('"' == input) { - e.subsection = readValue(r, true, '"'); - input = r.read(); - } - if (']' != input) { - throw new IOException("Bad group header."); - } - e.suffix = ""; - } else if (last != null) { - // Read a value. - e.section = last.section; - e.subsection = last.subsection; - r.reset(); - e.name = readKeyName(r); - if (e.name.endsWith("\n")) { - e.name = e.name.substring(0, e.name.length() - 1); - e.value = MAGIC_EMPTY_VALUE; - } else - e.value = readValue(r, false, -1); - } else { - throw new IOException("Invalid line in config file."); + public void fromText(final String text) throws ConfigInvalidException { + entries = new ArrayList<Entry>(); + byName = new HashMap<String, Object>(); + + final StringReader in = new StringReader(text); + Entry last = null; + Entry e = new Entry(); + for (;;) { + int input = in.read(); + if (-1 == input) + break; + + final char c = (char) input; + if ('\n' == c) { + // End of this entry. + add(e); + if (e.section != null) + last = e; + e = new Entry(); + + } else if (e.suffix != null) { + // Everything up until the end-of-line is in the suffix. + e.suffix += c; + + } else if (';' == c || '#' == c) { + // The rest of this line is a comment; put into suffix. + e.suffix = String.valueOf(c); + + } else if (e.section == null && Character.isWhitespace(c)) { + // Save the leading whitespace (if any). + if (e.prefix == null) + e.prefix = ""; + e.prefix += c; + + } else if ('[' == c) { + // This is a section header. + e.section = readSectionName(in); + input = in.read(); + if ('"' == input) { + e.subsection = readValue(in, true, '"'); + input = in.read(); } - } - } finally { - r.close(); + if (']' != input) + throw new ConfigInvalidException("Bad group header"); + e.suffix = ""; + + } else if (last != null) { + // Read a value. + e.section = last.section; + e.subsection = last.subsection; + in.reset(); + e.name = readKeyName(in); + if (e.name.endsWith("\n")) { + e.name = e.name.substring(0, e.name.length() - 1); + e.value = MAGIC_EMPTY_VALUE; + } else + e.value = readValue(in, false, -1); + + } else + throw new ConfigInvalidException("Invalid line in config file"); } } /** - * Open input stream for configuration file. It is used during the - * {@link #load()} method. - * - * @return input stream for the configuration file. - * @throws IOException - * if the stream cannot be created - */ - protected abstract InputStream openInputStream() throws IOException; - - /** * Clear the configuration file */ protected void clear() { @@ -827,132 +786,136 @@ private void add(final Entry e) { } } - private static String readSectionName(final BufferedReader r) - throws IOException { - final StringBuffer name = new StringBuffer(); + private static String readSectionName(final StringReader in) + throws ConfigInvalidException { + final StringBuilder name = new StringBuilder(); for (;;) { - r.mark(1); - int c = r.read(); - if (c < 0) { - throw new IOException("Unexpected end of config file."); - } else if (']' == c) { - r.reset(); + int c = in.read(); + if (c < 0) + throw new ConfigInvalidException("Unexpected end of config file"); + + if (']' == c) { + in.reset(); break; - } else if (' ' == c || '\t' == c) { + } + + if (' ' == c || '\t' == c) { for (;;) { - r.mark(1); - c = r.read(); - if (c < 0) { - throw new IOException("Unexpected end of config file."); - } else if ('"' == c) { - r.reset(); + c = in.read(); + if (c < 0) + throw new ConfigInvalidException("Unexpected end of config file"); + + if ('"' == c) { + in.reset(); break; - } else if (' ' == c || '\t' == c) { - // Skipped... - } else { - throw new IOException("Bad section entry. : " + name - + "," + c); } + + if (' ' == c || '\t' == c) + continue; // Skipped... + throw new ConfigInvalidException("Bad section entry: " + name); } break; - } else if (Character.isLetterOrDigit((char) c) || '.' == c - || '-' == c) { - name.append((char) c); - } else { - throw new IOException("Bad section entry. : " + name + ", " + c); } + + if (Character.isLetterOrDigit((char) c) || '.' == c || '-' == c) + name.append((char) c); + else + throw new ConfigInvalidException("Bad section entry: " + name); } return name.toString(); } - private static String readKeyName(final BufferedReader r) - throws IOException { + private static String readKeyName(final StringReader in) + throws ConfigInvalidException { final StringBuffer name = new StringBuffer(); for (;;) { - r.mark(1); - int c = r.read(); - if (c < 0) { - throw new IOException("Unexpected end of config file."); - } else if ('=' == c) { + int c = in.read(); + if (c < 0) + throw new ConfigInvalidException("Unexpected end of config file"); + + if ('=' == c) break; - } else if (' ' == c || '\t' == c) { + + if (' ' == c || '\t' == c) { for (;;) { - r.mark(1); - c = r.read(); - if (c < 0) { - throw new IOException("Unexpected end of config file."); - } else if ('=' == c) { + c = in.read(); + if (c < 0) + throw new ConfigInvalidException("Unexpected end of config file"); + + if ('=' == c) break; - } else if (';' == c || '#' == c || '\n' == c) { - r.reset(); + + if (';' == c || '#' == c || '\n' == c) { + in.reset(); break; - } else if (' ' == c || '\t' == c) { - // Skipped... - } else { - throw new IOException("Bad entry delimiter."); } + + if (' ' == c || '\t' == c) + continue; // Skipped... + throw new ConfigInvalidException("Bad entry delimiter"); } break; - } else if (Character.isLetterOrDigit((char) c) || c == '-') { + } + + if (Character.isLetterOrDigit((char) c) || c == '-') { // From the git-config man page: // The variable names are case-insensitive and only // alphanumeric characters and - are allowed. name.append((char) c); } else if ('\n' == c) { - r.reset(); + in.reset(); name.append((char) c); break; - } else { - throw new IOException("Bad config entry name: " + name - + (char) c); - } + } else + throw new ConfigInvalidException("Bad entry name: " + name); } return name.toString(); } - private static String readValue(final BufferedReader r, boolean quote, - final int eol) throws IOException { + private static String readValue(final StringReader in, boolean quote, + final int eol) throws ConfigInvalidException { final StringBuffer value = new StringBuffer(); boolean space = false; for (;;) { - r.mark(1); - int c = r.read(); + int c = in.read(); if (c < 0) { if (value.length() == 0) - throw new IOException("Unexpected end of config file."); + throw new ConfigInvalidException("Unexpected end of config file"); break; } + if ('\n' == c) { - if (quote) { - throw new IOException("Newline in quotes not allowed."); - } - r.reset(); + if (quote) + throw new ConfigInvalidException("Newline in quotes not allowed"); + in.reset(); break; } - if (eol == c) { + + if (eol == c) break; - } + if (!quote) { if (Character.isWhitespace((char) c)) { space = true; continue; } if (';' == c || '#' == c) { - r.reset(); + in.reset(); break; } } + if (space) { - if (value.length() > 0) { + if (value.length() > 0) value.append(' '); - } space = false; } + if ('\\' == c) { - c = r.read(); + c = in.read(); switch (c) { case -1: - throw new IOException("End of file in escape."); + throw new ConfigInvalidException("End of file in escape"); case '\n': continue; case 't': @@ -971,13 +934,15 @@ private static String readValue(final BufferedReader r, boolean quote, value.append('"'); continue; default: - throw new IOException("Bad escape: " + ((char) c)); + throw new ConfigInvalidException("Bad escape: " + ((char) c)); } } + if ('"' == c) { quote = !quote; continue; } + value.append((char) c); } return value.length() > 0 ? value.toString() : null; @@ -1040,4 +1005,27 @@ private static boolean eqSameCase(final String a, final String b) { return a.equals(b); } } + + private static class StringReader { + private final char[] buf; + + private int pos; + + StringReader(final String in) { + buf = in.toCharArray(); + } + + int read() { + try { + return buf[pos++]; + } catch (ArrayIndexOutOfBoundsException e) { + pos = buf.length; + return -1; + } + } + + void reset() { + pos--; + } + } } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java index aa1dbee..a419e7f 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java @@ -40,20 +40,18 @@ */ package org.spearce.jgit.lib; -import java.io.BufferedWriter; import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; +import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; + +import org.spearce.jgit.errors.ConfigInvalidException; +import org.spearce.jgit.util.NB; +import org.spearce.jgit.util.RawParseUtils; /** * The configuration file that is stored in the file of the file system. */ public class FileBasedConfig extends Config { - private final File configFile; /** @@ -69,47 +67,58 @@ public FileBasedConfig(Config base, File cfgLocation) { configFile = cfgLocation; } + /** @return location of the configuration file on disk */ + public final File getFile() { + return configFile; + } + /** - * Save config data to the git config file + * Load the configuration as a Git text style configuration file. + * <p> + * If the file does not exist, this configuration is cleared, and thus + * behaves the same as though the file exists, but is empty. * * @throws IOException + * the file could not be read (but does exist). + * @throws ConfigInvalidException + * the file is not a properly formatted configuration file. */ - public void save() throws IOException { - final File tmp = new File(configFile.getParentFile(), configFile - .getName() - + ".lock"); - final PrintWriter r = new PrintWriter(new BufferedWriter( - new OutputStreamWriter(new FileOutputStream(tmp), - Constants.CHARSET))); - boolean ok = false; + public void load() throws IOException, ConfigInvalidException { try { - printConfig(r); - ok = true; - r.close(); - if (!tmp.renameTo(configFile)) { - configFile.delete(); - if (!tmp.renameTo(configFile)) - throw new IOException("Cannot save config file " - + configFile + ", rename failed"); - } - } finally { - r.close(); - if (tmp.exists() && !tmp.delete()) { - System.err - .println("(warning) failed to delete tmp config file: " - + tmp); - } + fromText(RawParseUtils.decode(NB.readFully(getFile()))); + } catch (FileNotFoundException noFile) { + clear(); } - setFileRead(ok); } - @Override - protected InputStream openInputStream() throws IOException { - return new FileInputStream(configFile); + /** + * Save the configuration as a Git text style configuration file. + * <p> + * <b>Warning:</b> Although this method uses the traditional Git file + * locking approach to protect against concurrent writes of the + * configuration file, it does not ensure that the file has not been + * modified since the last read, which means updates performed by other + * objects accessing the same backing file may be lost. + * + * @throws IOException + * the file could not be written. + */ + public void save() throws IOException { + final byte[] out = Constants.encode(toText()); + final LockFile lf = new LockFile(getFile()); + if (!lf.lock()) + throw new IOException("Cannot lock " + getFile()); + try { + lf.write(out); + if (!lf.commit()) + throw new IOException("Cannot commit write to " + getFile()); + } finally { + lf.unlock(); + } } @Override public String toString() { - return getClass().getSimpleName() + "[" + configFile.getPath() + "]"; + return getClass().getSimpleName() + "[" + getFile().getPath() + "]"; } } \ No newline at end of file diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java index 98a276b..4e987e1 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java @@ -56,9 +56,11 @@ import java.util.Vector; import java.util.concurrent.atomic.AtomicInteger; +import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.errors.IncorrectObjectTypeException; import org.spearce.jgit.errors.RevisionSyntaxException; import org.spearce.jgit.util.FS; +import org.spearce.jgit.util.SystemReader; /** * Represents a Git repository. A repository holds all objects and refs used for @@ -114,10 +116,28 @@ public Repository(final File d) throws IOException { gitDir = d.getAbsoluteFile(); refs = new RefDatabase(this); objectDatabase = new ObjectDirectory(FS.resolve(gitDir, "objects")); - config = new RepositoryConfig(this); + + final FileBasedConfig userConfig; + userConfig = SystemReader.getInstance().openUserConfig(); + try { + userConfig.load(); + } catch (ConfigInvalidException e1) { + IOException e2 = new IOException("User config file " + + userConfig.getFile().getAbsolutePath() + " invalid: " + + e1); + e2.initCause(e1); + throw e2; + } + config = new RepositoryConfig(userConfig, FS.resolve(gitDir, "config")); if (objectDatabase.exists()) { - getConfig().load(); + try { + getConfig().load(); + } catch (ConfigInvalidException e1) { + IOException e2 = new IOException("Unknown repository format"); + e2.initCause(e1); + throw e2; + } final String repositoryFormatVersion = getConfig().getString( "core", null, "repositoryFormatVersion"); if (!"0".equals(repositoryFormatVersion)) { diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index 5c912b7..fb97747 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -44,7 +44,7 @@ import java.io.File; import java.io.IOException; -import org.spearce.jgit.util.FS; +import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.util.SystemReader; /** @@ -54,16 +54,6 @@ * file depending on how it is instantiated. */ public class RepositoryConfig extends FileBasedConfig { - /** - * Obtain a new configuration instance for ~/.gitconfig. - * - * @return a new configuration instance to read the user's global - * configuration file from their home directory. - */ - public static FileBasedConfig openUserConfig() { - return SystemReader.getInstance().openUserConfig(); - } - /** Section name for a branch configuration. */ public static final String BRANCH_SECTION = "branch"; @@ -71,10 +61,6 @@ public static FileBasedConfig openUserConfig() { TransferConfig transfer; - RepositoryConfig(final Repository repo) { - this(openUserConfig(), FS.resolve(repo.getDirectory(), "config")); - } - /** * Create a Git configuration file reader/writer/cache for a specific file. * @@ -187,7 +173,6 @@ private String getUserEmailInternal(String gitVariableKey) { */ public void create() { clear(); - setFileRead(true); setInt("core", null, "repositoryformatversion", 0); setBoolean("core", null, "filemode", true); @@ -196,7 +181,7 @@ public void create() { } @Override - public void load() throws IOException { + public void load() throws IOException, ConfigInvalidException { super.load(); core = new CoreConfig(this); transfer = new TransferConfig(this); -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 10/19] Test for the config file when creating a new repository 2009-07-25 18:52 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 11/19] Remove the map lookup for values in Config Shawn O. Pearce 2009-07-25 22:54 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Robin Rosenberg 1 sibling, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git If the configuration file already exists then we should assume that the repository also exists. Rather than computing the file name we should rely upon the computation done in the constructor, whose result is held in the RepositoryConfig's file property. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Repository.java | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java index 4e987e1..7fb1ef7 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java @@ -171,7 +171,8 @@ public synchronized void create() throws IOException { * in case of IO problem */ public void create(boolean bare) throws IOException { - if ((bare ? new File(gitDir, "config") : gitDir).exists()) { + final RepositoryConfig cfg = getConfig(); + if (cfg.getFile().exists()) { throw new IllegalStateException("Repository already exists: " + gitDir); } @@ -184,7 +185,6 @@ public void create(boolean bare) throws IOException { final String master = Constants.R_HEADS + Constants.MASTER; refs.link(Constants.HEAD, master); - RepositoryConfig cfg = getConfig(); cfg.create(); if (bare) cfg.setBoolean("core", null, "bare", true); -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 11/19] Remove the map lookup for values in Config 2009-07-25 18:52 ` [JGIT PATCH 10/19] Test for the config file when creating a new repository Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 12/19] Return base values first from Config.getStringList() Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Update operations are already O(N) time in the config file, as we scan through every entry in the file, in order, looking to see if it matches the key we need to mutate. As most configurations are rather small in size the running time is trivial. Doing the same for lookup simplifies our code considerably. Applications really want a different type of fast lookup strategy. Instead of by key lookups they want something like CoreConfig or RemoteConfig to be cached and quickly obtainable on demand, where the all relevant values have been parsed from their string content representation into the application specific data types. This change does not provide that functionality, but rather assumes we will do so in the near future. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Config.java | 129 +++++--------------- 1 files changed, 30 insertions(+), 99 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index d63e926..0f91412 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -43,10 +43,8 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import org.spearce.jgit.errors.ConfigInvalidException; @@ -66,11 +64,6 @@ final Config baseConfig; /** - * Map from name to values - */ - private Map<String, Object> byName; - - /** * Magic value indicating a missing entry. * <p> * This value is tested for reference equality in some contexts, so we @@ -334,19 +327,9 @@ public String getString(final String section, String subsection, */ public String[] getStringList(final String section, String subsection, final String name) { - final Object o = getRawEntry(section, subsection, name); - if (o instanceof List) { - final List lst = (List) o; - final String[] r = new String[lst.size()]; - for (int i = 0; i < r.length; i++) - r[i] = ((Entry) lst.get(i)).value; - return r; - } - - if (o instanceof Entry) { - return new String[] { ((Entry) o).value }; - } - + final List<String> lst = getRawStringList(section, subsection, name); + if (lst != null) + return lst.toArray(new String[lst.size()]); if (baseConfig != null) return baseConfig.getStringList(section, subsection, name); return new String[0]; @@ -373,43 +356,36 @@ public String getString(final String section, String subsection, private String getRawString(final String section, final String subsection, final String name) { - final Object o = getRawEntry(section, subsection, name); - if (o instanceof List) { - return ((Entry) ((List) o).get(0)).value; - } else if (o instanceof Entry) { - return ((Entry) o).value; - } else if (baseConfig != null) + final List<String> lst = getRawStringList(section, subsection, name); + if (lst != null) + return lst.get(0); + else if (baseConfig != null) return baseConfig.getRawString(section, subsection, name); else return null; } - private Object getRawEntry(final String section, final String subsection, - final String name) { - return byName.get(concatenateKey(section, subsection, name)); + private List<String> getRawStringList(final String section, + final String subsection, final String name) { + List<String> r = null; + for (final Entry e : entries) { + if (e.match(section, subsection, name)) + r = add(r, e.value); + } + return r; } - /** - * Create simple a key name from the key components - * - * @param section - * the section name - * @param subsection - * the subsection name - * @param name - * the key name - * @return a simple key name that have all components concatenated and the - * case converted - */ - private static String concatenateKey(final String section, - final String subsection, final String name) { - String ss; - if (subsection != null) - ss = "." + subsection; - else - ss = ""; - return StringUtils.toLowerCase(section) + ss + "." - + StringUtils.toLowerCase(name); + private static List<String> add(final List<String> curr, final String value) { + if (curr == null) + return Collections.singletonList(value); + if (curr.size() == 1) { + final List<String> r = new ArrayList<String>(2); + r.add(curr.get(0)); + r.add(value); + return r; + } + curr.add(value); + return curr; } /** @@ -551,31 +527,6 @@ public void unset(final String section, final String subsection, */ public void setStringList(final String section, final String subsection, final String name, final List<String> values) { - // Update our parsed cache of values for future reference. - // - String key = concatenateKey(section, subsection, name); - if (values.size() == 0) - byName.remove(key); - else if (values.size() == 1) { - final Entry e = new Entry(); - e.section = section; - e.subsection = subsection; - e.name = name; - e.value = values.get(0); - byName.put(key, e); - } else { - final ArrayList<Entry> eList = new ArrayList<Entry>(values.size()); - for (final String v : values) { - final Entry e = new Entry(); - e.section = section; - e.subsection = subsection; - e.name = name; - e.value = v; - eList.add(e); - } - byName.put(key, eList); - } - int entryIndex = 0; int valueIndex = 0; int insertPosition = -1; @@ -697,9 +648,7 @@ public String toText() { * made to {@code this}. */ public void fromText(final String text) throws ConfigInvalidException { - entries = new ArrayList<Entry>(); - byName = new HashMap<String, Object>(); - + final List<Entry> newEntries = new ArrayList<Entry>(); final StringReader in = new StringReader(text); Entry last = null; Entry e = new Entry(); @@ -711,7 +660,7 @@ public void fromText(final String text) throws ConfigInvalidException { final char c = (char) input; if ('\n' == c) { // End of this entry. - add(e); + newEntries.add(e); if (e.section != null) last = e; e = new Entry(); @@ -757,6 +706,8 @@ public void fromText(final String text) throws ConfigInvalidException { } else throw new ConfigInvalidException("Invalid line in config file"); } + + entries = newEntries; } /** @@ -764,26 +715,6 @@ public void fromText(final String text) throws ConfigInvalidException { */ protected void clear() { entries = new ArrayList<Entry>(); - byName = new HashMap<String, Object>(); - } - - @SuppressWarnings("unchecked") - private void add(final Entry e) { - entries.add(e); - if (e.section != null && e.name != null) { - final String key = concatenateKey(e.section, e.subsection, e.name); - final Object o = byName.get(key); - if (o == null) { - byName.put(key, e); - } else if (o instanceof Entry) { - final ArrayList<Object> l = new ArrayList<Object>(); - l.add(o); - l.add(e); - byName.put(key, l); - } else if (o instanceof List) { - ((List<Entry>) o).add(e); - } - } } private static String readSectionName(final StringReader in) -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 12/19] Return base values first from Config.getStringList() 2009-07-25 18:52 ` [JGIT PATCH 11/19] Remove the map lookup for values in Config Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 13/19] Make Config thread safe by using copy-on-write semantics Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git C Git's `git config --get-all foo.bar` returns the base configuration values for foo.bar (aka those found in /etc/gitconfig or ~/.gitconfig) before the repository specific configuration values for foo.bar. To better match C Git behavior when processing a multi-value key we must do the same in our getStringList() method. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Config.java | 24 +++++++++++++++---- 1 files changed, 19 insertions(+), 5 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index 0f91412..f4e78f3 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -55,6 +55,7 @@ * Git style {@code .config}, {@code .gitconfig}, {@code .gitmodules} file. */ public class Config { + private static final String[] EMPTY_STRING_ARRAY = {}; private static final long KiB = 1024; private static final long MiB = 1024 * KiB; private static final long GiB = 1024 * MiB; @@ -316,6 +317,9 @@ public String getString(final String section, String subsection, /** * Get a list of string values + * <p> + * If this instance was created with a base, the base's values are returned + * first (if any). * * @param section * the section @@ -327,12 +331,22 @@ public String getString(final String section, String subsection, */ public String[] getStringList(final String section, String subsection, final String name) { - final List<String> lst = getRawStringList(section, subsection, name); - if (lst != null) - return lst.toArray(new String[lst.size()]); + final String[] baseList; if (baseConfig != null) - return baseConfig.getStringList(section, subsection, name); - return new String[0]; + baseList = baseConfig.getStringList(section, subsection, name); + else + baseList = EMPTY_STRING_ARRAY; + + final List<String> lst = getRawStringList(section, subsection, name); + if (lst != null) { + final String[] res = new String[baseList.length + lst.size()]; + int idx = baseList.length; + System.arraycopy(baseList, 0, res, 0, idx); + for (final String val : lst) + res[idx++] = val; + return res; + } + return baseList; } /** -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 13/19] Make Config thread safe by using copy-on-write semantics 2009-07-25 18:52 ` [JGIT PATCH 12/19] Return base values first from Config.getStringList() Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 14/19] Support cached application models in a Config Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git It is relatively rare that the Config instance is modified from application code. When it is, we usually are working on a small set of values read from .git/config or ~/.gitconfig. Making the class thread-safe through basic copy-on-write semantics for its inner collection of entries simplifies the locking and permits us to optimize for the much more common read path. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Config.java | 85 +++++++++++++++++--- 1 files changed, 72 insertions(+), 13 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index f4e78f3..4d4b315 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -46,6 +46,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.util.StringUtils; @@ -60,9 +61,15 @@ private static final long MiB = 1024 * KiB; private static final long GiB = 1024 * MiB; - private List<Entry> entries; + /** + * Immutable current state of the configuration data. + * <p> + * This state is copy-on-write. It should always contain an immutable list + * of the configuration keys/values. + */ + private final AtomicReference<State> state; - final Config baseConfig; + private final Config baseConfig; /** * Magic value indicating a missing entry. @@ -87,7 +94,7 @@ public Config() { */ public Config(Config defaultConfig) { baseConfig = defaultConfig; - clear(); + state = new AtomicReference<State>(newState()); } /** @@ -358,7 +365,7 @@ public String getString(final String section, String subsection, */ public Set<String> getSubsections(final String section) { final Set<String> result = new HashSet<String>(); - for (final Entry e : entries) { + for (final Entry e : state.get().entryList) { if (StringUtils.equalsIgnoreCase(section, e.section) && e.subsection != null) result.add(e.subsection); @@ -382,7 +389,7 @@ else if (baseConfig != null) private List<String> getRawStringList(final String section, final String subsection, final String name) { List<String> r = null; - for (final Entry e : entries) { + for (final Entry e : state.get().entryList) { if (e.match(section, subsection, name)) r = add(r, e.value); } @@ -541,6 +548,17 @@ public void unset(final String section, final String subsection, */ public void setStringList(final String section, final String subsection, final String name, final List<String> values) { + State src, res; + do { + src = state.get(); + res = replaceStringList(src, section, subsection, name, values); + } while (!state.compareAndSet(src, res)); + } + + private State replaceStringList(final State srcState, + final String section, final String subsection, final String name, + final List<String> values) { + final List<Entry> entries = copy(srcState, values); int entryIndex = 0; int valueIndex = 0; int insertPosition = -1; @@ -548,11 +566,12 @@ public void setStringList(final String section, final String subsection, // Reset the first n Entry objects that match this input name. // while (entryIndex < entries.size() && valueIndex < values.size()) { - final Entry e = entries.get(entryIndex++); + final Entry e = entries.get(entryIndex); if (e.match(section, subsection, name)) { - e.value = values.get(valueIndex++); - insertPosition = entryIndex; + entries.set(entryIndex, e.forValue(values.get(valueIndex++))); + insertPosition = entryIndex + 1; } + entryIndex++; } // Remove any extra Entry objects that we no longer need. @@ -573,7 +592,7 @@ public void setStringList(final String section, final String subsection, // is already a section available that matches. Insert // after the last key of that section. // - insertPosition = findSectionEnd(section, subsection); + insertPosition = findSectionEnd(entries, section, subsection); } if (insertPosition < 0) { // We didn't find any matching section header for this key, @@ -594,9 +613,22 @@ public void setStringList(final String section, final String subsection, entries.add(insertPosition++, e); } } + + return newState(entries); + } + + private static List<Entry> copy(final State src, final List<String> values) { + // At worst we need to insert 1 line for each value, plus 1 line + // for a new section header. Assume that and allocate the space. + // + final int max = src.entryList.size() + values.size() + 1; + final ArrayList<Entry> r = new ArrayList<Entry>(max); + r.addAll(src.entryList); + return r; } - private int findSectionEnd(final String section, final String subsection) { + private static int findSectionEnd(final List<Entry> entries, + final String section, final String subsection) { for (int i = 0; i < entries.size(); i++) { Entry e = entries.get(i); if (e.match(section, subsection, null)) { @@ -619,7 +651,7 @@ private int findSectionEnd(final String section, final String subsection) { */ public String toText() { final StringBuilder out = new StringBuilder(); - for (final Entry e : entries) { + for (final Entry e : state.get().entryList) { if (e.prefix != null) out.append(e.prefix); if (e.section != null && e.name == null) { @@ -721,14 +753,22 @@ public void fromText(final String text) throws ConfigInvalidException { throw new ConfigInvalidException("Invalid line in config file"); } - entries = newEntries; + state.set(newState(newEntries)); + } + + private State newState() { + return new State(Collections.<Entry> emptyList()); + } + + private State newState(final List<Entry> entries) { + return new State(Collections.unmodifiableList(entries)); } /** * Clear the configuration file */ protected void clear() { - entries = new ArrayList<Entry>(); + state.set(newState()); } private static String readSectionName(final StringReader in) @@ -893,6 +933,14 @@ private static String readValue(final StringReader in, boolean quote, return value.length() > 0 ? value.toString() : null; } + private static class State { + final List<Entry> entryList; + + State(List<Entry> entries) { + entryList = entries; + } + } + /** * The configuration file entry */ @@ -927,6 +975,17 @@ private static String readValue(final StringReader in, boolean quote, */ String suffix; + Entry forValue(final String newValue) { + final Entry e = new Entry(); + e.prefix = prefix; + e.section = section; + e.subsection = subsection; + e.name = name; + e.value = newValue; + e.suffix = suffix; + return e; + } + boolean match(final String aSection, final String aSubsection, final String aKey) { return eqIgnoreCase(section, aSection) -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 14/19] Support cached application models in a Config 2009-07-25 18:52 ` [JGIT PATCH 13/19] Make Config thread safe by using copy-on-write semantics Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 15/19] Cache Config subsection names when requested by application code Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git, Constantine Plotnikov The Config.SectionParser acts as a key into a cache of model object instances, but also knows how to create its model instance if the cache does not contain the corresponding instance. Because we don't keep track of which keys were accessed by a parser when it created its model, we discard the entire cache anytime any key is modified, forcing all models to reparse on the next access. Both CoreConfig and TransferConfig are trivial to switch to this method of access, although the typical get path is quite a bit longer as we now need to do a hash lookup instead of a getfield instruction. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> CC: Constantine Plotnikov <constantine.plotnikov@gmail.com> --- .../src/org/spearce/jgit/lib/Config.java | 92 +++++++++++++++++++- .../src/org/spearce/jgit/lib/CoreConfig.java | 16 +++- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 20 +---- .../src/org/spearce/jgit/lib/TransferConfig.java | 11 ++- 4 files changed, 112 insertions(+), 27 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index 4d4b315..6036c4c 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -45,7 +45,9 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import org.spearce.jgit.errors.ConfigInvalidException; @@ -375,6 +377,43 @@ public String getString(final String section, String subsection, return result; } + /** + * Obtain a handle to a parsed set of configuration values. + * + * @param <T> + * type of configuration model to return. + * @param parser + * parser which can create the model if it is not already + * available in this configuration file. The parser is also used + * as the key into a cache and must obey the hashCode and equals + * contract in order to reuse a parsed model. + * @return the parsed object instance, which is cached inside this config. + */ + @SuppressWarnings("unchecked") + public <T> T get(final SectionParser<T> parser) { + final State myState = getState(); + T obj = (T) myState.cache.get(parser); + if (obj == null) { + obj = parser.parse(this); + myState.cache.put(parser, obj); + } + return obj; + } + + /** + * Remove a cached configuration object. + * <p> + * If the associated configuration object has not yet been cached, this + * method has no effect. + * + * @param parser + * parser used to obtain the configuration object. + * @see #get(SectionParser) + */ + public void uncache(final SectionParser<?> parser) { + state.get().cache.remove(parser); + } + private String getRawString(final String section, final String subsection, final String name) { final List<String> lst = getRawStringList(section, subsection, name); @@ -409,6 +448,22 @@ else if (baseConfig != null) return curr; } + private State getState() { + State cur, upd; + do { + cur = state.get(); + final State base = getBaseState(); + if (cur.baseState == base) + return cur; + upd = new State(cur.entryList, base); + } while (!state.compareAndSet(cur, upd)); + return upd; + } + + private State getBaseState() { + return baseConfig != null ? baseConfig.getState() : null; + } + /** * Add or modify a configuration value. The parameters will result in a * configuration entry like this. @@ -757,11 +812,11 @@ public void fromText(final String text) throws ConfigInvalidException { } private State newState() { - return new State(Collections.<Entry> emptyList()); + return new State(Collections.<Entry> emptyList(), getBaseState()); } private State newState(final List<Entry> entries) { - return new State(Collections.unmodifiableList(entries)); + return new State(Collections.unmodifiableList(entries), getBaseState()); } /** @@ -933,11 +988,42 @@ private static String readValue(final StringReader in, boolean quote, return value.length() > 0 ? value.toString() : null; } + /** + * Parses a section of the configuration into an application model object. + * <p> + * Instances must implement hashCode and equals such that model objects can + * be cached by using the {@code SectionParser} as a key of a HashMap. + * <p> + * As the {@code SectionParser} itself is used as the key of the internal + * HashMap applications should be careful to ensure the SectionParser key + * does not retain unnecessary application state which may cause memory to + * be held longer than expected. + * + * @param <T> + * type of the application model created by the parser. + */ + public static interface SectionParser<T> { + /** + * Create a model object from a configuration. + * + * @param cfg + * the configuration to read values from. + * @return the application model instance. + */ + T parse(Config cfg); + } + private static class State { final List<Entry> entryList; - State(List<Entry> entries) { + final Map<Object, Object> cache; + + final State baseState; + + State(List<Entry> entries, State base) { entryList = entries; + cache = new ConcurrentHashMap<Object, Object>(16, 0.75f, 1); + baseState = base; } } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java index e98e0bc..ed3827b 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java @@ -38,22 +38,28 @@ package org.spearce.jgit.lib; -import java.util.zip.Deflater; +import static java.util.zip.Deflater.DEFAULT_COMPRESSION; + +import org.spearce.jgit.lib.Config.SectionParser; /** * This class keeps git repository core parameters. */ public class CoreConfig { - private static final int DEFAULT_COMPRESSION = Deflater.DEFAULT_COMPRESSION; - private static final int DEFAULT_INDEXVERSION = 2; + /** Key for {@link Config#get(SectionParser)}. */ + public static final Config.SectionParser<CoreConfig> KEY = new SectionParser<CoreConfig>() { + public CoreConfig parse(final Config cfg) { + return new CoreConfig(cfg); + } + }; private final int compression; private final int packIndexVersion; - CoreConfig(final RepositoryConfig rc) { + private CoreConfig(final Config rc) { compression = rc.getInt("core", "compression", DEFAULT_COMPRESSION); - packIndexVersion = rc.getInt("pack", "indexversion", DEFAULT_INDEXVERSION); + packIndexVersion = rc.getInt("pack", "indexversion", 2); } /** diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index fb97747..c6a13b6 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -42,9 +42,7 @@ package org.spearce.jgit.lib; import java.io.File; -import java.io.IOException; -import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.util.SystemReader; /** @@ -57,10 +55,6 @@ /** Section name for a branch configuration. */ public static final String BRANCH_SECTION = "branch"; - CoreConfig core; - - TransferConfig transfer; - /** * Create a Git configuration file reader/writer/cache for a specific file. * @@ -79,14 +73,14 @@ public RepositoryConfig(final Config base, final File cfgLocation) { * @return Core configuration values */ public CoreConfig getCore() { - return core; + return get(CoreConfig.KEY); } /** * @return transfer, fetch and receive configuration values */ public TransferConfig getTransfer() { - return transfer; + return get(TransferConfig.KEY); } /** @@ -175,15 +169,5 @@ public void create() { clear(); setInt("core", null, "repositoryformatversion", 0); setBoolean("core", null, "filemode", true); - - core = new CoreConfig(this); - transfer = new TransferConfig(this); - } - - @Override - public void load() throws IOException, ConfigInvalidException { - super.load(); - core = new CoreConfig(this); - transfer = new TransferConfig(this); } } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java index ff3a5eb..8760103 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/TransferConfig.java @@ -37,13 +37,22 @@ package org.spearce.jgit.lib; +import org.spearce.jgit.lib.Config.SectionParser; + /** * The standard "transfer", "fetch" and "receive" configuration parameters. */ public class TransferConfig { + /** Key for {@link Config#get(SectionParser)}. */ + public static final Config.SectionParser<TransferConfig> KEY = new SectionParser<TransferConfig>() { + public TransferConfig parse(final Config cfg) { + return new TransferConfig(cfg); + } + }; + private final boolean fsckObjects; - TransferConfig(final RepositoryConfig rc) { + private TransferConfig(final Config rc) { fsckObjects = rc.getBoolean("receive", "fsckobjects", false); } -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 15/19] Cache Config subsection names when requested by application code 2009-07-25 18:52 ` [JGIT PATCH 14/19] Support cached application models in a Config Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 16/19] Refactor author/committer lookup to use cached data Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Using the SectionParser based cache allows us to read through the configuration once to produce the set of available subsections, but then reuse that set in the future. This makes lookups like "find all remotes" more efficient. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Config.java | 42 +++++++++++++++---- 1 files changed, 33 insertions(+), 9 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java index 6036c4c..1668a3f 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java @@ -366,15 +366,7 @@ public String getString(final String section, String subsection, * subsection exists. */ public Set<String> getSubsections(final String section) { - final Set<String> result = new HashSet<String>(); - for (final Entry e : state.get().entryList) { - if (StringUtils.equalsIgnoreCase(section, e.section) - && e.subsection != null) - result.add(e.subsection); - } - if (baseConfig != null) - result.addAll(baseConfig.getSubsections(section)); - return result; + return get(new SubsectionNames(section)); } /** @@ -1013,6 +1005,38 @@ private static String readValue(final StringReader in, boolean quote, T parse(Config cfg); } + private static class SubsectionNames implements SectionParser<Set<String>> { + private final String section; + + SubsectionNames(final String sectionName) { + section = sectionName; + } + + public int hashCode() { + return section.hashCode(); + } + + public boolean equals(Object other) { + if (other instanceof SubsectionNames) { + return section.equals(((SubsectionNames) other).section); + } + return false; + } + + public Set<String> parse(Config cfg) { + final Set<String> result = new HashSet<String>(); + while (cfg != null) { + for (final Entry e : cfg.state.get().entryList) { + if (e.subsection != null && e.name == null + && StringUtils.equalsIgnoreCase(section, e.section)) + result.add(e.subsection); + } + cfg = cfg.baseConfig; + } + return Collections.unmodifiableSet(result); + } + } + private static class State { final List<Entry> entryList; -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 16/19] Refactor author/committer lookup to use cached data 2009-07-25 18:52 ` [JGIT PATCH 15/19] Cache Config subsection names when requested by application code Shawn O. Pearce @ 2009-07-25 18:52 ` Shawn O. Pearce 2009-07-25 18:53 ` [JGIT PATCH 17/19] Move repository config creation fully into Repository class Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:52 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git The author and committer don't typically change when processing, so we can maintain them in a cached entity just like we do with the TransferConfig and the CoreConfig. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/lib/RepositoryConfigTest.java | 29 ++-- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 56 ++------ .../src/org/spearce/jgit/lib/UserConfig.java | 149 ++++++++++++++++++++ 3 files changed, 173 insertions(+), 61 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/UserConfig.java diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java index d4a6dd2..67d9964 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java @@ -109,55 +109,56 @@ public void test007_readUserConfig() { SystemReader.setInstance(mockSystemReader); final String hostname = mockSystemReader.getHostname(); final Config userGitConfig = mockSystemReader.userGitConfig; - final RepositoryConfig localConfig = new RepositoryConfig(userGitConfig, null); - localConfig.create(); + final Config localConfig = new Config(userGitConfig); mockSystemReader.values.clear(); String authorName; String authorEmail; // no values defined nowhere - authorName = localConfig.getAuthorName(); - authorEmail = localConfig.getAuthorEmail(); + authorName = localConfig.get(UserConfig.KEY).getAuthorName(); + authorEmail = localConfig.get(UserConfig.KEY).getAuthorEmail(); assertEquals(Constants.UNKNOWN_USER_DEFAULT, authorName); assertEquals(Constants.UNKNOWN_USER_DEFAULT + "@" + hostname, authorEmail); // the system user name is defined mockSystemReader.values.put(Constants.OS_USER_NAME_KEY, "os user name"); - authorName = localConfig.getAuthorName(); + localConfig.uncache(UserConfig.KEY); + authorName = localConfig.get(UserConfig.KEY).getAuthorName(); assertEquals("os user name", authorName); if (hostname != null && hostname.length() != 0) { - authorEmail = localConfig.getAuthorEmail(); + authorEmail = localConfig.get(UserConfig.KEY).getAuthorEmail(); assertEquals("os user name@" + hostname, authorEmail); } // the git environment variables are defined mockSystemReader.values.put(Constants.GIT_AUTHOR_NAME_KEY, "git author name"); mockSystemReader.values.put(Constants.GIT_AUTHOR_EMAIL_KEY, "author@email"); - authorName = localConfig.getAuthorName(); - authorEmail = localConfig.getAuthorEmail(); + localConfig.uncache(UserConfig.KEY); + authorName = localConfig.get(UserConfig.KEY).getAuthorName(); + authorEmail = localConfig.get(UserConfig.KEY).getAuthorEmail(); assertEquals("git author name", authorName); assertEquals("author@email", authorEmail); // the values are defined in the global configuration userGitConfig.setString("user", null, "name", "global username"); userGitConfig.setString("user", null, "email", "author@globalemail"); - authorName = localConfig.getAuthorName(); - authorEmail = localConfig.getAuthorEmail(); + authorName = localConfig.get(UserConfig.KEY).getAuthorName(); + authorEmail = localConfig.get(UserConfig.KEY).getAuthorEmail(); assertEquals("global username", authorName); assertEquals("author@globalemail", authorEmail); // the values are defined in the local configuration localConfig.setString("user", null, "name", "local username"); localConfig.setString("user", null, "email", "author@localemail"); - authorName = localConfig.getAuthorName(); - authorEmail = localConfig.getAuthorEmail(); + authorName = localConfig.get(UserConfig.KEY).getAuthorName(); + authorEmail = localConfig.get(UserConfig.KEY).getAuthorEmail(); assertEquals("local username", authorName); assertEquals("author@localemail", authorEmail); - authorName = localConfig.getCommitterName(); - authorEmail = localConfig.getCommitterEmail(); + authorName = localConfig.get(UserConfig.KEY).getCommitterName(); + authorEmail = localConfig.get(UserConfig.KEY).getCommitterEmail(); assertEquals("local username", authorName); assertEquals("author@localemail", authorEmail); } diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index c6a13b6..15fe9de 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -43,8 +43,6 @@ import java.io.File; -import org.spearce.jgit.util.SystemReader; - /** * An object representing the Git config file. * @@ -83,13 +81,18 @@ public TransferConfig getTransfer() { return get(TransferConfig.KEY); } + /** @return standard user configuration data */ + public UserConfig getUserConfig() { + return get(UserConfig.KEY); + } + /** * @return the author name as defined in the git variables * and configurations. If no name could be found, try * to use the system user name instead. */ public String getAuthorName() { - return getUsernameInternal(Constants.GIT_AUTHOR_NAME_KEY); + return getUserConfig().getAuthorName(); } /** @@ -98,26 +101,7 @@ public String getAuthorName() { * to use the system user name instead. */ public String getCommitterName() { - return getUsernameInternal(Constants.GIT_COMMITTER_NAME_KEY); - } - - private String getUsernameInternal(String gitVariableKey) { - SystemReader systemReader = SystemReader.getInstance(); - // try to get the user name from the local and global configurations. - String username = getString("user", null, "name"); - - if (username == null) { - // try to get the user name for the system property GIT_XXX_NAME - username = systemReader.getenv(gitVariableKey); - } - if (username == null) { - // get the system user name - username = systemReader.getProperty(Constants.OS_USER_NAME_KEY); - } - if (username == null) { - username = Constants.UNKNOWN_USER_DEFAULT; - } - return username; + return getUserConfig().getCommitterName(); } /** @@ -127,7 +111,7 @@ private String getUsernameInternal(String gitVariableKey) { * host name. */ public String getAuthorEmail() { - return getUserEmailInternal(Constants.GIT_AUTHOR_EMAIL_KEY); + return getUserConfig().getAuthorEmail(); } /** @@ -137,29 +121,7 @@ public String getAuthorEmail() { * host name. */ public String getCommitterEmail() { - return getUserEmailInternal(Constants.GIT_COMMITTER_EMAIL_KEY); - } - - private String getUserEmailInternal(String gitVariableKey) { - SystemReader systemReader = SystemReader.getInstance(); - // try to get the email from the local and global configurations. - String email = getString("user", null, "email"); - - if (email == null) { - // try to get the email for the system property GIT_XXX_EMAIL - email = systemReader.getenv(gitVariableKey); - } - - if (email == null) { - // try to construct an email - String username = systemReader.getProperty(Constants.OS_USER_NAME_KEY); - if (username == null){ - username = Constants.UNKNOWN_USER_DEFAULT; - } - email = username + "@" + systemReader.getHostname(); - } - - return email; + return getUserConfig().getCommitterEmail(); } /** diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UserConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UserConfig.java new file mode 100644 index 0000000..27c7d69 --- /dev/null +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UserConfig.java @@ -0,0 +1,149 @@ +/* + * Copyright (C) 2009, Yann Simon <yann.simon.fr@gmail.com> + * Copyright (C) 2009, Google Inc. + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Git Development Community nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.spearce.jgit.lib; + +import org.spearce.jgit.lib.Config.SectionParser; +import org.spearce.jgit.util.SystemReader; + +/** The standard "user" configuration parameters. */ +public class UserConfig { + /** Key for {@link Config#get(SectionParser)}. */ + public static final Config.SectionParser<UserConfig> KEY = new SectionParser<UserConfig>() { + public UserConfig parse(final Config cfg) { + return new UserConfig(cfg); + } + }; + + private final String authorName; + + private final String authorEmail; + + private final String committerName; + + private final String committerEmail; + + private UserConfig(final Config rc) { + authorName = getNameInternal(rc, Constants.GIT_AUTHOR_NAME_KEY); + authorEmail = getEmailInternal(rc, Constants.GIT_AUTHOR_EMAIL_KEY); + + committerName = getNameInternal(rc, Constants.GIT_COMMITTER_NAME_KEY); + committerEmail = getEmailInternal(rc, Constants.GIT_COMMITTER_EMAIL_KEY); + } + + /** + * @return the author name as defined in the git variables and + * configurations. If no name could be found, try to use the system + * user name instead. + */ + public String getAuthorName() { + return authorName; + } + + /** + * @return the committer name as defined in the git variables and + * configurations. If no name could be found, try to use the system + * user name instead. + */ + public String getCommitterName() { + return committerName; + } + + /** + * @return the author email as defined in git variables and + * configurations. If no email could be found, try to + * propose one default with the user name and the + * host name. + */ + public String getAuthorEmail() { + return authorEmail; + } + + /** + * @return the committer email as defined in git variables and + * configurations. If no email could be found, try to + * propose one default with the user name and the + * host name. + */ + public String getCommitterEmail() { + return committerEmail; + } + + private static String getNameInternal(Config rc, String envKey) { + // try to get the user name from the local and global configurations. + String username = rc.getString("user", null, "name"); + + if (username == null) { + // try to get the user name for the system property GIT_XXX_NAME + username = system().getenv(envKey); + } + if (username == null) { + // get the system user name + username = system().getProperty(Constants.OS_USER_NAME_KEY); + } + if (username == null) { + username = Constants.UNKNOWN_USER_DEFAULT; + } + return username; + } + + private static String getEmailInternal(Config rc, String envKey) { + // try to get the email from the local and global configurations. + String email = rc.getString("user", null, "email"); + + if (email == null) { + // try to get the email for the system property GIT_XXX_EMAIL + email = system().getenv(envKey); + } + + if (email == null) { + // try to construct an email + String username = system().getProperty(Constants.OS_USER_NAME_KEY); + if (username == null){ + username = Constants.UNKNOWN_USER_DEFAULT; + } + email = username + "@" + system().getHostname(); + } + + return email; + } + + private static SystemReader system() { + return SystemReader.getInstance(); + } +} -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 17/19] Move repository config creation fully into Repository class 2009-07-25 18:52 ` [JGIT PATCH 16/19] Refactor author/committer lookup to use cached data Shawn O. Pearce @ 2009-07-25 18:53 ` Shawn O. Pearce 2009-07-25 18:53 ` [JGIT PATCH 18/19] Use Config SectionParser cache to store daemon enable states Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:53 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Rather than having the core properties initialized in two different locations its cleaner to do all of the setup work here in one place, in case we need to make any further changes to the repository create code path. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/Repository.java | 5 ++--- .../src/org/spearce/jgit/lib/RepositoryConfig.java | 9 --------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java index 7fb1ef7..468cf4c 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java @@ -144,8 +144,6 @@ public Repository(final File d) throws IOException { throw new IOException("Unknown repository format \"" + repositoryFormatVersion + "\"; expected \"0\"."); } - } else { - getConfig().create(); } } @@ -185,7 +183,8 @@ public void create(boolean bare) throws IOException { final String master = Constants.R_HEADS + Constants.MASTER; refs.link(Constants.HEAD, master); - cfg.create(); + cfg.setInt("core", null, "repositoryformatversion", 0); + cfg.setBoolean("core", null, "filemode", true); if (bare) cfg.setBoolean("core", null, "bare", true); cfg.save(); diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java index 15fe9de..ccfe184 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java @@ -123,13 +123,4 @@ public String getAuthorEmail() { public String getCommitterEmail() { return getUserConfig().getCommitterEmail(); } - - /** - * Create a new default config - */ - public void create() { - clear(); - setInt("core", null, "repositoryformatversion", 0); - setBoolean("core", null, "filemode", true); - } } -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 18/19] Use Config SectionParser cache to store daemon enable states 2009-07-25 18:53 ` [JGIT PATCH 17/19] Move repository config creation fully into Repository class Shawn O. Pearce @ 2009-07-25 18:53 ` Shawn O. Pearce 2009-07-25 18:53 ` [JGIT PATCH 19/19] Use Config cache for fetch and receive configuration parsing Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:53 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Rather than looking up the boolean each time we start a new connection in a repository we now cache it in the configuration cache under a key that is unique to the daemon service. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/transport/DaemonService.java | 30 ++++++++++++++++---- 1 files changed, 24 insertions(+), 6 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/DaemonService.java b/org.spearce.jgit/src/org/spearce/jgit/transport/DaemonService.java index 817aeee..b7198c7 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/DaemonService.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/DaemonService.java @@ -39,13 +39,15 @@ import java.io.IOException; +import org.spearce.jgit.lib.Config; import org.spearce.jgit.lib.Repository; +import org.spearce.jgit.lib.Config.SectionParser; /** A service exposed by {@link Daemon} over anonymous <code>git://</code>. */ public abstract class DaemonService { private final String command; - private final String config; + private final SectionParser<ServiceConfig> configKey; private boolean enabled; @@ -53,10 +55,23 @@ DaemonService(final String cmdName, final String cfgName) { command = cmdName.startsWith("git-") ? cmdName : "git-" + cmdName; - config = cfgName; + configKey = new SectionParser<ServiceConfig>() { + public ServiceConfig parse(final Config cfg) { + return new ServiceConfig(DaemonService.this, cfg, cfgName); + } + }; overridable = true; } + private static class ServiceConfig { + final boolean enabled; + + ServiceConfig(final DaemonService service, final Config cfg, + final String name) { + enabled = cfg.getBoolean("daemon", name, service.isEnabled()); + } + } + /** @return is this service enabled for invocation? */ public boolean isEnabled() { return enabled; @@ -109,16 +124,19 @@ void execute(final DaemonClient client, final String commandLine) if (db == null) return; try { - boolean on = isEnabled(); - if (isOverridable()) - on = db.getConfig().getBoolean("daemon", config, on); - if (on) + if (isEnabledFor(db)) execute(client, db); } finally { db.close(); } } + private boolean isEnabledFor(final Repository db) { + if (isOverridable()) + return db.getConfig().get(configKey).enabled; + return isEnabled(); + } + abstract void execute(DaemonClient client, Repository db) throws IOException; } -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [JGIT PATCH 19/19] Use Config cache for fetch and receive configuration parsing 2009-07-25 18:53 ` [JGIT PATCH 18/19] Use Config SectionParser cache to store daemon enable states Shawn O. Pearce @ 2009-07-25 18:53 ` Shawn O. Pearce 0 siblings, 0 replies; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 18:53 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Rather than parsing the fields on each transfer we now parse the fields once and cache them in the Config object, under a unique section key for the fetch or receive direction. This permits the keys to be scanned only once, rather than per-request. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../jgit/transport/BasePackFetchConnection.java | 21 ++++++++- .../org/spearce/jgit/transport/ReceivePack.java | 45 ++++++++++++++++---- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java index 7163e02..dea6181 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java @@ -46,12 +46,13 @@ import org.spearce.jgit.errors.TransportException; import org.spearce.jgit.lib.AnyObjectId; +import org.spearce.jgit.lib.Config; import org.spearce.jgit.lib.MutableObjectId; import org.spearce.jgit.lib.ObjectId; import org.spearce.jgit.lib.PackLock; import org.spearce.jgit.lib.ProgressMonitor; import org.spearce.jgit.lib.Ref; -import org.spearce.jgit.lib.RepositoryConfig; +import org.spearce.jgit.lib.Config.SectionParser; import org.spearce.jgit.revwalk.RevCommit; import org.spearce.jgit.revwalk.RevCommitList; import org.spearce.jgit.revwalk.RevFlag; @@ -146,10 +147,10 @@ BasePackFetchConnection(final PackTransport packTransport) { super(packTransport); - final RepositoryConfig cfg = local.getConfig(); + final FetchConfig cfg = local.getConfig().get(FetchConfig.KEY); includeTags = transport.getTagOpt() != TagOpt.NO_TAGS; thinPack = transport.isFetchThin(); - allowOfsDelta = cfg.getBoolean("repack", "usedeltabaseoffset", true); + allowOfsDelta = cfg.allowOfsDelta; walk = new RevWalk(local); reachableCommits = new RevCommitList<RevCommit>(); @@ -162,6 +163,20 @@ walk.carry(ADVERTISED); } + private static class FetchConfig { + static final SectionParser<FetchConfig> KEY = new SectionParser<FetchConfig>() { + public FetchConfig parse(final Config cfg) { + return new FetchConfig(cfg); + } + }; + + final boolean allowOfsDelta; + + FetchConfig(final Config c) { + allowOfsDelta = c.getBoolean("repack", "usedeltabaseoffset", true); + } + } + public final void fetch(final ProgressMonitor monitor, final Collection<Ref> want, final Set<ObjectId> have) throws TransportException { diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java index 38f0b5c..ca4a7ec 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java @@ -54,6 +54,7 @@ import org.spearce.jgit.errors.MissingObjectException; import org.spearce.jgit.errors.PackProtocolException; +import org.spearce.jgit.lib.Config; import org.spearce.jgit.lib.Constants; import org.spearce.jgit.lib.NullProgressMonitor; import org.spearce.jgit.lib.ObjectId; @@ -62,7 +63,7 @@ import org.spearce.jgit.lib.Ref; import org.spearce.jgit.lib.RefUpdate; import org.spearce.jgit.lib.Repository; -import org.spearce.jgit.lib.RepositoryConfig; +import org.spearce.jgit.lib.Config.SectionParser; import org.spearce.jgit.revwalk.ObjectWalk; import org.spearce.jgit.revwalk.RevCommit; import org.spearce.jgit.revwalk.RevFlag; @@ -158,17 +159,45 @@ public ReceivePack(final Repository into) { db = into; walk = new RevWalk(db); - final RepositoryConfig cfg = db.getConfig(); - checkReceivedObjects = cfg.getBoolean("receive", "fsckobjects", false); - allowCreates = true; - allowDeletes = !cfg.getBoolean("receive", "denydeletes", false); - allowNonFastForwards = !cfg.getBoolean("receive", - "denynonfastforwards", false); - allowOfsDelta = cfg.getBoolean("repack", "usedeltabaseoffset", true); + final ReceiveConfig cfg = db.getConfig().get(ReceiveConfig.KEY); + checkReceivedObjects = cfg.checkReceivedObjects; + allowCreates = cfg.allowCreates; + allowDeletes = cfg.allowDeletes; + allowNonFastForwards = cfg.allowNonFastForwards; + allowOfsDelta = cfg.allowOfsDelta; preReceive = PreReceiveHook.NULL; postReceive = PostReceiveHook.NULL; } + private static class ReceiveConfig { + static final SectionParser<ReceiveConfig> KEY = new SectionParser<ReceiveConfig>() { + public ReceiveConfig parse(final Config cfg) { + return new ReceiveConfig(cfg); + } + }; + + final boolean checkReceivedObjects; + + final boolean allowCreates; + + final boolean allowDeletes; + + final boolean allowNonFastForwards; + + final boolean allowOfsDelta; + + ReceiveConfig(final Config config) { + checkReceivedObjects = config.getBoolean("receive", "fsckobjects", + false); + allowCreates = true; + allowDeletes = !config.getBoolean("receive", "denydeletes", false); + allowNonFastForwards = !config.getBoolean("receive", + "denynonfastforwards", false); + allowOfsDelta = config.getBoolean("repack", "usedeltabaseoffset", + true); + } + } + /** @return the repository this receive completes into. */ public final Repository getRepository() { return db; -- 1.6.4.rc2.216.g769fa ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit 2009-07-25 18:52 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 10/19] Test for the config file when creating a new repository Shawn O. Pearce @ 2009-07-25 22:54 ` Robin Rosenberg 2009-07-25 22:55 ` Shawn O. Pearce 1 sibling, 1 reply; 26+ messages in thread From: Robin Rosenberg @ 2009-07-25 22:54 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: git, Constantine Plotnikov This one does not apply for the Config class here. -- robin ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit 2009-07-25 22:54 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Robin Rosenberg @ 2009-07-25 22:55 ` Shawn O. Pearce 2009-07-25 23:34 ` Robin Rosenberg 0 siblings, 1 reply; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 22:55 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git, Constantine Plotnikov Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote: > > This one does not apply for the Config class here. Did you apply with --whitespace=strip or something and fall afoul of a merge conflict due to changed context? -- Shawn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit 2009-07-25 22:55 ` Shawn O. Pearce @ 2009-07-25 23:34 ` Robin Rosenberg 2009-07-25 23:38 ` Shawn O. Pearce 0 siblings, 1 reply; 26+ messages in thread From: Robin Rosenberg @ 2009-07-25 23:34 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: git, Constantine Plotnikov söndag 26 juli 2009 00:55:04 skrev "Shawn O. Pearce" <spearce@spearce.org>: > Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote: > > > > This one does not apply for the Config class here. > > Did you apply with --whitespace=strip or something and fall afoul > of a merge conflict due to changed context? > I tried both ways. Config.java looks (partially) like this: -------------------------- import java.util.Map; import java.util.Set; /** ---------------------------- and the patch looks (partially again) like this: @@ -55,19 +49,18 @@ import java.util.Map; import java.util.Set; +import org.spearce.jgit.errors.ConfigInvalidException; import org.spearce.jgit.util.StringUtils; + /** ---------------- So the import among other things, does not match. -- robin ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit 2009-07-25 23:34 ` Robin Rosenberg @ 2009-07-25 23:38 ` Shawn O. Pearce 0 siblings, 0 replies; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 23:38 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git, Constantine Plotnikov Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote: > s?ndag 26 juli 2009 00:55:04 skrev "Shawn O. Pearce" <spearce@spearce.org>: > > Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote: > > > > > > This one does not apply for the Config class here. > > > > Did you apply with --whitespace=strip or something and fall afoul > > of a merge conflict due to changed context? > > Config.java looks (partially) like this: > > -------------------------- > import java.util.Map; > import java.util.Set; > > /** > > ---------------------------- > > and the patch looks (partially again) like this: > > @@ -55,19 +49,18 @@ > import java.util.Map; > import java.util.Set; > > +import org.spearce.jgit.errors.ConfigInvalidException; > import org.spearce.jgit.util.StringUtils; You missed the patch where I added StringUtils. See "[PATCH] Ensure Config readers handle case insensitive names correctly" from Friday Jul 24. -- Shawn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method 2009-07-25 18:52 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 03/19] Make Config.escapeValue a private method Shawn O. Pearce @ 2009-07-25 20:32 ` Robin Rosenberg 2009-07-25 20:33 ` Shawn O. Pearce 1 sibling, 1 reply; 26+ messages in thread From: Robin Rosenberg @ 2009-07-25 20:32 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: git lördag 25 juli 2009 20:52:45 skrev "Shawn O. Pearce" <spearce@spearce.org>: > I don't know how this Javadoc got here, but it predates the code > refactor done by Constantine Plotnikov in 2564768e63. Why do you think so? The getRawString before his patch did not have any javadoc. It was added there in that patch. -- robin ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method 2009-07-25 20:32 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Robin Rosenberg @ 2009-07-25 20:33 ` Shawn O. Pearce 0 siblings, 0 replies; 26+ messages in thread From: Shawn O. Pearce @ 2009-07-25 20:33 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote: > l?rdag 25 juli 2009 20:52:45 skrev "Shawn O. Pearce" <spearce@spearce.org>: > > I don't know how this Javadoc got here, but it predates the code > > refactor done by Constantine Plotnikov in 2564768e63. > > Why do you think so? The getRawString before his patch did not have any > javadoc. It was added there in that patch. Oh. I failed to look at the history, I just assumed he carried it over in the refactoring. Well, it still should die. -- Shawn. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2009-07-25 23:38 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-07-25 18:52 [JGIT PATCH 00/19] More Config class cleanup work Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 01/19] Cleanup nonstandard references to encoding strings to bytes Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 03/19] Make Config.escapeValue a private method Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 04/19] Allow a RemoteConfig to use the more generic Config class Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 05/19] Use type specific sets when creating a new RepositoryConfig Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 06/19] Move SystemReader out of RepositoryConfig Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 07/19] Correct user config to be of type FileBasedConfig Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 08/19] Extract the test specific SystemReader out of RepositoryTestCase Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 10/19] Test for the config file when creating a new repository Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 11/19] Remove the map lookup for values in Config Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 12/19] Return base values first from Config.getStringList() Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 13/19] Make Config thread safe by using copy-on-write semantics Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 14/19] Support cached application models in a Config Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 15/19] Cache Config subsection names when requested by application code Shawn O. Pearce 2009-07-25 18:52 ` [JGIT PATCH 16/19] Refactor author/committer lookup to use cached data Shawn O. Pearce 2009-07-25 18:53 ` [JGIT PATCH 17/19] Move repository config creation fully into Repository class Shawn O. Pearce 2009-07-25 18:53 ` [JGIT PATCH 18/19] Use Config SectionParser cache to store daemon enable states Shawn O. Pearce 2009-07-25 18:53 ` [JGIT PATCH 19/19] Use Config cache for fetch and receive configuration parsing Shawn O. Pearce 2009-07-25 22:54 ` [JGIT PATCH 09/19] Refactor Config hierarchy to make IO more explicit Robin Rosenberg 2009-07-25 22:55 ` Shawn O. Pearce 2009-07-25 23:34 ` Robin Rosenberg 2009-07-25 23:38 ` Shawn O. Pearce 2009-07-25 20:32 ` [JGIT PATCH 02/19] Delete incorrect Javadoc from Config's getRawString method Robin Rosenberg 2009-07-25 20:33 ` Shawn O. Pearce
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).