[push] Call the pre-push hook later in the push process
Call the pre-push hook only after having received the remote
advertisement and having determined rejections, like C git does.
Also similar to C git, don't pass rejected or up-to-date updates
to the pre-push hook.
Bug: 578852
Change-Id: I51d379ea7bd8234ec815f8f4a9fa325816f476cf
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushProcessTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushProcessTest.java
index 6928859..2e8b30f 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushProcessTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushProcessTest.java
@@ -14,14 +14,19 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import org.eclipse.jgit.api.errors.AbortedByHookException;
import org.eclipse.jgit.errors.NotSupportedException;
import org.eclipse.jgit.errors.TransportException;
+import org.eclipse.jgit.hooks.PrePushHook;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.ProgressMonitor;
@@ -31,6 +36,7 @@
import org.eclipse.jgit.lib.TextProgressMonitor;
import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.eclipse.jgit.util.io.NullOutputStream;
import org.junit.Before;
import org.junit.Test;
@@ -220,7 +226,17 @@ public void testUpdateUnexpectedRemoteVsForce() throws IOException {
.fromString("0000000000000000000000000000000000000001"));
final Ref ref = new ObjectIdRef.Unpeeled(Ref.Storage.LOOSE, "refs/heads/master",
ObjectId.fromString("ac7e7e44c1885efb472ad54a78327d66bfc4ecef"));
- testOneUpdateStatus(rru, ref, Status.REJECTED_REMOTE_CHANGED, null);
+ try (ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ PrintStream out = new PrintStream(bytes, true,
+ StandardCharsets.UTF_8);
+ PrintStream err = new PrintStream(NullOutputStream.INSTANCE)) {
+ MockPrePushHook hook = new MockPrePushHook(db, out, err);
+ testOneUpdateStatus(rru, ref, Status.REJECTED_REMOTE_CHANGED, null,
+ hook);
+ out.flush();
+ String result = new String(bytes.toString(StandardCharsets.UTF_8));
+ assertEquals("", result);
+ }
}
/**
@@ -256,10 +272,22 @@ public void testUpdateMixedCases() throws IOException {
refUpdates.add(rruOk);
refUpdates.add(rruReject);
advertisedRefs.add(refToChange);
- executePush();
- assertEquals(Status.OK, rruOk.getStatus());
- assertTrue(rruOk.isFastForward());
- assertEquals(Status.NON_EXISTING, rruReject.getStatus());
+ try (ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ PrintStream out = new PrintStream(bytes, true,
+ StandardCharsets.UTF_8);
+ PrintStream err = new PrintStream(NullOutputStream.INSTANCE)) {
+ MockPrePushHook hook = new MockPrePushHook(db, out, err);
+ executePush(hook);
+ assertEquals(Status.OK, rruOk.getStatus());
+ assertTrue(rruOk.isFastForward());
+ assertEquals(Status.NON_EXISTING, rruReject.getStatus());
+ out.flush();
+ String result = new String(bytes.toString(StandardCharsets.UTF_8));
+ assertEquals(
+ "null 0000000000000000000000000000000000000000 "
+ + "refs/heads/master 2c349335b7f797072cf729c4f3bb0914ecb6dec9\n",
+ result);
+ }
}
/**
@@ -346,10 +374,18 @@ private PushResult testOneUpdateStatus(final RemoteRefUpdate rru,
final Ref advertisedRef, final Status expectedStatus,
Boolean fastForward) throws NotSupportedException,
TransportException {
+ return testOneUpdateStatus(rru, advertisedRef, expectedStatus,
+ fastForward, null);
+ }
+
+ private PushResult testOneUpdateStatus(final RemoteRefUpdate rru,
+ final Ref advertisedRef, final Status expectedStatus,
+ Boolean fastForward, PrePushHook hook)
+ throws NotSupportedException, TransportException {
refUpdates.add(rru);
if (advertisedRef != null)
advertisedRefs.add(advertisedRef);
- final PushResult result = executePush();
+ final PushResult result = executePush(hook);
assertEquals(expectedStatus, rru.getStatus());
if (fastForward != null)
assertEquals(fastForward, Boolean.valueOf(rru.isFastForward()));
@@ -358,7 +394,12 @@ private PushResult testOneUpdateStatus(final RemoteRefUpdate rru,
private PushResult executePush() throws NotSupportedException,
TransportException {
- process = new PushProcess(transport, refUpdates);
+ return executePush(null);
+ }
+
+ private PushResult executePush(PrePushHook hook)
+ throws NotSupportedException, TransportException {
+ process = new PushProcess(transport, refUpdates, hook);
return process.execute(new TextProgressMonitor());
}
@@ -416,4 +457,20 @@ public void push(ProgressMonitor monitor,
}
}
}
+
+ private static class MockPrePushHook extends PrePushHook {
+
+ private final PrintStream output;
+
+ public MockPrePushHook(Repository repo, PrintStream out,
+ PrintStream err) {
+ super(repo, out, err);
+ output = out;
+ }
+
+ @Override
+ protected void doRun() throws AbortedByHookException, IOException {
+ output.print(getStdinArgs());
+ }
+ }
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java
index a244c55..c4f2769 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java
@@ -18,10 +18,13 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.api.errors.AbortedByHookException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.NotSupportedException;
import org.eclipse.jgit.errors.TransportException;
+import org.eclipse.jgit.hooks.PrePushHook;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ProgressMonitor;
@@ -58,6 +61,8 @@ class PushProcess {
/** A list of option strings associated with this push */
private List<String> pushOptions;
+ private final PrePushHook prePush;
+
/**
* Create process for specified transport and refs updates specification.
*
@@ -66,12 +71,15 @@ class PushProcess {
* connection.
* @param toPush
* specification of refs updates (and local tracking branches).
- *
+ * @param prePush
+ * {@link PrePushHook} to run after the remote advertisement has
+ * been gotten
* @throws TransportException
*/
PushProcess(final Transport transport,
- final Collection<RemoteRefUpdate> toPush) throws TransportException {
- this(transport, toPush, null);
+ final Collection<RemoteRefUpdate> toPush, PrePushHook prePush)
+ throws TransportException {
+ this(transport, toPush, prePush, null);
}
/**
@@ -82,16 +90,20 @@ class PushProcess {
* connection.
* @param toPush
* specification of refs updates (and local tracking branches).
+ * @param prePush
+ * {@link PrePushHook} to run after the remote advertisement has
+ * been gotten
* @param out
* OutputStream to write messages to
* @throws TransportException
*/
PushProcess(final Transport transport,
- final Collection<RemoteRefUpdate> toPush, OutputStream out)
- throws TransportException {
+ final Collection<RemoteRefUpdate> toPush, PrePushHook prePush,
+ OutputStream out) throws TransportException {
this.walker = new RevWalk(transport.local);
this.transport = transport;
this.toPush = new LinkedHashMap<>();
+ this.prePush = prePush;
this.out = out;
this.pushOptions = transport.getPushOptions();
for (RemoteRefUpdate rru : toPush) {
@@ -133,6 +145,30 @@ PushResult execute(ProgressMonitor monitor)
monitor.endTask();
final Map<String, RemoteRefUpdate> preprocessed = prepareRemoteUpdates();
+ List<RemoteRefUpdate> willBeAttempted = preprocessed.values()
+ .stream().filter(u -> {
+ switch (u.getStatus()) {
+ case NON_EXISTING:
+ case REJECTED_NODELETE:
+ case REJECTED_NONFASTFORWARD:
+ case REJECTED_OTHER_REASON:
+ case REJECTED_REMOTE_CHANGED:
+ case UP_TO_DATE:
+ return false;
+ default:
+ return true;
+ }
+ }).collect(Collectors.toList());
+ if (!willBeAttempted.isEmpty()) {
+ if (prePush != null) {
+ try {
+ prePush.setRefs(willBeAttempted);
+ prePush.call();
+ } catch (AbortedByHookException | IOException e) {
+ throw new TransportException(e.getMessage(), e);
+ }
+ }
+ }
if (transport.isDryRun())
modifyUpdatesForDryRun();
else if (!preprocessed.isEmpty())
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 bfe26d9..1245f78 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
@@ -40,7 +40,6 @@
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
-import org.eclipse.jgit.api.errors.AbortedByHookException;
import org.eclipse.jgit.errors.NotSupportedException;
import org.eclipse.jgit.errors.TransportException;
import org.eclipse.jgit.hooks.Hooks;
@@ -1375,16 +1374,9 @@ public PushResult push(final ProgressMonitor monitor,
if (toPush.isEmpty())
throw new TransportException(JGitText.get().nothingToPush);
}
- if (prePush != null) {
- try {
- prePush.setRefs(toPush);
- prePush.call();
- } catch (AbortedByHookException | IOException e) {
- throw new TransportException(e.getMessage(), e);
- }
- }
- final PushProcess pushProcess = new PushProcess(this, toPush, out);
+ final PushProcess pushProcess = new PushProcess(this, toPush, prePush,
+ out);
return pushProcess.execute(monitor);
}
