* [JGIT PATCH 0/5] Some minor bug fixes @ 2009-07-29 15:50 Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 1/5] Don't dispose of RevFlag used to advertise objects in ReceivePack Shawn O. Pearce 0 siblings, 1 reply; 6+ messages in thread From: Shawn O. Pearce @ 2009-07-29 15:50 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git I'm just throwing this series into the repository as-is. The first in the series is needed to fix Gerrit Code Review (busted due to a recent change to ReceivePack). The rest of the series is really minor public API corrections. Shawn O. Pearce (5): Don't dispose of RevFlag used to advertise objects in ReceivePack Allow RemoteConfig.getAllRemoteConfigs on any Config object Add ConfigInvalidException constructor to take Throwable Add no fallback constructor to FileBasedConfig Include the file path when FileBasedConfig.load throws an exception .../tst/org/spearce/jgit/lib/MockSystemReader.java | 2 +- .../org/spearce/jgit/lib/RepositoryTestCase.java | 2 +- .../jgit/errors/ConfigInvalidException.java | 12 ++++++++++++ .../src/org/spearce/jgit/lib/FileBasedConfig.java | 16 ++++++++++++++++ .../org/spearce/jgit/transport/ReceivePack.java | 1 - .../org/spearce/jgit/transport/RemoteConfig.java | 5 ++--- .../src/org/spearce/jgit/util/SystemReader.java | 3 +-- 7 files changed, 33 insertions(+), 8 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [JGIT PATCH 1/5] Don't dispose of RevFlag used to advertise objects in ReceivePack 2009-07-29 15:50 [JGIT PATCH 0/5] Some minor bug fixes Shawn O. Pearce @ 2009-07-29 15:50 ` Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 2/5] Allow RemoteConfig.getAllRemoteConfigs on any Config object Shawn O. Pearce 0 siblings, 1 reply; 6+ messages in thread From: Shawn O. Pearce @ 2009-07-29 15:50 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Back in 3ae30ae64d56113a71a07a3ff12d0af71cf538e2 I refactored how the ReceivePack and UploadPack programs advertise the set of refs that the server-side currently has. As part of this change ReceivePack allocated a new RevFlag and marked it on all objects it advertised to the client, because the common code with UploadPack requires the RevFlag to mark objects. It also uses this RevFlag to avoid sending ".have" lines for objects we have already described. Disposing of the RevFlag however is problematic for any application that uses the ReceivePack's RevWalk instance within a PreReceiveHook or a PostReceiveHook implementation. By disposing of the RevFlag, the flag bit was not actually unset from the advertised objects, it was only marked as being eligible for reallocation in another RevFlag. If the flag bit was reallocated, any advertised objects would automatically have the bit set, which can confuse the new user of the bit. RevWalk's isMergedInto() really got confused, as it allocates a temporary RevFlag for each input RevCommit in order to compute the merge base. The simplest fix is to just never dispose of this RevFlag once it has been allocated. We could dispose of it and also unset it from all objects we advertised, but that would require us to keep track of the full advertised set, a potentially expensive thing to do given that most callers will never need to allocate a RevFlag, let alone care about the 1 bit leaked by the protocol. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/transport/ReceivePack.java | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) 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 ca4a7ec..eb21254 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java @@ -547,7 +547,6 @@ private void sendAdvertisedRefs() throws IOException { if (adv.isEmpty()) adv.advertiseId(ObjectId.zeroId(), "capabilities^{}"); pckOut.end(); - walk.disposeFlag(advertised); } private void recvCommands() throws IOException { -- 1.6.4.rc3.201.gd9d59 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 2/5] Allow RemoteConfig.getAllRemoteConfigs on any Config object 2009-07-29 15:50 ` [JGIT PATCH 1/5] Don't dispose of RevFlag used to advertise objects in ReceivePack Shawn O. Pearce @ 2009-07-29 15:50 ` Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 3/5] Add ConfigInvalidException constructor to take Throwable Shawn O. Pearce 0 siblings, 1 reply; 6+ messages in thread From: Shawn O. Pearce @ 2009-07-29 15:50 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Back in 07c8dc6e94729ff4104f0a1c815dd81f3e71c562 I changed the RemoteConfig class to accept any Config object, but its static getAllRemoteConfigs() utility method was missed and still used a RepositoryConfig. It should take any Config. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../org/spearce/jgit/transport/RemoteConfig.java | 5 ++--- 1 files changed, 2 insertions(+), 3 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 ca84acf..dd7a50c 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java @@ -44,7 +44,6 @@ import java.util.List; import org.spearce.jgit.lib.Config; -import org.spearce.jgit.lib.RepositoryConfig; /** * A remembered remote repository, including URLs and RefSpecs. @@ -96,8 +95,8 @@ * @throws URISyntaxException * one of the URIs within the remote's configuration is invalid. */ - public static List<RemoteConfig> getAllRemoteConfigs( - final RepositoryConfig rc) throws URISyntaxException { + public static List<RemoteConfig> getAllRemoteConfigs(final Config rc) + throws URISyntaxException { final List<String> names = new ArrayList<String>(rc .getSubsections(SECTION)); Collections.sort(names); -- 1.6.4.rc3.201.gd9d59 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 3/5] Add ConfigInvalidException constructor to take Throwable 2009-07-29 15:50 ` [JGIT PATCH 2/5] Allow RemoteConfig.getAllRemoteConfigs on any Config object Shawn O. Pearce @ 2009-07-29 15:50 ` Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 4/5] Add no fallback constructor to FileBasedConfig Shawn O. Pearce 0 siblings, 1 reply; 6+ messages in thread From: Shawn O. Pearce @ 2009-07-29 15:50 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git This permits applications to wrap a ConfigInvalidException with more detail before throwing it to the caller. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../jgit/errors/ConfigInvalidException.java | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java index 6e861fd..28c87a5 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java +++ b/org.spearce.jgit/src/org/spearce/jgit/errors/ConfigInvalidException.java @@ -50,4 +50,16 @@ public ConfigInvalidException(final String message) { super(message); } + + /** + * Construct an invalid configuration error. + * + * @param message + * why the configuration is invalid. + * @param cause + * root cause of the error. + */ + public ConfigInvalidException(final String message, final Throwable cause) { + super(message, cause); + } } -- 1.6.4.rc3.201.gd9d59 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 4/5] Add no fallback constructor to FileBasedConfig 2009-07-29 15:50 ` [JGIT PATCH 3/5] Add ConfigInvalidException constructor to take Throwable Shawn O. Pearce @ 2009-07-29 15:50 ` Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 5/5] Include the file path when FileBasedConfig.load throws an exception Shawn O. Pearce 0 siblings, 1 reply; 6+ messages in thread From: Shawn O. Pearce @ 2009-07-29 15:50 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Sometimes when reading a FileBasedConfig the caller does not want any fallback lookup. Instead always passing a null first parameter to the constructor, we can use a 1-arg constructor that takes only the File path for this file. This simplifies a number of cases within the JGit library and test cases, as well as some application code. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../tst/org/spearce/jgit/lib/MockSystemReader.java | 2 +- .../org/spearce/jgit/lib/RepositoryTestCase.java | 2 +- .../src/org/spearce/jgit/lib/FileBasedConfig.java | 10 ++++++++++ .../src/org/spearce/jgit/util/SystemReader.java | 3 +-- 4 files changed, 13 insertions(+), 4 deletions(-) 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 index 62862d1..7a65f99 100644 --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/MockSystemReader.java @@ -53,7 +53,7 @@ init(Constants.GIT_AUTHOR_EMAIL_KEY); init(Constants.GIT_COMMITTER_NAME_KEY); init(Constants.GIT_COMMITTER_EMAIL_KEY); - userGitConfig = new FileBasedConfig(null, null); + userGitConfig = new FileBasedConfig(null); } private void init(final String n) { 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 24a99ca..6de9afe 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 @@ -237,7 +237,7 @@ public void run() { } final MockSystemReader mockSystemReader = new MockSystemReader(); - mockSystemReader.userGitConfig = new FileBasedConfig(null, new File( + mockSystemReader.userGitConfig = new FileBasedConfig(new File( trash_git, "usergitconfig")); SystemReader.setInstance(mockSystemReader); 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 a419e7f..adf85c6 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java @@ -55,6 +55,16 @@ private final File configFile; /** + * Create a configuration with no default fallback. + * + * @param cfgLocation + * the location of the configuration file on the file system + */ + public FileBasedConfig(File cfgLocation) { + this(null, cfgLocation); + } + + /** * The constructor * * @param base 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 a30dfef..51a0d29 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java +++ b/org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java @@ -42,7 +42,6 @@ import java.net.UnknownHostException; import org.spearce.jgit.lib.FileBasedConfig; -import org.spearce.jgit.lib.RepositoryConfig; /** * Interface to read values from the system. @@ -66,7 +65,7 @@ public String getProperty(String key) { public FileBasedConfig openUserConfig() { final File home = FS.userHome(); - return new RepositoryConfig(null, new File(home, ".gitconfig")); + return new FileBasedConfig(new File(home, ".gitconfig")); } public String getHostname() { -- 1.6.4.rc3.201.gd9d59 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [JGIT PATCH 5/5] Include the file path when FileBasedConfig.load throws an exception 2009-07-29 15:50 ` [JGIT PATCH 4/5] Add no fallback constructor to FileBasedConfig Shawn O. Pearce @ 2009-07-29 15:50 ` Shawn O. Pearce 0 siblings, 0 replies; 6+ messages in thread From: Shawn O. Pearce @ 2009-07-29 15:50 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git This may help an end-user debug the problem, if they know what file the error occurred in. We have to wrap the exception here because the methods throwing the original error do not have access to the file path, and cannot include it in the message. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> --- .../src/org/spearce/jgit/lib/FileBasedConfig.java | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) 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 adf85c6..518b31d 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java @@ -98,6 +98,12 @@ public void load() throws IOException, ConfigInvalidException { fromText(RawParseUtils.decode(NB.readFully(getFile()))); } catch (FileNotFoundException noFile) { clear(); + } catch (IOException e) { + final IOException e2 = new IOException("Cannot read " + getFile()); + e2.initCause(e); + throw e2; + } catch (ConfigInvalidException e) { + throw new ConfigInvalidException("Cannot read " + getFile(), e); } } -- 1.6.4.rc3.201.gd9d59 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-07-29 15:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-07-29 15:50 [JGIT PATCH 0/5] Some minor bug fixes Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 1/5] Don't dispose of RevFlag used to advertise objects in ReceivePack Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 2/5] Allow RemoteConfig.getAllRemoteConfigs on any Config object Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 3/5] Add ConfigInvalidException constructor to take Throwable Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 4/5] Add no fallback constructor to FileBasedConfig Shawn O. Pearce 2009-07-29 15:50 ` [JGIT PATCH 5/5] Include the file path when FileBasedConfig.load throws an exception 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).