Add TypedConfigGetter.getPath()

This enables EGit to override with a lenient variant that logs the
problem and continues with the default value. EGit needs this because
otherwise a corrupt core.excludesFile entry can render the whole UI
unusable (until the git config is fixed) because any use of the
WorkingTreeIterator will throw an InvalidPathException.

This is not a problem on OS X, where all characters are allowed in
file names. But on Windows some characters are forbidden... see bug
567296. The message of the InvalidPathException is not helpful since
it doesn't point to the origin of the problem. EGit can log a much
better message indicating the offending config file and entry in the
Eclipse error log, where the user can see it.

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

diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
new file mode 100644 (file)
index 0000000..4c4e86e
--- /dev/null
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.jgit" version="2">
+    <resource path="src/org/eclipse/jgit/lib/TypedConfigGetter.java" type="org.eclipse.jgit.lib.TypedConfigGetter">
+        <filter id="404000815">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.lib.TypedConfigGetter"/>
+                <message_argument value="getPath(Config, String, String, String, FS, File, Path)"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>

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 a51593b5092ffed889c69291355a822021e1ef0c..a369026c97d7243997c5194b77cb1b8ac76d5132 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -20,6 +20,9 @@ package org.eclipse.jgit.lib;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import java.io.File;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -29,6 +32,7 @@ import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.events.ConfigChangedEvent;
 import org.eclipse.jgit.events.ConfigChangedListener;
@@ -36,6 +40,7 @@ import org.eclipse.jgit.events.ListenerHandle;
 import org.eclipse.jgit.events.ListenerList;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.RawParseUtils;
 
 /**
@@ -474,6 +479,37 @@ public class Config {
                                defaultValue, wantUnit);
        }
 
+       /**
+        * Parse a string value and treat it as a file path, replacing a ~/ prefix
+        * by the user's home directory.
+        * <p>
+        * <b>Note:</b> this may throw {@link InvalidPathException} if the string is
+        * not a valid path.
+        * </p>
+        *
+        * @param section
+        *            section the key is in.
+        * @param subsection
+        *            subsection the key is in, or null if not in a subsection.
+        * @param name
+        *            the key name.
+        * @param fs
+        *            to use to convert the string into a path.
+        * @param resolveAgainst
+        *            directory to resolve the path against if it is a relative
+        *            path; {@code null} to use the Java process's current
+        *            directory.
+        * @param defaultValue
+        *            to return if no value was present
+        * @return the {@link Path}, or {@code defaultValue} if not set
+        * @since 5.10
+        */
+       public Path getPath(String section, String subsection, String name,
+                       @NonNull FS fs, File resolveAgainst, Path defaultValue) {
+               return typedGetter.getPath(this, section, subsection, name, fs,
+                               resolveAgainst, defaultValue);
+       }
+
        /**
         * Parse a list of {@link org.eclipse.jgit.transport.RefSpec}s from the
         * configuration.

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/TypedConfigGetter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/TypedConfigGetter.java
index dd4be345e7e27d34a37ee6152b27cc50b2a2c472..0f2f6cff8a0677a50ece36edb02347864eca1e3f 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/TypedConfigGetter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/TypedConfigGetter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017, Thomas Wolf <thomas.wolf@paranor.ch> and others
+ * Copyright (C) 2017, 2020 Thomas Wolf <thomas.wolf@paranor.ch> and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -10,22 +10,26 @@
 
 package org.eclipse.jgit.lib;
 
+import java.io.File;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.util.FS;
 
 /**
- * Something that knows how to convert plain strings from a git
- * {@link org.eclipse.jgit.lib.Config} to typed values.
+ * Something that knows how to convert plain strings from a git {@link Config}
+ * to typed values.
  *
  * @since 4.9
  */
 public interface TypedConfigGetter {
 
        /**
-        * Get a boolean value from a git {@link org.eclipse.jgit.lib.Config}.
+        * Get a boolean value from a git {@link Config}.
         *
         * @param config
         *            to get the value from
@@ -44,7 +48,7 @@ public interface TypedConfigGetter {
                        String name, boolean defaultValue);
 
        /**
-        * Parse an enumeration from a git {@link org.eclipse.jgit.lib.Config}.
+        * Parse an enumeration from a git {@link Config}.
         *
         * @param config
         *            to get the value from
@@ -65,7 +69,7 @@ public interface TypedConfigGetter {
                        String subsection, String name, T defaultValue);
 
        /**
-        * Obtain an integer value from a git {@link org.eclipse.jgit.lib.Config}.
+        * Obtain an integer value from a git {@link Config}.
         *
         * @param config
         *            to get the value from
@@ -83,7 +87,7 @@ public interface TypedConfigGetter {
                        int defaultValue);
 
        /**
-        * Obtain a long value from a git {@link org.eclipse.jgit.lib.Config}.
+        * Obtain a long value from a git {@link Config}.
         *
         * @param config
         *            to get the value from
@@ -102,7 +106,7 @@ public interface TypedConfigGetter {
 
        /**
         * Parse a numerical time unit, such as "1 minute", from a git
-        * {@link org.eclipse.jgit.lib.Config}.
+        * {@link Config}.
         *
         * @param config
         *            to get the value from
@@ -124,10 +128,50 @@ public interface TypedConfigGetter {
        long getTimeUnit(Config config, String section, String subsection,
                        String name, long defaultValue, TimeUnit wantUnit);
 
+       /**
+        * Parse a string value from a git {@link Config} and treat it as a file
+        * path, replacing a ~/ prefix by the user's home directory.
+        * <p>
+        * <b>Note:</b> this may throw {@link InvalidPathException} if the string is
+        * not a valid path.
+        * </p>
+        *
+        * @param config
+        *            to get the path from.
+        * @param section
+        *            section the key is in.
+        * @param subsection
+        *            subsection the key is in, or null if not in a subsection.
+        * @param name
+        *            the key name.
+        * @param fs
+        *            to use to convert the string into a path.
+        * @param resolveAgainst
+        *            directory to resolve the path against if it is a relative
+        *            path.
+        * @param defaultValue
+        *            to return if no value was present
+        * @return the {@link Path}, or {@code defaultValue} if not set
+        * @since 5.10
+        */
+       default Path getPath(Config config, String section, String subsection,
+                       String name, @NonNull FS fs, File resolveAgainst,
+                       Path defaultValue) {
+               String value = config.getString(section, subsection, name);
+               if (value == null) {
+                       return defaultValue;
+               }
+               File file;
+               if (value.startsWith("~/")) { //$NON-NLS-1$
+                       file = fs.resolve(fs.userHome(), value.substring(2));
+               } else {
+                       file = fs.resolve(resolveAgainst, value);
+               }
+               return file.toPath();
+       }
 
        /**
-        * Parse a list of {@link org.eclipse.jgit.transport.RefSpec}s from a git
-        * {@link org.eclipse.jgit.lib.Config}.
+        * Parse a list of {@link RefSpec}s from a git {@link Config}.
         *
         * @param config
         *            to get the list from

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
index 4c26dd0c4038f96407c4d039697337111d82458a..72278dc9c351fe044b10a11612bc39e88af649af 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
@@ -25,6 +25,7 @@ import java.nio.ByteBuffer;
 import java.nio.CharBuffer;
 import java.nio.charset.CharacterCodingException;
 import java.nio.charset.CharsetEncoder;
+import java.nio.file.Path;
 import java.text.MessageFormat;
 import java.time.Instant;
 import java.util.Arrays;
@@ -48,8 +49,8 @@ import org.eclipse.jgit.errors.NoWorkTreeException;
 import org.eclipse.jgit.ignore.FastIgnoreRule;
 import org.eclipse.jgit.ignore.IgnoreNode;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.CoreConfig;
 import org.eclipse.jgit.lib.CoreConfig.CheckStat;
 import org.eclipse.jgit.lib.CoreConfig.EolStreamType;
 import org.eclipse.jgit.lib.CoreConfig.SymLinks;
@@ -1308,15 +1309,11 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                        }
 
                        FS fs = repository.getFS();
-                       String path = repository.getConfig().get(CoreConfig.KEY)
-                                       .getExcludesFile();
+                       Path path = repository.getConfig().getPath(
+                                       ConfigConstants.CONFIG_CORE_SECTION, null,
+                                       ConfigConstants.CONFIG_KEY_EXCLUDESFILE, fs, null, null);
                        if (path != null) {
-                               File excludesfile;
-                               if (path.startsWith("~/")) //$NON-NLS-1$
-                                       excludesfile = fs.resolve(fs.userHome(), path.substring(2));
-                               else
-                                       excludesfile = fs.resolve(null, path);
-                               loadRulesFromFile(r, excludesfile);
+                               loadRulesFromFile(r, path.toFile());
                        }
 
                        File exclude = fs.resolve(repository.getDirectory(),