Don't log error if system git config does not exist
- enhance FS.readPipe to throw an exception if the external command
fails to enable the caller to handle the command failure
- reduce log level to warning if system git config does not exist
- improve log message
Bug: 476639
Change-Id: I94ae3caec22150dde81f1ea8e1e665df55290d42
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java
index 3d09b92..4061b56 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/FSTest.java
@@ -50,11 +50,13 @@
import java.io.File;
import java.io.IOException;
+import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Set;
+import org.eclipse.jgit.errors.CommandFailedException;
import org.eclipse.jgit.junit.RepositoryTestCase;
import org.junit.After;
import org.junit.Assume;
@@ -164,4 +166,15 @@ private Set<PosixFilePermission> readPermissions(File f) throws IOException {
.readAttributes().permissions();
}
+ @Test(expected = CommandFailedException.class)
+ public void testReadPipePosixCommandFailure()
+ throws CommandFailedException {
+ FS fs = FS.DETECTED.newInstance();
+ assumeTrue(fs instanceof FS_POSIX);
+
+ String r = FS.readPipe(fs.userHome(),
+ new String[] { "bash", "--login", "-c", "foobar" },
+ Charset.defaultCharset().name());
+ System.out.println(r);
+ }
}
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 57b5b8d..c1954ff 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -264,7 +264,7 @@
exceptionCaughtDuringExecutionOfRevertCommand=Exception caught during execution of revert command. {0}
exceptionCaughtDuringExecutionOfRmCommand=Exception caught during execution of rm command
exceptionCaughtDuringExecutionOfTagCommand=Exception caught during execution of tag command
-exceptionCaughtDuringExcecutionOfCommand=Exception caught during execution of command {0} in {1}
+exceptionCaughtDuringExcecutionOfCommand=Exception caught during execution of command ''{0}'' in ''{1}'', return code ''{2}'', error message ''{3}''
exceptionHookExecutionInterrupted=Execution of "{0}" hook interrupted.
exceptionOccurredDuringAddingOfOptionToALogCommand=Exception occurred during adding of {0} as option to a Log command
exceptionOccurredDuringReadingOfGIT_DIR=Exception occurred during reading of $GIT_DIR/{0}. {1}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/errors/CommandFailedException.java b/org.eclipse.jgit/src/org/eclipse/jgit/errors/CommandFailedException.java
new file mode 100644
index 0000000..93749f5
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/errors/CommandFailedException.java
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 2016, Matthias Sohn <matthias.sohn@sap.com>
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.errors;
+
+/**
+ * Thrown when an external command failed
+ *
+ * @since 4.5
+ */
+public class CommandFailedException extends Exception {
+
+ private static final long serialVersionUID = 1L;
+
+ private int returnCode;
+
+ /**
+ * @param returnCode
+ * return code returned by the command
+ * @param message
+ * error message
+ */
+ public CommandFailedException(int returnCode, String message) {
+ super(message);
+ this.returnCode = returnCode;
+ }
+
+ /**
+ * @param returnCode
+ * return code returned by the command
+ * @param message
+ * error message
+ * @param cause
+ * exception causing this exception
+ */
+ public CommandFailedException(int returnCode, String message,
+ Throwable cause) {
+ super(message, cause);
+ this.returnCode = returnCode;
+ }
+
+ /**
+ * @return return code returned by the command
+ */
+ public int getReturnCode() {
+ return returnCode;
+ }
+}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index ec587d5..a55b5c0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -64,9 +64,11 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.errors.CommandFailedException;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
@@ -453,9 +455,12 @@ protected static File searchPath(final String path, final String... lookFor) {
* to be used to parse the command's output
* @return the one-line output of the command or {@code null} if there is
* none
+ * @throws CommandFailedException
+ * thrown when the command failed (return code was non-zero)
*/
@Nullable
- protected static String readPipe(File dir, String[] command, String encoding) {
+ protected static String readPipe(File dir, String[] command,
+ String encoding) throws CommandFailedException {
return readPipe(dir, command, encoding, null);
}
@@ -473,10 +478,14 @@ protected static String readPipe(File dir, String[] command, String encoding) {
* current process
* @return the one-line output of the command or {@code null} if there is
* none
+ * @throws CommandFailedException
+ * thrown when the command failed (return code was non-zero)
* @since 4.0
*/
@Nullable
- protected static String readPipe(File dir, String[] command, String encoding, Map<String, String> env) {
+ protected static String readPipe(File dir, String[] command,
+ String encoding, Map<String, String> env)
+ throws CommandFailedException {
final boolean debug = LOG.isDebugEnabled();
try {
if (debug) {
@@ -512,11 +521,14 @@ protected static String readPipe(File dir, String[] command, String encoding, Ma
gobbler.join();
if (rc == 0 && !gobbler.fail.get()) {
return r;
+ } else {
+ if (debug) {
+ LOG.debug("readpipe rc=" + rc); //$NON-NLS-1$
+ }
+ throw new CommandFailedException(rc,
+ gobbler.errorMessage.get(),
+ gobbler.exception.get());
}
- if (debug) {
- LOG.debug("readpipe rc=" + rc); //$NON-NLS-1$
- }
- break;
} catch (InterruptedException ie) {
// Stop bothering me, I have a zombie to reap.
}
@@ -535,6 +547,8 @@ private static class GobblerThread extends Thread {
private final String desc;
private final String dir;
final AtomicBoolean fail = new AtomicBoolean();
+ final AtomicReference<String> errorMessage = new AtomicReference<>();
+ final AtomicReference<Throwable> exception = new AtomicReference<>();
GobblerThread(Process p, String[] command, File dir) {
this.p = p;
@@ -542,6 +556,7 @@ private static class GobblerThread extends Thread {
this.dir = Objects.toString(dir);
}
+ @Override
public void run() {
StringBuilder err = new StringBuilder();
try (InputStream is = p.getErrorStream()) {
@@ -551,22 +566,26 @@ public void run() {
}
} catch (IOException e) {
if (p.exitValue() != 0) {
- logError(e);
+ setError(e, e.getMessage());
fail.set(true);
} else {
- // ignore. git terminated faster and stream was just closed
+ // ignore. command terminated faster and stream was just closed
}
} finally {
if (err.length() > 0) {
- LOG.error(err.toString());
+ setError(null, err.toString());
+ if (p.exitValue() != 0) {
+ fail.set(true);
+ }
}
}
}
- private void logError(Throwable t) {
- String msg = MessageFormat.format(
- JGitText.get().exceptionCaughtDuringExcecutionOfCommand, desc, dir);
- LOG.error(msg, t);
+ private void setError(IOException e, String message) {
+ exception.set(e);
+ errorMessage.set(MessageFormat.format(
+ JGitText.get().exceptionCaughtDuringExcecutionOfCommand,
+ desc, dir, Integer.valueOf(p.exitValue()), message));
}
}
@@ -589,10 +608,17 @@ protected File discoverGitSystemConfig() {
}
// Bug 480782: Check if the discovered git executable is JGit CLI
- String v = readPipe(gitExe.getParentFile(),
+ String v;
+ try {
+ v = readPipe(gitExe.getParentFile(),
new String[] { "git", "--version" }, //$NON-NLS-1$ //$NON-NLS-2$
Charset.defaultCharset().name());
- if (v != null && v.startsWith("jgit")) { //$NON-NLS-1$
+ } catch (CommandFailedException e) {
+ LOG.warn(e.getMessage());
+ return null;
+ }
+ if (StringUtils.isEmptyOrNull(v)
+ || (v != null && v.startsWith("jgit"))) { //$NON-NLS-1$
return null;
}
@@ -601,9 +627,15 @@ protected File discoverGitSystemConfig() {
Map<String, String> env = new HashMap<>();
env.put("GIT_EDITOR", "echo"); //$NON-NLS-1$ //$NON-NLS-2$
- String w = readPipe(gitExe.getParentFile(),
+ String w;
+ try {
+ w = readPipe(gitExe.getParentFile(),
new String[] { "git", "config", "--system", "--edit" }, //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
Charset.defaultCharset().name(), env);
+ } catch (CommandFailedException e) {
+ LOG.warn(e.getMessage());
+ return null;
+ }
if (StringUtils.isEmptyOrNull(w)) {
return null;
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
index b4f12f4..cb4868c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
@@ -57,8 +57,11 @@
import java.util.Set;
import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.errors.CommandFailedException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Base FS for POSIX based systems
@@ -66,6 +69,8 @@
* @since 3.0
*/
public class FS_POSIX extends FS {
+ private final static Logger LOG = LoggerFactory.getLogger(FS_POSIX.class);
+
private static final int DEFAULT_UMASK = 0022;
private volatile int umask = -1;
@@ -144,11 +149,18 @@ protected File discoverGitExe() {
// On MacOSX, PATH is shorter when Eclipse is launched from the
// Finder than from a terminal. Therefore try to launch bash as a
// login shell and search using that.
- String w = readPipe(userHome(),
+ String w;
+ try {
+ w = readPipe(userHome(),
new String[]{"bash", "--login", "-c", "which git"}, // //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
Charset.defaultCharset().name());
- if (!StringUtils.isEmptyOrNull(w))
+ } catch (CommandFailedException e) {
+ LOG.warn(e.getMessage());
+ return null;
+ }
+ if (!StringUtils.isEmptyOrNull(w)) {
gitExe = new File(w);
+ }
}
}
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
index defe14f..0e9172e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
@@ -51,6 +51,10 @@
import java.util.Arrays;
import java.util.List;
+import org.eclipse.jgit.errors.CommandFailedException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
/**
* FS implementation for Windows
@@ -58,6 +62,7 @@
* @since 3.0
*/
public class FS_Win32 extends FS {
+ private final static Logger LOG = LoggerFactory.getLogger(FS_Win32.class);
private volatile Boolean supportSymlinks;
@@ -113,12 +118,19 @@ protected File discoverGitExe() {
if (searchPath(path, "bash.exe") != null) { //$NON-NLS-1$
// This isn't likely to work, but its worth trying:
// If bash is in $PATH, git should also be in $PATH.
- String w = readPipe(userHome(),
+ String w;
+ try {
+ w = readPipe(userHome(),
new String[]{"bash", "--login", "-c", "which git"}, // //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
Charset.defaultCharset().name());
- if (!StringUtils.isEmptyOrNull(w))
+ } catch (CommandFailedException e) {
+ LOG.warn(e.getMessage());
+ return null;
+ }
+ if (!StringUtils.isEmptyOrNull(w)) {
// The path may be in cygwin/msys notation so resolve it right away
gitExe = resolve(null, w);
+ }
}
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
index ec581b3..f8ea5d0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
@@ -54,8 +54,11 @@
import java.util.List;
import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.errors.CommandFailedException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* FS implementation for Cygwin on Windows
@@ -63,6 +66,9 @@
* @since 3.0
*/
public class FS_Win32_Cygwin extends FS_Win32 {
+ private final static Logger LOG = LoggerFactory
+ .getLogger(FS_Win32_Cygwin.class);
+
private static String cygpath;
/**
@@ -107,11 +113,18 @@ public FS newInstance() {
public File resolve(final File dir, final String pn) {
String useCygPath = System.getProperty("jgit.usecygpath"); //$NON-NLS-1$
if (useCygPath != null && useCygPath.equals("true")) { //$NON-NLS-1$
- String w = readPipe(dir, //
+ String w;
+ try {
+ w = readPipe(dir, //
new String[] { cygpath, "--windows", "--absolute", pn }, // //$NON-NLS-1$ //$NON-NLS-2$
"UTF-8"); //$NON-NLS-1$
- if (w != null)
+ } catch (CommandFailedException e) {
+ LOG.warn(e.getMessage());
+ return null;
+ }
+ if (!StringUtils.isEmptyOrNull(w)) {
return new File(w);
+ }
}
return super.resolve(dir, pn);
}
