Add ability to redirect stderr from git hooks

This will change the behavior in the CLI to resemble that of C-Git more
closely by printing the stderr of the hooks to the CLI stderr
independently of the exit code of the hook.

This is also useful for the corresponding EGIT-Change, which will add
the ability to show the hook output in eclipse.
With this also the stderr can be shown even if the exit code is 0.

Bug: 553471
Change-Id: Ie7bc503fe39e270e9b93dd1108b5879f02a12b4c
Signed-off-by: Tim Neumann <Tim.Neumann@advantest.com>

9 files changed, 283 insertions, 10 deletions

diff --git a/org.eclipse.jgit/META-INF/MANIFEST.MF b/org.eclipse.jgit/META-INF/MANIFEST.MF
index ab0f1a2c09..899d2b6e61 100644
--- a/org.eclipse.jgit/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit/META-INF/MANIFEST.MF
@@ -172,6 +172,7 @@ Import-Package: com.googlecode.javaewah;version="[1.1.6,2.0.0)",
  org.bouncycastle.openpgp.operator;version="[1.61.0,2.0.0)",
  org.bouncycastle.openpgp.operator.jcajce;version="[1.61.0,2.0.0)",
  org.bouncycastle.util.encoders;version="[1.61.0,2.0.0)",
+ org.bouncycastle.util.io;version="[1.61.0,2.0.0)",
  org.slf4j;version="[1.7.0,2.0.0)",
  org.xml.sax,
  org.xml.sax.helpers
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
index b55987ead4..915b9860bb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java
@@ -143,6 +143,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 
 	private HashMap<String, PrintStream> hookOutRedirect = new HashMap<>(3);
 
+	private HashMap<String, PrintStream> hookErrRedirect = new HashMap<>(3);
+
 	private Boolean allowEmpty;
 
 	private Boolean signCommit;
@@ -188,7 +190,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 						state.name()));
 
 			if (!noVerify) {
-				Hooks.preCommit(repo, hookOutRedirect.get(PreCommitHook.NAME))
+				Hooks.preCommit(repo, hookOutRedirect.get(PreCommitHook.NAME),
+						hookErrRedirect.get(PreCommitHook.NAME))
 						.call();
 			}
 
@@ -230,7 +233,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 			if (!noVerify) {
 				message = Hooks
 						.commitMsg(repo,
-								hookOutRedirect.get(CommitMsgHook.NAME))
+								hookOutRedirect.get(CommitMsgHook.NAME),
+								hookErrRedirect.get(CommitMsgHook.NAME))
 						.setCommitMessage(message).call();
 			}
 
@@ -311,7 +315,8 @@ public class CommitCommand extends GitCommand<RevCommit> {
 						repo.writeRevertHead(null);
 					}
 					Hooks.postCommit(repo,
-							hookOutRedirect.get(PostCommitHook.NAME)).call();
+							hookOutRedirect.get(PostCommitHook.NAME),
+							hookErrRedirect.get(PostCommitHook.NAME)).call();
 
 					return revCommit;
 				}
@@ -891,6 +896,23 @@ public class CommitCommand extends GitCommand<RevCommit> {
 	}
 
 	/**
+	 * Set the error stream for all hook scripts executed by this command
+	 * (pre-commit, commit-msg, post-commit). If not set it defaults to
+	 * {@code System.err}.
+	 *
+	 * @param hookStdErr
+	 *            the error stream for hook scripts executed by this command
+	 * @return {@code this}
+	 * @since 5.6
+	 */
+	public CommitCommand setHookErrorStream(PrintStream hookStdErr) {
+		setHookErrorStream(PreCommitHook.NAME, hookStdErr);
+		setHookErrorStream(CommitMsgHook.NAME, hookStdErr);
+		setHookErrorStream(PostCommitHook.NAME, hookStdErr);
+		return this;
+	}
+
+	/**
 	 * Set the output stream for a selected hook script executed by this command
 	 * (pre-commit, commit-msg, post-commit). If not set it defaults to
 	 * {@code System.out}.
@@ -916,6 +938,30 @@ public class CommitCommand extends GitCommand<RevCommit> {
 	}
 
 	/**
+	 * Set the error stream for a selected hook script executed by this command
+	 * (pre-commit, commit-msg, post-commit). If not set it defaults to
+	 * {@code System.err}.
+	 *
+	 * @param hookName
+	 *            name of the hook to set the output stream for
+	 * @param hookStdErr
+	 *            the output stream to use for the selected hook
+	 * @return {@code this}
+	 * @since 5.6
+	 */
+	public CommitCommand setHookErrorStream(String hookName,
+			PrintStream hookStdErr) {
+		if (!(PreCommitHook.NAME.equals(hookName)
+				|| CommitMsgHook.NAME.equals(hookName)
+				|| PostCommitHook.NAME.equals(hookName))) {
+			throw new IllegalArgumentException(MessageFormat
+					.format(JGitText.get().illegalHookName, hookName));
+		}
+		hookErrRedirect.put(hookName, hookStdErr);
+		return this;
+	}
+
+	/**
 	 * Sets the signing key
 	 * <p>
 	 * Per spec of user.signingKey: this will be sent to the GPG program as is,
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/CommitMsgHook.java b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/CommitMsgHook.java
index f33168d814..6dbe0a6609 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/CommitMsgHook.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/CommitMsgHook.java
@@ -72,6 +72,9 @@ public class CommitMsgHook extends GitHook<String> {
 
 	/**
 	 * Constructor for CommitMsgHook
+	 * <p>
+	 * This constructor will use the default error stream.
+	 * </p>
 	 *
 	 * @param repo
 	 *            The repository
@@ -83,6 +86,24 @@ public class CommitMsgHook extends GitHook<String> {
 		super(repo, outputStream);
 	}
 
+	/**
+	 * Constructor for CommitMsgHook
+	 *
+	 * @param repo
+	 *            The repository
+	 * @param outputStream
+	 *            The output stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.out}.
+	 * @param errorStream
+	 *            The error stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.err}.
+	 * @since 5.6
+	 */
+	protected CommitMsgHook(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
+		super(repo, outputStream, errorStream);
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public String call() throws IOException, AbortedByHookException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/GitHook.java b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/GitHook.java
index 6bb5bfc4c4..aa307c9378 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/GitHook.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/GitHook.java
@@ -50,6 +50,7 @@ import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
 import java.util.concurrent.Callable;
 
+import org.bouncycastle.util.io.TeeOutputStream;
 import org.eclipse.jgit.api.errors.AbortedByHookException;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.util.FS;
@@ -79,7 +80,15 @@ abstract class GitHook<T> implements Callable<T> {
 	protected final PrintStream outputStream;
 
 	/**
-	 * Constructor for GitHook
+	 * The error stream to be used by the hook.
+	 */
+	protected final PrintStream errorStream;
+
+	/**
+	 * Constructor for GitHook.
+	 * <p>
+	 * This constructor will use stderr for the error stream.
+	 * </p>
 	 *
 	 * @param repo
 	 *            a {@link org.eclipse.jgit.lib.Repository} object.
@@ -88,8 +97,26 @@ abstract class GitHook<T> implements Callable<T> {
 	 *            in which case the hook will use {@code System.out}.
 	 */
 	protected GitHook(Repository repo, PrintStream outputStream) {
+		this(repo, outputStream, null);
+	}
+
+	/**
+	 * Constructor for GitHook
+	 *
+	 * @param repo
+	 *            a {@link org.eclipse.jgit.lib.Repository} object.
+	 * @param outputStream
+	 *            The output stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.out}.
+	 * @param errorStream
+	 *            The error stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.err}.
+	 */
+	protected GitHook(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
 		this.repo = repo;
 		this.outputStream = outputStream;
+		this.errorStream = errorStream;
 	}
 
 	/**
@@ -148,6 +175,16 @@ abstract class GitHook<T> implements Callable<T> {
 	}
 
 	/**
+	 * Get error stream
+	 *
+	 * @return The error stream the hook must use. Never {@code null},
+	 *         {@code System.err} is returned by default.
+	 */
+	protected PrintStream getErrorStream() {
+		return errorStream == null ? System.err : errorStream;
+	}
+
+	/**
 	 * Runs the hook, without performing any validity checks.
 	 *
 	 * @throws org.eclipse.jgit.api.errors.AbortedByHookException
@@ -155,9 +192,11 @@ abstract class GitHook<T> implements Callable<T> {
 	 */
 	protected void doRun() throws AbortedByHookException {
 		final ByteArrayOutputStream errorByteArray = new ByteArrayOutputStream();
+		final TeeOutputStream stderrStream = new TeeOutputStream(errorByteArray,
+				getErrorStream());
 		PrintStream hookErrRedirect = null;
 		try {
-			hookErrRedirect = new PrintStream(errorByteArray, false,
+			hookErrRedirect = new PrintStream(stderrStream, false,
 					UTF_8.name());
 		} catch (UnsupportedEncodingException e) {
 			// UTF-8 is guaranteed to be available
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/Hooks.java b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/Hooks.java
index b801d6872b..f29dcd178b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/Hooks.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/Hooks.java
@@ -57,7 +57,8 @@ import org.eclipse.jgit.util.LfsFactory;
 public class Hooks {
 
 	/**
-	 * Create pre-commit hook for the given repository
+	 * Create pre-commit hook for the given repository with the default error
+	 * stream
 	 *
 	 * @param repo
 	 *            a {@link org.eclipse.jgit.lib.Repository} object.
@@ -71,7 +72,25 @@ public class Hooks {
 	}
 
 	/**
-	 * Create post-commit hook for the given repository
+	 * Create pre-commit hook for the given repository
+	 *
+	 * @param repo
+	 *            a {@link org.eclipse.jgit.lib.Repository} object.
+	 * @param outputStream
+	 *            The output stream, or {@code null} to use {@code System.out}
+	 * @param errorStream
+	 *            The error stream, or {@code null} to use {@code System.err}
+	 * @return The pre-commit hook for the given repository.
+	 * @since 5.6
+	 */
+	public static PreCommitHook preCommit(Repository repo,
+			PrintStream outputStream, PrintStream errorStream) {
+		return new PreCommitHook(repo, outputStream, errorStream);
+	}
+
+	/**
+	 * Create post-commit hook for the given repository with the default error
+	 * stream
 	 *
 	 * @param repo
 	 *            a {@link org.eclipse.jgit.lib.Repository} object.
@@ -86,7 +105,25 @@ public class Hooks {
 	}
 
 	/**
-	 * Create commit-msg hook for the given repository
+	 * Create post-commit hook for the given repository
+	 *
+	 * @param repo
+	 *            a {@link org.eclipse.jgit.lib.Repository} object.
+	 * @param outputStream
+	 *            The output stream, or {@code null} to use {@code System.out}
+	 * @param errorStream
+	 *            The error stream, or {@code null} to use {@code System.err}
+	 * @return The pre-commit hook for the given repository.
+	 * @since 5.6
+	 */
+	public static PostCommitHook postCommit(Repository repo,
+			PrintStream outputStream, PrintStream errorStream) {
+		return new PostCommitHook(repo, outputStream, errorStream);
+	}
+
+	/**
+	 * Create commit-msg hook for the given repository with the default error
+	 * stream
 	 *
 	 * @param repo
 	 *            a {@link org.eclipse.jgit.lib.Repository} object.
@@ -100,7 +137,25 @@ public class Hooks {
 	}
 
 	/**
-	 * Create pre-push hook for the given repository
+	 * Create commit-msg hook for the given repository
+	 *
+	 * @param repo
+	 *            a {@link org.eclipse.jgit.lib.Repository} object.
+	 * @param outputStream
+	 *            The output stream, or {@code null} to use {@code System.out}
+	 * @param errorStream
+	 *            The error stream, or {@code null} to use {@code System.err}
+	 * @return The pre-commit hook for the given repository.
+	 * @since 5.6
+	 */
+	public static CommitMsgHook commitMsg(Repository repo,
+			PrintStream outputStream, PrintStream errorStream) {
+		return new CommitMsgHook(repo, outputStream, errorStream);
+	}
+
+	/**
+	 * Create pre-push hook for the given repository with the default error
+	 * stream
 	 *
 	 * @param repo
 	 *            a {@link org.eclipse.jgit.lib.Repository} object.
@@ -127,4 +182,36 @@ public class Hooks {
 		}
 		return new PrePushHook(repo, outputStream);
 	}
+
+	/**
+	 * Create pre-push hook for the given repository
+	 *
+	 * @param repo
+	 *            a {@link org.eclipse.jgit.lib.Repository} object.
+	 * @param outputStream
+	 *            The output stream, or {@code null} to use {@code System.out}
+	 * @param errorStream
+	 *            The error stream, or {@code null} to use {@code System.err}
+	 * @return The pre-push hook for the given repository.
+	 * @since 5.6
+	 */
+	public static PrePushHook prePush(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
+		if (LfsFactory.getInstance().isAvailable()) {
+			PrePushHook hook = LfsFactory.getInstance().getPrePushHook(repo,
+					outputStream, errorStream);
+			if (hook != null) {
+				if (hook.isNativeHookPresent()) {
+					PrintStream ps = outputStream;
+					if (ps == null) {
+						ps = System.out;
+					}
+					ps.println(MessageFormat
+							.format(JGitText.get().lfsHookConflict, repo));
+				}
+				return hook;
+			}
+		}
+		return new PrePushHook(repo, outputStream, errorStream);
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PostCommitHook.java b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PostCommitHook.java
index 24bad16ecb..b6e576fc23 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PostCommitHook.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PostCommitHook.java
@@ -61,6 +61,9 @@ public class PostCommitHook extends GitHook<Void> {
 
 	/**
 	 * Constructor for PostCommitHook
+	 * <p>
+	 * This constructor will use the default error stream.
+	 * </p>
 	 *
 	 * @param repo
 	 *            The repository
@@ -72,6 +75,24 @@ public class PostCommitHook extends GitHook<Void> {
 		super(repo, outputStream);
 	}
 
+	/**
+	 * Constructor for PostCommitHook
+	 *
+	 * @param repo
+	 *            The repository
+	 * @param outputStream
+	 *            The output stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.out}.
+	 * @param errorStream
+	 *            The error stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.err}.
+	 * @since 5.6
+	 */
+	protected PostCommitHook(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
+		super(repo, outputStream, errorStream);
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public Void call() throws IOException, AbortedByHookException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PreCommitHook.java b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PreCommitHook.java
index 0d9290da3e..dbdaf8669c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PreCommitHook.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PreCommitHook.java
@@ -61,6 +61,9 @@ public class PreCommitHook extends GitHook<Void> {
 
 	/**
 	 * Constructor for PreCommitHook
+	 * <p>
+	 * This constructor will use the default error stream.
+	 * </p>
 	 *
 	 * @param repo
 	 *            The repository
@@ -72,6 +75,24 @@ public class PreCommitHook extends GitHook<Void> {
 		super(repo, outputStream);
 	}
 
+	/**
+	 * Constructor for PreCommitHook
+	 *
+	 * @param repo
+	 *            The repository
+	 * @param outputStream
+	 *            The output stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.out}.
+	 * @param errorStream
+	 *            The error stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.err}.
+	 * @since 5.6
+	 */
+	protected PreCommitHook(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
+		super(repo, outputStream, errorStream);
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	public Void call() throws IOException, AbortedByHookException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PrePushHook.java b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PrePushHook.java
index 431944f9d4..61180fd021 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PrePushHook.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/hooks/PrePushHook.java
@@ -73,6 +73,9 @@ public class PrePushHook extends GitHook<String> {
 
 	/**
 	 * Constructor for PrePushHook
+	 * <p>
+	 * This constructor will use the default error stream.
+	 * </p>
 	 *
 	 * @param repo
 	 *            The repository
@@ -84,6 +87,24 @@ public class PrePushHook extends GitHook<String> {
 		super(repo, outputStream);
 	}
 
+	/**
+	 * Constructor for PrePushHook
+	 *
+	 * @param repo
+	 *            The repository
+	 * @param outputStream
+	 *            The output stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.out}.
+	 * @param errorStream
+	 *            The error stream the hook must use. {@code null} is allowed,
+	 *            in which case the hook will use {@code System.err}.
+	 * @since 5.6
+	 */
+	protected PrePushHook(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
+		super(repo, outputStream, errorStream);
+	}
+
 	/** {@inheritDoc} */
 	@Override
 	protected String getStdinArgs() {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/LfsFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/LfsFactory.java
index 96636b7994..85ee095013 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/LfsFactory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/LfsFactory.java
@@ -145,7 +145,7 @@ public class LfsFactory {
 	}
 
 	/**
-	 * Retrieve a pre-push hook to be applied.
+	 * Retrieve a pre-push hook to be applied using the default error stream.
 	 *
 	 * @param repo
 	 *            the {@link Repository} the hook is applied to.
@@ -159,6 +159,22 @@ public class LfsFactory {
 	}
 
 	/**
+	 * Retrieve a pre-push hook to be applied.
+	 *
+	 * @param repo
+	 *            the {@link Repository} the hook is applied to.
+	 * @param outputStream
+	 * @param errorStream
+	 * @return a {@link PrePushHook} implementation or <code>null</code>
+	 * @since 5.6
+	 */
+	@Nullable
+	public PrePushHook getPrePushHook(Repository repo, PrintStream outputStream,
+			PrintStream errorStream) {
+		return getPrePushHook(repo, outputStream);
+	}
+
+	/**
 	 * Retrieve an {@link LfsInstallCommand} which can be used to enable LFS
 	 * support (if available) either per repository or for the user.
 	 *