Minor improvements in git config file inclusions

* Section and key names in git config files are case-insensitive.
* If an include directive is invalid, include the line in the
  exception message.
* If inclusion of the included file fails, put the file name into
  the exception message so that the user knows in which file the
  problem is.

Change-Id: If920943af7ff93f5321b3d315dfec5222091256c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
index fb1ee8cadb13ae6139ba10cf581b196fc3523d44..7862005ebc27eb2e48e392add0cd81c3cbc7e13c 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
@@ -808,8 +808,14 @@ public class ConfigTest {
                        fbConfig.load();
                        fail();
                } catch (ConfigInvalidException cie) {
-                       assertEquals(JGitText.get().tooManyIncludeRecursions,
-                                       cie.getCause().getMessage());
+                       for (Throwable t = cie; t != null; t = t.getCause()) {
+                               if (t.getMessage()
+                                               .equals(JGitText.get().tooManyIncludeRecursions)) {
+                                       return;
+                               }
+                       }
+                       fail("Expected to find expected exception message: "
+                                       + JGitText.get().tooManyIncludeRecursions);
                }
        }
 
@@ -824,6 +830,80 @@ public class ConfigTest {
                assertFalse(parsed.getBoolean("foo", "bar", false));
        }
 
+       @Test
+       public void testIncludeCaseInsensitiveSection()
+                       throws IOException, ConfigInvalidException {
+               File included = tmp.newFile("included");
+               String content = "[foo]\nbar=true\n";
+               Files.write(included.toPath(), content.getBytes());
+
+               File config = tmp.newFile("config");
+               content = "[Include]\npath=" + pathToString(included) + "\n";
+               Files.write(config.toPath(), content.getBytes());
+
+               FileBasedConfig fbConfig = new FileBasedConfig(null, config,
+                               FS.DETECTED);
+               fbConfig.load();
+               assertTrue(fbConfig.getBoolean("foo", "bar", false));
+       }
+
+       @Test
+       public void testIncludeCaseInsensitiveKey()
+                       throws IOException, ConfigInvalidException {
+               File included = tmp.newFile("included");
+               String content = "[foo]\nbar=true\n";
+               Files.write(included.toPath(), content.getBytes());
+
+               File config = tmp.newFile("config");
+               content = "[include]\nPath=" + pathToString(included) + "\n";
+               Files.write(config.toPath(), content.getBytes());
+
+               FileBasedConfig fbConfig = new FileBasedConfig(null, config,
+                               FS.DETECTED);
+               fbConfig.load();
+               assertTrue(fbConfig.getBoolean("foo", "bar", false));
+       }
+
+       @Test
+       public void testIncludeExceptionContainsLine() {
+               try {
+                       parse("[include]\npath=\n");
+                       fail("Expected ConfigInvalidException");
+               } catch (ConfigInvalidException e) {
+                       assertTrue(
+                                       "Expected to find the problem line in the exception message",
+                                       e.getMessage().contains("include.path"));
+               }
+       }
+
+       @Test
+       public void testIncludeExceptionContainsFile() throws IOException {
+               File included = tmp.newFile("included");
+               String includedPath = pathToString(included);
+               String content = "[include]\npath=\n";
+               Files.write(included.toPath(), content.getBytes());
+
+               File config = tmp.newFile("config");
+               String include = "[include]\npath=" + includedPath + "\n";
+               Files.write(config.toPath(), include.getBytes());
+               FileBasedConfig fbConfig = new FileBasedConfig(null, config,
+                               FS.DETECTED);
+               try {
+                       fbConfig.load();
+                       fail("Expected ConfigInvalidException");
+               } catch (ConfigInvalidException e) {
+                       // Check that there is some exception in the chain that contains
+                       // includedPath
+                       for (Throwable t = e; t != null; t = t.getCause()) {
+                               if (t.getMessage().contains(includedPath)) {
+                                       return;
+                               }
+                       }
+                       fail("Expected to find the path in the exception message: "
+                                       + includedPath);
+               }
+       }
+
        private static void assertReadLong(long exp) throws ConfigInvalidException {
                assertReadLong(exp, String.valueOf(exp));
        }

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 74be2466598e35af55a9691fcb1d81438c0503d4..c43411779d71ab8327e0af5914073810fd3b35be 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -368,6 +368,7 @@ invalidIgnoreRule=Exception caught while parsing ignore rule ''{0}''.
 invalidIntegerValue=Invalid integer value: {0}.{1}={2}
 invalidKey=Invalid key: {0}
 invalidLineInConfigFile=Invalid line in config file
+invalidLineInConfigFileWithParam=Invalid line in config file: {0}
 invalidModeFor=Invalid mode {0} for {1} {2} in {3}.
 invalidModeForPath=Invalid mode {0} for path {1}
 invalidObject=Invalid {0} {1}: {2}

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 7f91b30a91d4e594997512d7471b6a1952902e91..9739672f847671f965586186e125ccddc92961e3 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -429,6 +429,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String invalidIntegerValue;
        /***/ public String invalidKey;
        /***/ public String invalidLineInConfigFile;
+       /***/ public String invalidLineInConfigFileWithParam;
        /***/ public String invalidModeFor;
        /***/ public String invalidModeForPath;
        /***/ public String invalidObject;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
index a6313f0cc5103b702ac62f7c1b5a587aeeafa1c1..4d558c9fc2f5314efee53e49e72cb0c37d395fc1 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -1125,7 +1125,7 @@ public class Config {
                                } else
                                        e.value = readValue(in);
 
-                               if (e.section.equals("include")) { //$NON-NLS-1$
+                               if (e.section.equalsIgnoreCase("include")) { //$NON-NLS-1$
                                        addIncludedConfig(newEntries, e, depth);
                                }
                        } else
@@ -1154,10 +1154,10 @@ public class Config {
 
        private void addIncludedConfig(final List<ConfigLine> newEntries,
                        ConfigLine line, int depth) throws ConfigInvalidException {
-               if (!line.name.equals("path") || //$NON-NLS-1$
+               if (!line.name.equalsIgnoreCase("path") || //$NON-NLS-1$
                                line.value == null || line.value.equals(MAGIC_EMPTY_VALUE)) {
-                       throw new ConfigInvalidException(
-                                       JGitText.get().invalidLineInConfigFile);
+                       throw new ConfigInvalidException(MessageFormat.format(
+                                       JGitText.get().invalidLineInConfigFileWithParam, line));
                }
                byte[] bytes = readIncludedConfig(line.value);
                if (bytes == null) {
@@ -1171,7 +1171,12 @@ public class Config {
                } else {
                        decoded = RawParseUtils.decode(bytes);
                }
-               newEntries.addAll(fromTextRecurse(decoded, depth + 1));
+               try {
+                       newEntries.addAll(fromTextRecurse(decoded, depth + 1));
+               } catch (ConfigInvalidException e) {
+                       throw new ConfigInvalidException(MessageFormat
+                                       .format(JGitText.get().cannotReadFile, line.value), e);
+               }
        }
 
        private ConfigSnapshot newState() {