From: Thomas Wolf Date: Sun, 20 Feb 2022 17:10:24 +0000 (+0100) Subject: [push] Call the pre-push hook later in the push process X-Git-Tag: v6.1.0.202203080745-r~4 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fchanges%2F89%2F190989%2F2;p=jgit.git [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 --- 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 6928859622..2e8b30f151 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.assertEquals; 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.Repository; 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 class PushProcessTest extends SampleDataRepositoryTestCase { .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 class PushProcessTest extends SampleDataRepositoryTestCase { 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 @@ public class PushProcessTest extends SampleDataRepositoryTestCase { 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 @@ public class PushProcessTest extends SampleDataRepositoryTestCase { 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 class PushProcessTest extends SampleDataRepositoryTestCase { } } } + + 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 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 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 toPush) throws TransportException { - this(transport, toPush, null); + final Collection 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 toPush, OutputStream out) - throws TransportException { + final Collection 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 preprocessed = prepareRemoteUpdates(); + List 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); }