From 71af0d6a5c4417a9c9c6523d4aa811579d8c867f Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 20 Oct 2022 23:25:38 +0200 Subject: [PATCH] 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 --- .../org/eclipse/jgit/api/PushCommandTest.java | 53 ++++++++++++++++++- .../src/org/eclipse/jgit/api/PushCommand.java | 47 ++++++++++++++++ .../org/eclipse/jgit/transport/Transport.java | 46 +++++++++++++--- 3 files changed, 138 insertions(+), 8 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java index 6f7aa63edc..ff5f8b76cc 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java @@ -16,9 +16,12 @@ import static org.junit.Assert.assertThrows; 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 class PushCommandTest extends RepositoryTestCase { + "\"\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 class PushCommandTest extends RepositoryTestCase { } } + @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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java index 08353dfdfa..c4fb7a2184 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java @@ -11,6 +11,7 @@ package org.eclipse.jgit.api; 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 pushOptions; @@ -140,6 +145,8 @@ public class PushCommand extends transport.setOptionReceivePack(receivePack); transport.setDryRun(dryRun); transport.setPushOptions(pushOptions); + transport.setHookOutputStream(hookOutRedirect); + transport.setHookErrorStream(hookErrRedirect); configure(transport); final Collection toPush = transport @@ -304,6 +311,46 @@ public class PushCommand extends return remote; } + /** + * Sets a {@link PrintStream} a "pre-push" hook may write its stdout to. If + * not set, {@link System#out} will be used. + *

+ * When pushing to several remote repositories the stream is shared for all + * pushes. + *

+ * + * @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. + *

+ * When pushing to several remote repositories the stream is shared for all + * pushes. + *

+ * + * @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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java index 7cea998474..c3271ebc76 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java @@ -519,9 +519,7 @@ public abstract class Transport implements AutoCloseable { 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 @@ public abstract class Transport implements AutoCloseable { private PrintStream hookOutRedirect; - private PrePushHook prePush; + private PrintStream hookErrRedirect; + + private String remoteName; private Integer depth; @@ -812,7 +812,6 @@ public abstract class Transport implements AutoCloseable { this.protocol = tc.protocolVersion; this.objectChecker = tc.newObjectChecker(); this.credentialsProvider = CredentialsProvider.getDefault(); - prePush = Hooks.prePush(local, hookOutRedirect); } /** @@ -861,6 +860,32 @@ public abstract class Transport implements AutoCloseable { 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 abstract class Transport implements AutoCloseable { 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); } -- 2.39.5