[JGIT PATCH 0/5] Some minor bug fixes

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(-)

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 <redacted>
---
 .../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

This permits applications to wrap a ConfigInvalidException with
more detail before throwing it to the caller.

Signed-off-by: Shawn O. Pearce <redacted>
---
 .../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

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 <redacted>
---
 .../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

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 <redacted>
---
 .../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

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 <redacted>
---
 .../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