[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>

2 files changed, 43 insertions, 15 deletions

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 a244c55a38..c4f276988c 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.Collections;
 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 @@ class PushProcess {
 				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 bfe26d9808..1245f78a6e 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 java.util.concurrent.CopyOnWriteArrayList;
 
 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 abstract class Transport implements AutoCloseable {
 			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);
 	}