From 6f41458d8a3fc436569520974b1072dfbcf7347b Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 24 Sep 2020 21:34:29 +0200 Subject: [PATCH] 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 --- org.eclipse.jgit/.settings/.api_filters | 11 ++++ .../src/org/eclipse/jgit/lib/Config.java | 36 +++++++++++ .../eclipse/jgit/lib/TypedConfigGetter.java | 64 ++++++++++++++++--- .../jgit/treewalk/WorkingTreeIterator.java | 15 ++--- 4 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 org.eclipse.jgit/.settings/.api_filters diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters new file mode 100644 index 0000000000..4c4e86ee39 --- /dev/null +++ b/org.eclipse.jgit/.settings/.api_filters @@ -0,0 +1,11 @@ + + + + + + + + + + + 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 a51593b509..a369026c97 100644 --- 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. + *

+ * Note: this may throw {@link InvalidPathException} if the string is + * not a valid path. + *

+ * + * @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 dd4be345e7..0f2f6cff8a 100644 --- 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 and others + * Copyright (C) 2017, 2020 Thomas Wolf 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. + *

+ * Note: this may throw {@link InvalidPathException} if the string is + * not a valid path. + *

+ * + * @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 4c26dd0c40..72278dc9c3 100644 --- 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(), -- 2.39.5