]> source.dussan.org Git - jgit.git/commitdiff
push: support per-ref force-with-lease 27/88127/3
authorDavid Turner <dturner@twosigma.com>
Wed, 4 Jan 2017 04:56:08 +0000 (23:56 -0500)
committerDavid Turner <dturner@twosigma.com>
Thu, 9 Feb 2017 00:42:33 +0000 (19:42 -0500)
When rebasing, force-pushing has a race condition: someone else might
have pushed a commit since the one you just rewrote. The force-with-lease
option prevents this by ensuring that the ref's old value is the one
that you expected.

Change-Id: I97ca9f8395396c76332bdd07c486e60549ca4401
Signed-off-by: David Turner <dturner@twosigma.com>
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TransportTest.java
org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java
org.eclipse.jgit/src/org/eclipse/jgit/transport/RefLeaseSpec.java [new file with mode: 0644]
org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java

index 2a325405e8977ad6ad7ac97ffa02dcb1468a135c..eaf64b649f0b4ec1756da898f3b7d15286bbe1aa 100644 (file)
@@ -66,8 +66,10 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RefLeaseSpec;
 import org.eclipse.jgit.transport.RefSpec;
 import org.eclipse.jgit.transport.RemoteConfig;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.eclipse.jgit.transport.TrackingRefUpdate;
 import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.util.FS;
@@ -379,4 +381,56 @@ public class PushCommandTest extends RepositoryTestCase {
                                        db2.resolve(commit3.getId().getName() + "^{commit}"));
                }
        }
+
+       @Test
+       public void testPushWithLease() 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();
+
+               try (Git git1 = new Git(db)) {
+                       // create one commit and push it
+                       RevCommit commit = git1.commit().setMessage("initial commit").call();
+                       Ref branchRef = git1.branchCreate().setName("initial").call();
+
+                       RefSpec spec = new RefSpec("refs/heads/master:refs/heads/x");
+                       git1.push().setRemote("test").setRefSpecs(spec)
+                                       .call();
+
+                       assertEquals(commit.getId(),
+                                       db2.resolve(commit.getId().getName() + "^{commit}"));
+                       //now try to force-push a new commit, with a good lease
+
+                       RevCommit commit2 = git1.commit().setMessage("second commit").call();
+                       Iterable<PushResult> results =
+                                       git1.push().setRemote("test").setRefSpecs(spec)
+                                                       .setRefLeaseSpecs(new RefLeaseSpec("refs/heads/x", "initial"))
+                                                       .call();
+                       for (PushResult result : results) {
+                               RemoteRefUpdate update = result.getRemoteUpdate("refs/heads/x");
+                               assertEquals(update.getStatus(), RemoteRefUpdate.Status.OK);
+                       }
+
+                       RevCommit commit3 = git1.commit().setMessage("third commit").call();
+                       //now try to force-push a new commit, with a bad lease
+
+                       results =
+                                       git1.push().setRemote("test").setRefSpecs(spec)
+                                                       .setRefLeaseSpecs(new RefLeaseSpec("refs/heads/x", "initial"))
+                                                       .call();
+                       for (PushResult result : results) {
+                               RemoteRefUpdate update = result.getRemoteUpdate("refs/heads/x");
+                               assertEquals(update.getStatus(), RemoteRefUpdate.Status.REJECTED_REMOTE_CHANGED);
+                       }
+               }
+       }
 }
index 5519f61ac255e6b4953ef8494aafc0a9d1a7db07..d4c47d37e3da1950d8fcf69243d77b9b6359958a 100644 (file)
@@ -53,7 +53,9 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
@@ -209,6 +211,45 @@ public class TransportTest extends SampleDataRepositoryTestCase {
                assertEquals(ObjectId.zeroId(), tru.getOldObjectId());
        }
 
+       /**
+        * Test RefSpec to RemoteRefUpdate conversion with leases.
+        *
+        * @throws IOException
+        */
+       @Test
+       public void testFindRemoteRefUpdatesWithLeases() throws IOException {
+               final RefSpec specA = new RefSpec("+refs/heads/a:refs/heads/b");
+               final RefSpec specC = new RefSpec("+refs/heads/c:refs/heads/d");
+               final Collection<RefSpec> specs = Arrays.asList(specA, specC);
+               final Map<String, RefLeaseSpec> leases = new HashMap<>();
+               leases.put("refs/heads/b",
+                               new RefLeaseSpec("refs/heads/b", "refs/heads/c"));
+
+               Collection<RemoteRefUpdate> result;
+               try (Transport transport = Transport.open(db, remoteConfig)) {
+                       result = transport.findRemoteRefUpdatesFor(specs, leases);
+               }
+
+               assertEquals(2, result.size());
+               boolean foundA = false;
+               boolean foundC = false;
+               for (final RemoteRefUpdate rru : result) {
+                       if ("refs/heads/a".equals(rru.getSrcRef())
+                                       && "refs/heads/b".equals(rru.getRemoteName())) {
+                               foundA = true;
+                               assertEquals(db.exactRef("refs/heads/c").getObjectId(),
+                                               rru.getExpectedOldObjectId());
+                       }
+                       if ("refs/heads/c".equals(rru.getSrcRef())
+                                       && "refs/heads/d".equals(rru.getRemoteName())) {
+                               foundC = true;
+                               assertNull(rru.getExpectedOldObjectId());
+                       }
+               }
+               assertTrue(foundA);
+               assertTrue(foundC);
+       }
+
        @Test
        public void testLocalTransportWithRelativePath() throws Exception {
                Repository other = createWorkRepository();
index bd4521b5175e36ac672a073c7648528501cd2249..be15ab13bd96f12c9c527b50198bd7ab89fcae85 100644 (file)
@@ -47,9 +47,12 @@ import java.io.OutputStream;
 import java.net.URISyntaxException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.InvalidRemoteException;
@@ -65,6 +68,7 @@ import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RefLeaseSpec;
 import org.eclipse.jgit.transport.RefSpec;
 import org.eclipse.jgit.transport.RemoteConfig;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
@@ -85,6 +89,8 @@ public class PushCommand extends
 
        private final List<RefSpec> refSpecs;
 
+       private final Map<String, RefLeaseSpec> refLeaseSpecs;
+
        private ProgressMonitor monitor = NullProgressMonitor.INSTANCE;
 
        private String receivePack = RemoteConfig.DEFAULT_RECEIVE_PACK;
@@ -104,6 +110,7 @@ public class PushCommand extends
        protected PushCommand(Repository repo) {
                super(repo);
                refSpecs = new ArrayList<RefSpec>(3);
+               refLeaseSpecs = new HashMap<>();
        }
 
        /**
@@ -155,7 +162,7 @@ public class PushCommand extends
                                configure(transport);
 
                                final Collection<RemoteRefUpdate> toPush = transport
-                                               .findRemoteRefUpdatesFor(refSpecs);
+                                               .findRemoteRefUpdatesFor(refSpecs, refLeaseSpecs);
 
                                try {
                                        PushResult result = transport.push(monitor, toPush, out);
@@ -270,6 +277,43 @@ public class PushCommand extends
                return this;
        }
 
+       /**
+        * @return the ref lease specs
+        * @since 4.7
+        */
+       public List<RefLeaseSpec> getRefLeaseSpecs() {
+               return new ArrayList<RefLeaseSpec>(refLeaseSpecs.values());
+       }
+
+       /**
+        * The ref lease specs to be used in the push operation,
+        * for a force-with-lease push operation.
+        *
+        * @param specs
+        * @return {@code this}
+        * @since 4.7
+        */
+       public PushCommand setRefLeaseSpecs(RefLeaseSpec... specs) {
+               return setRefLeaseSpecs(Arrays.asList(specs));
+       }
+
+       /**
+        * The ref lease specs to be used in the push operation,
+        * for a force-with-lease push operation.
+        *
+        * @param specs
+        * @return {@code this}
+        * @since 4.7
+        */
+       public PushCommand setRefLeaseSpecs(List<RefLeaseSpec> specs) {
+               checkCallable();
+               this.refLeaseSpecs.clear();
+               for (RefLeaseSpec spec : specs) {
+                       refLeaseSpecs.put(spec.getRef(), spec);
+               }
+               return this;
+       }
+
        /**
         * @return the ref specs
         */
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefLeaseSpec.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefLeaseSpec.java
new file mode 100644 (file)
index 0000000..02c3c3c
--- /dev/null
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2017 Two Sigma Open Source
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.transport;
+
+import java.io.Serializable;
+
+import org.eclipse.jgit.internal.JGitText;
+
+/**
+ * Describes the expected value for a ref being pushed.
+ * @since 4.7
+ */
+public class RefLeaseSpec implements Serializable {
+       private static final long serialVersionUID = 1L;
+
+       /** Name of the ref whose value we want to check. */
+       private final String ref;
+
+       /** Local commitish to get expected value from. */
+       private final String expected;
+
+       private RefLeaseSpec(final RefLeaseSpec p) {
+               ref = p.getRef();
+               expected = p.getExpected();
+       }
+
+       public RefLeaseSpec(final String ref, final String expected) {
+               this.ref = ref;
+               this.expected = expected;
+       }
+
+       /**
+        * Get the ref to protect.
+        *
+        * @return name of ref to check.
+        */
+       public String getRef() {
+               return ref;
+       }
+
+       /**
+        * Get the expected value of the ref, in the form
+        * of a local committish
+        *
+        * @return expected ref value.
+        */
+       public String getExpected() {
+               return expected;
+       }
+
+       public String toString() {
+               final StringBuilder r = new StringBuilder();
+               r.append(getRef());
+               r.append(':');
+               r.append(getExpected());
+               return r.toString();
+       }
+}
index df860695df4caa0445edbea89091be66a5de5a9f..64bdbe7d4a5427c22ba92d740be34239f8ffbe38 100644 (file)
@@ -80,6 +80,7 @@ import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectChecker;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
@@ -603,14 +604,16 @@ public abstract class Transport implements AutoCloseable {
         * Convert push remote refs update specification from {@link RefSpec} form
         * to {@link RemoteRefUpdate}. Conversion expands wildcards by matching
         * source part to local refs. expectedOldObjectId in RemoteRefUpdate is
-        * always set as null. Tracking branch is configured if RefSpec destination
-        * matches source of any fetch ref spec for this transport remote
-        * configuration.
+        * set when specified in leases. Tracking branch is configured if RefSpec
+        * destination matches source of any fetch ref spec for this transport
+        * remote configuration.
         *
         * @param db
         *            local database.
         * @param specs
         *            collection of RefSpec to convert.
+        * @param leases
+        *            map from ref to lease (containing expected old object id)
         * @param fetchSpecs
         *            fetch specifications used for finding localtracking refs. May
         *            be null or empty collection.
@@ -618,9 +621,11 @@ public abstract class Transport implements AutoCloseable {
         * @throws IOException
         *             when problem occurred during conversion or specification set
         *             up: most probably, missing objects or refs.
+        * @since 4.7
         */
        public static Collection<RemoteRefUpdate> findRemoteRefUpdatesFor(
                        final Repository db, final Collection<RefSpec> specs,
+                       final Map<String, RefLeaseSpec> leases,
                        Collection<RefSpec> fetchSpecs) throws IOException {
                if (fetchSpecs == null)
                        fetchSpecs = Collections.emptyList();
@@ -652,13 +657,43 @@ public abstract class Transport implements AutoCloseable {
 
                        final boolean forceUpdate = spec.isForceUpdate();
                        final String localName = findTrackingRefName(destSpec, fetchSpecs);
+                       final RefLeaseSpec leaseSpec = leases.get(destSpec);
+                       final ObjectId expected = leaseSpec == null ? null :
+                               db.resolve(leaseSpec.getExpected());
                        final RemoteRefUpdate rru = new RemoteRefUpdate(db, srcSpec,
-                                       destSpec, forceUpdate, localName, null);
+                                       destSpec, forceUpdate, localName, expected);
                        result.add(rru);
                }
                return result;
        }
 
+       /**
+        * Convert push remote refs update specification from {@link RefSpec} form
+        * to {@link RemoteRefUpdate}. Conversion expands wildcards by matching
+        * source part to local refs. expectedOldObjectId in RemoteRefUpdate is
+        * always set as null. Tracking branch is configured if RefSpec destination
+        * matches source of any fetch ref spec for this transport remote
+        * configuration.
+        *
+        * @param db
+        *            local database.
+        * @param specs
+        *            collection of RefSpec to convert.
+        * @param fetchSpecs
+        *            fetch specifications used for finding localtracking refs. May
+        *            be null or empty collection.
+        * @return collection of set up {@link RemoteRefUpdate}.
+        * @throws IOException
+        *             when problem occurred during conversion or specification set
+        *             up: most probably, missing objects or refs.
+        */
+       public static Collection<RemoteRefUpdate> findRemoteRefUpdatesFor(
+                       final Repository db, final Collection<RefSpec> specs,
+                       Collection<RefSpec> fetchSpecs) throws IOException {
+               return findRemoteRefUpdatesFor(db, specs, Collections.emptyMap(),
+                                              fetchSpecs);
+       }
+
        private static Collection<RefSpec> expandPushWildcardsFor(
                        final Repository db, final Collection<RefSpec> specs)
                        throws IOException {
@@ -1341,7 +1376,36 @@ public abstract class Transport implements AutoCloseable {
         */
        public Collection<RemoteRefUpdate> findRemoteRefUpdatesFor(
                        final Collection<RefSpec> specs) throws IOException {
-               return findRemoteRefUpdatesFor(local, specs, fetch);
+               return findRemoteRefUpdatesFor(local, specs, Collections.emptyMap(),
+                                              fetch);
+       }
+
+       /**
+        * Convert push remote refs update specification from {@link RefSpec} form
+        * to {@link RemoteRefUpdate}. Conversion expands wildcards by matching
+        * source part to local refs. expectedOldObjectId in RemoteRefUpdate is
+        * set according to leases. Tracking branch is configured if RefSpec destination
+        * matches source of any fetch ref spec for this transport remote
+        * configuration.
+        * <p>
+        * Conversion is performed for context of this transport (database, fetch
+        * specifications).
+        *
+        * @param specs
+        *            collection of RefSpec to convert.
+        * @param leases
+        *            map from ref to lease (containing expected old object id)
+        * @return collection of set up {@link RemoteRefUpdate}.
+        * @throws IOException
+        *             when problem occurred during conversion or specification set
+        *             up: most probably, missing objects or refs.
+        * @since 4.7
+        */
+       public Collection<RemoteRefUpdate> findRemoteRefUpdatesFor(
+                       final Collection<RefSpec> specs,
+                       final Map<String, RefLeaseSpec> leases) throws IOException {
+               return findRemoteRefUpdatesFor(local, specs, leases,
+                                              fetch);
        }
 
        /**