I/O redirection for the pre-push hook

Fix and complete the implementation of calling the pre-push hook.
Add the missing error stream redirect, and add the missing setters
in Transport and in PushCommand. In Transport, delay setting up a
PrePushHook such that it happens only on a push. Previously, the
hook was set up also for fetches.

Bug: 549246
Change-Id: I64a576dfc6b139426f05d9ea6654027ab805734e
Signed-off-by: Thomas Wolf <twolf@apache.org>

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java

@@ -16,9 +16,12 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.PrintStream;
 import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
 import java.util.Properties;
 
 import org.eclipse.jgit.api.errors.DetachedHeadException;
@@ -115,7 +118,7 @@ public void testPrePushHook() throws JGitInternalException, IOException,
 				+ "\"\nexit 0");
 
 		try (Git git1 = new Git(db)) {
-			// create some refs via commits and tag
+			// create a commit
 			RevCommit commit = git1.commit().setMessage("initial commit").call();
 
 			RefSpec spec = new RefSpec("refs/heads/master:refs/heads/x");
@@ -126,6 +129,54 @@ public void testPrePushHook() throws JGitInternalException, IOException,
 		}
 	}
 
+	@Test
+	public void testPrePushHookRedirects() throws JGitInternalException,
+			IOException, GitAPIException, URISyntaxException {
+
+		// create other repository
+		Repository db2 = createWorkRepository();
+
+		// setup the first repository
+		final StoredConfig config = db.getConfig();
+		RemoteConfig remoteConfig = new RemoteConfig(config, "test");
+		URIish uri = new URIish(db2.getDirectory().toURI().toURL());
+		remoteConfig.addURI(uri);
+		remoteConfig.update(config);
+		config.save();
+
+		writeHookFile(PrePushHook.NAME, "#!/bin/sh\n"
+				+ "echo \"1:$1, 2:$2, 3:$3\"\n" // to stdout
+				+ "cat - 1>&2\n" // to stderr
+				+ "exit 0\n");
+
+		try (Git git1 = new Git(db)) {
+			// create a commit
+			RevCommit commit = git1.commit().setMessage("initial commit")
+					.call();
+
+			RefSpec spec = new RefSpec("refs/heads/master:refs/heads/x");
+			try (ByteArrayOutputStream outBytes = new ByteArrayOutputStream();
+					ByteArrayOutputStream errBytes = new ByteArrayOutputStream();
+					PrintStream stdout = new PrintStream(outBytes, true,
+							StandardCharsets.UTF_8);
+					PrintStream stderr = new PrintStream(errBytes, true,
+							StandardCharsets.UTF_8)) {
+				git1.push()
+						.setRemote("test")
+						.setRefSpecs(spec)
+						.setHookOutputStream(stdout)
+						.setHookErrorStream(stderr)
+						.call();
+				String out = outBytes.toString(StandardCharsets.UTF_8);
+				String err = errBytes.toString(StandardCharsets.UTF_8);
+				assertEquals("1:test, 2:" + uri + ", 3:\n", out);
+				assertEquals("refs/heads/master " + commit.getName()
+						+ " refs/heads/x " + ObjectId.zeroId().name() + '\n',
+						err);
+			}
+		}
+	}
+
 	private File writeHookFile(String name, String data)
 			throws IOException {
 		File path = new File(db.getWorkTree() + "/.git/hooks/", name);

org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java

@@ -11,6 +11,7 @@
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.PrintStream;
 import java.net.URISyntaxException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
@@ -74,6 +75,10 @@ public class PushCommand extends
 	private boolean force;
 	private boolean thin = Transport.DEFAULT_PUSH_THIN;
 
+	private PrintStream hookOutRedirect;
+
+	private PrintStream hookErrRedirect;
+
 	private OutputStream out;
 
 	private List<String> pushOptions;
@@ -140,6 +145,8 @@ public Iterable<PushResult> call() throws GitAPIException,
 					transport.setOptionReceivePack(receivePack);
 				transport.setDryRun(dryRun);
 				transport.setPushOptions(pushOptions);
+				transport.setHookOutputStream(hookOutRedirect);
+				transport.setHookErrorStream(hookErrRedirect);
 				configure(transport);
 
 				final Collection<RemoteRefUpdate> toPush = transport
@@ -304,6 +311,46 @@ public String getRemote() {
 		return remote;
 	}
 
+	/**
+	 * Sets a {@link PrintStream} a "pre-push" hook may write its stdout to. If
+	 * not set, {@link System#out} will be used.
+	 * <p>
+	 * When pushing to several remote repositories the stream is shared for all
+	 * pushes.
+	 * </p>
+	 *
+	 * @param redirect
+	 *            {@link PrintStream} to use; if {@code null},
+	 *            {@link System#out} will be used
+	 * @return {@code this}
+	 * @since 6.4
+	 */
+	public PushCommand setHookOutputStream(PrintStream redirect) {
+		checkCallable();
+		hookOutRedirect = redirect;
+		return this;
+	}
+
+	/**
+	 * Sets a {@link PrintStream} a "pre-push" hook may write its stderr to. If
+	 * not set, {@link System#err} will be used.
+	 * <p>
+	 * When pushing to several remote repositories the stream is shared for all
+	 * pushes.
+	 * </p>
+	 *
+	 * @param redirect
+	 *            {@link PrintStream} to use; if {@code null},
+	 *            {@link System#err} will be used
+	 * @return {@code this}
+	 * @since 6.4
+	 */
+	public PushCommand setHookErrorStream(PrintStream redirect) {
+		checkCallable();
+		hookErrRedirect = redirect;
+		return this;
+	}
+
 	/**
 	 * The remote executable providing receive-pack service for pack transports.
 	 * If no receive-pack is set, the default value of

org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java

@@ -519,9 +519,7 @@ public static Transport open(Repository local, URIish uri, String remoteName)
 
 			if (proto.canHandle(uri, local, remoteName)) {
 				Transport tn = proto.open(uri, local, remoteName);
-				tn.prePush = Hooks.prePush(local, tn.hookOutRedirect);
-				tn.prePush.setRemoteLocation(uri.toString());
-				tn.prePush.setRemoteName(remoteName);
+				tn.remoteName = remoteName;
 				return tn;
 			}
 		}
@@ -783,7 +781,9 @@ static String findTrackingRefName(final String remoteName,
 
 	private PrintStream hookOutRedirect;
 
-	private PrePushHook prePush;
+	private PrintStream hookErrRedirect;
+
+	private String remoteName;
 
 	private Integer depth;
 
@@ -812,7 +812,6 @@ protected Transport(Repository local, URIish uri) {
 		this.protocol = tc.protocolVersion;
 		this.objectChecker = tc.newObjectChecker();
 		this.credentialsProvider = CredentialsProvider.getDefault();
-		prePush = Hooks.prePush(local, hookOutRedirect);
 	}
 
 	/**
@@ -861,6 +860,32 @@ public void setOptionUploadPack(String where) {
 			optionUploadPack = RemoteConfig.DEFAULT_UPLOAD_PACK;
 	}
 
+	/**
+	 * Sets a {@link PrintStream} a {@link PrePushHook} may write its stdout to.
+	 * If not set, {@link System#out} will be used.
+	 *
+	 * @param redirect
+	 *            {@link PrintStream} to use; if {@code null},
+	 *            {@link System#out} will be used
+	 * @since 6.4
+	 */
+	public void setHookOutputStream(PrintStream redirect) {
+		hookOutRedirect = redirect;
+	}
+
+	/**
+	 * Sets a {@link PrintStream} a {@link PrePushHook} may write its stderr to.
+	 * If not set, {@link System#err} will be used.
+	 *
+	 * @param redirect
+	 *            {@link PrintStream} to use; if {@code null},
+	 *            {@link System#err} will be used
+	 * @since 6.4
+	 */
+	public void setHookErrorStream(PrintStream redirect) {
+		hookErrRedirect = redirect;
+	}
+
 	/**
 	 * Get the description of how annotated tags should be treated during fetch.
 	 *
@@ -1468,8 +1493,15 @@ public PushResult push(final ProgressMonitor monitor,
 				throw new TransportException(JGitText.get().nothingToPush);
 		}
 
-		final PushProcess pushProcess = new PushProcess(this, toPush, prePush,
-				out);
+		PrePushHook prePush = null;
+		if (local != null) {
+			// Pushing will always have a local repository. But better safe than
+			// sorry.
+			prePush = Hooks.prePush(local, hookOutRedirect, hookErrRedirect);
+			prePush.setRemoteLocation(uri.toString());
+			prePush.setRemoteName(remoteName);
+		}
+		PushProcess pushProcess = new PushProcess(this, toPush, prePush, out);
 		return pushProcess.execute(monitor);
 	}