[push] support the "matching" RefSpecs ":" and "+:"

The implementation of push.default=matching was not correct.
It used the RefSpec "refs/heads/*:refs/heads/*", which would push
_all_ local branches. But "matching" must push only those local
branches for which a remote branch with the same name already exists
at the remote.

This RefSpec can be expanded only once the advertisement from the
remote has been received.

Enhance RefSpec so that ":" and "+:" can be represented. Introduce a
special RemoteRefUpdate for such a RefSpec; it must carry through the
fetch RefSpecs to be able to fill in the remote tracking updates as
needed. Implement the expansion in PushProcess.

Bug: 353405
Change-Id: I54a4bfbb0a6a7d77b9128bf4a9c951d6586c3df4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

5 files changed, 280 insertions, 64 deletions

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 4f57f35127..08353dfdfa 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/PushCommand.java
@@ -230,7 +230,7 @@ public class PushCommand extends
 			refSpecs.add(new RefSpec(getCurrentBranch()));
 			break;
 		case MATCHING:
-			setPushAll();
+			refSpecs.add(new RefSpec(":")); //$NON-NLS-1$
 			break;
 		case NOTHING:
 			throw new InvalidRefNameException(
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 c4f276988c..c76cee6984 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java
@@ -26,6 +26,7 @@ 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.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
@@ -141,9 +142,13 @@ class PushProcess {
 				res.setAdvertisedRefs(transport.getURI(), connection
 						.getRefsMap());
 				res.peerUserAgent = connection.getPeerUserAgent();
-				res.setRemoteUpdates(toPush);
 				monitor.endTask();
 
+				Map<String, RemoteRefUpdate> expanded = expandMatching();
+				toPush.clear();
+				toPush.putAll(expanded);
+
+				res.setRemoteUpdates(toPush);
 				final Map<String, RemoteRefUpdate> preprocessed = prepareRemoteUpdates();
 				List<RemoteRefUpdate> willBeAttempted = preprocessed.values()
 						.stream().filter(u -> {
@@ -237,25 +242,8 @@ class PushProcess {
 				continue;
 			}
 
-			// check for fast-forward:
-			// - both old and new ref must point to commits, AND
-			// - both of them must be known for us, exist in repository, AND
-			// - old commit must be ancestor of new commit
-			boolean fastForward = true;
-			try {
-				RevObject oldRev = walker.parseAny(advertisedOld);
-				final RevObject newRev = walker.parseAny(rru.getNewObjectId());
-				if (!(oldRev instanceof RevCommit)
-						|| !(newRev instanceof RevCommit)
-						|| !walker.isMergedInto((RevCommit) oldRev,
-								(RevCommit) newRev))
-					fastForward = false;
-			} catch (MissingObjectException x) {
-				fastForward = false;
-			} catch (Exception x) {
-				throw new TransportException(transport.getURI(), MessageFormat.format(
-						JGitText.get().readingObjectsFromLocalRepositoryFailed, x.getMessage()), x);
-			}
+			boolean fastForward = isFastForward(advertisedOld,
+					rru.getNewObjectId());
 			rru.setFastForward(fastForward);
 			if (!fastForward && !rru.isForceUpdate()) {
 				rru.setStatus(Status.REJECTED_NONFASTFORWARD);
@@ -269,6 +257,134 @@ class PushProcess {
 		return result;
 	}
 
+	/**
+	 * Determines whether an update from {@code oldOid} to {@code newOid} is a
+	 * fast-forward update:
+	 * <ul>
+	 * <li>both old and new must be commits, AND</li>
+	 * <li>both of them must be known to us and exist in the repository,
+	 * AND</li>
+	 * <li>the old commit must be an ancestor of the new commit.</li>
+	 * </ul>
+	 *
+	 * @param oldOid
+	 *            {@link ObjectId} of the old commit
+	 * @param newOid
+	 *            {@link ObjectId} of the new commit
+	 * @return {@code true} if the update fast-forwards, {@code false} otherwise
+	 * @throws TransportException
+	 */
+	private boolean isFastForward(ObjectId oldOid, ObjectId newOid)
+			throws TransportException {
+		try {
+			RevObject oldRev = walker.parseAny(oldOid);
+			RevObject newRev = walker.parseAny(newOid);
+			if (!(oldRev instanceof RevCommit) || !(newRev instanceof RevCommit)
+					|| !walker.isMergedInto((RevCommit) oldRev,
+							(RevCommit) newRev)) {
+				return false;
+			}
+		} catch (MissingObjectException x) {
+			return false;
+		} catch (Exception x) {
+			throw new TransportException(transport.getURI(),
+					MessageFormat.format(JGitText
+							.get().readingObjectsFromLocalRepositoryFailed,
+							x.getMessage()),
+					x);
+		}
+		return true;
+	}
+
+	/**
+	 * Expands all placeholder {@link RemoteRefUpdate}s for "matching"
+	 * {@link RefSpec}s ":" in {@link #toPush} and returns the resulting map in
+	 * which the placeholders have been replaced by their expansion.
+	 *
+	 * @return a new map of {@link RemoteRefUpdate}s keyed by remote name
+	 * @throws TransportException
+	 *             if the expansion results in duplicate updates
+	 */
+	private Map<String, RemoteRefUpdate> expandMatching()
+			throws TransportException {
+		Map<String, RemoteRefUpdate> result = new LinkedHashMap<>();
+		boolean hadMatch = false;
+		for (RemoteRefUpdate update : toPush.values()) {
+			if (update.isMatching()) {
+				if (hadMatch) {
+					throw new TransportException(MessageFormat.format(
+							JGitText.get().duplicateRemoteRefUpdateIsIllegal,
+							":")); //$NON-NLS-1$
+				}
+				expandMatching(result, update);
+				hadMatch = true;
+			} else if (result.put(update.getRemoteName(), update) != null) {
+				throw new TransportException(MessageFormat.format(
+						JGitText.get().duplicateRemoteRefUpdateIsIllegal,
+						update.getRemoteName()));
+			}
+		}
+		return result;
+	}
+
+	/**
+	 * Expands the placeholder {@link RemoteRefUpdate} {@code match} for a
+	 * "matching" {@link RefSpec} ":" or "+:" and puts the expansion into the
+	 * given map {@code updates}.
+	 *
+	 * @param updates
+	 *            map to put the expansion in
+	 * @param match
+	 *            the placeholder {@link RemoteRefUpdate} to expand
+	 *
+	 * @throws TransportException
+	 *             if the expansion results in duplicate updates, or the local
+	 *             branches cannot be determined
+	 */
+	private void expandMatching(Map<String, RemoteRefUpdate> updates,
+			RemoteRefUpdate match) throws TransportException {
+		try {
+			Map<String, Ref> advertisement = connection.getRefsMap();
+			Collection<RefSpec> fetchSpecs = match.getFetchSpecs();
+			boolean forceUpdate = match.isForceUpdate();
+			for (Ref local : transport.local.getRefDatabase()
+					.getRefsByPrefix(Constants.R_HEADS)) {
+				if (local.isSymbolic()) {
+					continue;
+				}
+				String name = local.getName();
+				Ref advertised = advertisement.get(name);
+				if (advertised == null || advertised.isSymbolic()) {
+					continue;
+				}
+				ObjectId oldOid = advertised.getObjectId();
+				if (oldOid == null || ObjectId.zeroId().equals(oldOid)) {
+					continue;
+				}
+				ObjectId newOid = local.getObjectId();
+				if (newOid == null || ObjectId.zeroId().equals(newOid)) {
+					continue;
+				}
+
+				RemoteRefUpdate rru = new RemoteRefUpdate(transport.local, name,
+						newOid, name, forceUpdate,
+						Transport.findTrackingRefName(name, fetchSpecs),
+						oldOid);
+				if (updates.put(rru.getRemoteName(), rru) != null) {
+					throw new TransportException(MessageFormat.format(
+							JGitText.get().duplicateRemoteRefUpdateIsIllegal,
+							rru.getRemoteName()));
+				}
+			}
+		} catch (IOException x) {
+			throw new TransportException(transport.getURI(),
+					MessageFormat.format(JGitText
+							.get().readingObjectsFromLocalRepositoryFailed,
+							x.getMessage()),
+					x);
+		}
+	}
+
 	private Map<String, RemoteRefUpdate> rejectAll() {
 		for (RemoteRefUpdate rru : toPush.values()) {
 			if (rru.getStatus() == Status.NOT_ATTEMPTED) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java
index ac357afdae..51fda84f33 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java
@@ -12,11 +12,11 @@ package org.eclipse.jgit.transport;
 
 import java.io.Serializable;
 import java.text.MessageFormat;
+import java.util.Objects;
 
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.util.References;
 
 /**
  * Describes how refs in one repository copy into another repository.
@@ -50,6 +50,9 @@ public class RefSpec implements Serializable {
 	/** Is this specification actually a wildcard match? */
 	private boolean wildcard;
 
+	/** Is this the special ":" RefSpec? */
+	private boolean matching;
+
 	/**
 	 * How strict to be about wildcards.
 	 *
@@ -71,6 +74,7 @@ public class RefSpec implements Serializable {
 		 */
 		ALLOW_MISMATCH
 	}
+
 	/** Whether a wildcard is allowed on one side but not the other. */
 	private WildcardMode allowMismatchedWildcards;
 
@@ -87,6 +91,7 @@ public class RefSpec implements Serializable {
 	 * applications, as at least one field must be set to match a source name.
 	 */
 	public RefSpec() {
+		matching = false;
 		force = false;
 		wildcard = false;
 		srcName = Constants.HEAD;
@@ -133,17 +138,25 @@ public class RefSpec implements Serializable {
 			s = s.substring(1);
 		}
 
+		boolean matchPushSpec = false;
 		final int c = s.lastIndexOf(':');
 		if (c == 0) {
 			s = s.substring(1);
-			if (isWildcard(s)) {
+			if (s.isEmpty()) {
+				matchPushSpec = true;
 				wildcard = true;
-				if (mode == WildcardMode.REQUIRE_MATCH) {
-					throw new IllegalArgumentException(MessageFormat
-							.format(JGitText.get().invalidWildcards, spec));
+				srcName = Constants.R_HEADS + '*';
+				dstName = srcName;
+			} else {
+				if (isWildcard(s)) {
+					wildcard = true;
+					if (mode == WildcardMode.REQUIRE_MATCH) {
+						throw new IllegalArgumentException(MessageFormat
+								.format(JGitText.get().invalidWildcards, spec));
+					}
 				}
+				dstName = checkValid(s);
 			}
-			dstName = checkValid(s);
 		} else if (c > 0) {
 			String src = s.substring(0, c);
 			String dst = s.substring(c + 1);
@@ -168,6 +181,7 @@ public class RefSpec implements Serializable {
 			}
 			srcName = checkValid(s);
 		}
+		matching = matchPushSpec;
 	}
 
 	/**
@@ -195,6 +209,7 @@ public class RefSpec implements Serializable {
 	}
 
 	private RefSpec(RefSpec p) {
+		matching = false;
 		force = p.isForceUpdate();
 		wildcard = p.isWildcard();
 		srcName = p.getSource();
@@ -203,6 +218,17 @@ public class RefSpec implements Serializable {
 	}
 
 	/**
+	 * Tells whether this {@link RefSpec} is the special "matching" RefSpec ":"
+	 * for pushing.
+	 *
+	 * @return whether this is a "matching" RefSpec
+	 * @since 6.1
+	 */
+	public boolean isMatching() {
+		return matching;
+	}
+
+	/**
 	 * Check if this specification wants to forcefully update the destination.
 	 *
 	 * @return true if this specification asks for updates without merge tests.
@@ -220,6 +246,7 @@ public class RefSpec implements Serializable {
 	 */
 	public RefSpec setForceUpdate(boolean forceUpdate) {
 		final RefSpec r = new RefSpec(this);
+		r.matching = matching;
 		r.force = forceUpdate;
 		return r;
 	}
@@ -541,37 +568,36 @@ public class RefSpec implements Serializable {
 		if (!(obj instanceof RefSpec))
 			return false;
 		final RefSpec b = (RefSpec) obj;
-		if (isForceUpdate() != b.isForceUpdate())
-			return false;
-		if (isWildcard() != b.isWildcard())
+		if (isForceUpdate() != b.isForceUpdate()) {
 			return false;
-		if (!eq(getSource(), b.getSource()))
-			return false;
-		if (!eq(getDestination(), b.getDestination()))
-			return false;
-		return true;
-	}
-
-	private static boolean eq(String a, String b) {
-		if (References.isSameObject(a, b)) {
-			return true;
 		}
-		if (a == null || b == null)
+		if (isMatching()) {
+			return b.isMatching();
+		} else if (b.isMatching()) {
 			return false;
-		return a.equals(b);
+		}
+		return isWildcard() == b.isWildcard()
+				&& Objects.equals(getSource(), b.getSource())
+				&& Objects.equals(getDestination(), b.getDestination());
 	}
 
 	/** {@inheritDoc} */
 	@Override
 	public String toString() {
 		final StringBuilder r = new StringBuilder();
-		if (isForceUpdate())
+		if (isForceUpdate()) {
 			r.append('+');
-		if (getSource() != null)
-			r.append(getSource());
-		if (getDestination() != null) {
+		}
+		if (isMatching()) {
 			r.append(':');
-			r.append(getDestination());
+		} else {
+			if (getSource() != null) {
+				r.append(getSource());
+			}
+			if (getDestination() != null) {
+				r.append(':');
+				r.append(getDestination());
+			}
 		}
 		return r.toString();
 	}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteRefUpdate.java
index 43eaac7927..b90ae813e4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteRefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteRefUpdate.java
@@ -12,7 +12,9 @@ package org.eclipse.jgit.transport;
 
 import java.io.IOException;
 import java.text.MessageFormat;
+import java.util.Collection;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
@@ -116,6 +118,12 @@ public class RemoteRefUpdate {
 	private RefUpdate localUpdate;
 
 	/**
+	 * If set, the RemoteRefUpdate is a placeholder for the "matching" RefSpec
+	 * to be expanded after the advertisements have been received in a push.
+	 */
+	private Collection<RefSpec> fetchSpecs;
+
+	/**
 	 * Construct remote ref update request by providing an update specification.
 	 * Object is created with default
 	 * {@link org.eclipse.jgit.transport.RemoteRefUpdate.Status#NOT_ATTEMPTED}
@@ -259,24 +267,38 @@ public class RemoteRefUpdate {
 			final ObjectId srcId, final String remoteName,
 			final boolean forceUpdate, final String localName,
 			final ObjectId expectedOldObjectId) throws IOException {
-		if (remoteName == null)
-			throw new IllegalArgumentException(JGitText.get().remoteNameCannotBeNull);
-		if (srcId == null && srcRef != null)
-			throw new IOException(MessageFormat.format(
-					JGitText.get().sourceRefDoesntResolveToAnyObject, srcRef));
+		this(localDb, srcRef, srcId, remoteName, forceUpdate, localName, null,
+				expectedOldObjectId);
+	}
 
-		if (srcRef != null)
+	private RemoteRefUpdate(Repository localDb, String srcRef, ObjectId srcId,
+			String remoteName, boolean forceUpdate, String localName,
+			Collection<RefSpec> fetchSpecs, ObjectId expectedOldObjectId)
+			throws IOException {
+		if (fetchSpecs == null) {
+			if (remoteName == null) {
+				throw new IllegalArgumentException(
+						JGitText.get().remoteNameCannotBeNull);
+			}
+			if (srcId == null && srcRef != null) {
+				throw new IOException(MessageFormat.format(
+						JGitText.get().sourceRefDoesntResolveToAnyObject,
+						srcRef));
+			}
+		}
+		if (srcRef != null) {
 			this.srcRef = srcRef;
-		else if (srcId != null && !srcId.equals(ObjectId.zeroId()))
+		} else if (srcId != null && !srcId.equals(ObjectId.zeroId())) {
 			this.srcRef = srcId.name();
-		else
+		} else {
 			this.srcRef = null;
-
-		if (srcId != null)
+		}
+		if (srcId != null) {
 			this.newObjectId = srcId;
-		else
+		} else {
 			this.newObjectId = ObjectId.zeroId();
-
+		}
+		this.fetchSpecs = fetchSpecs;
 		this.remoteName = remoteName;
 		this.forceUpdate = forceUpdate;
 		if (localName != null && localDb != null) {
@@ -292,8 +314,9 @@ public class RemoteRefUpdate {
 						? localUpdate.getOldObjectId()
 						: ObjectId.zeroId(),
 					newObjectId);
-		} else
+		} else {
 			trackingRefUpdate = null;
+		}
 		this.localDb = localDb;
 		this.expectedOldObjectId = expectedOldObjectId;
 		this.status = Status.NOT_ATTEMPTED;
@@ -318,9 +341,55 @@ public class RemoteRefUpdate {
 	 */
 	public RemoteRefUpdate(final RemoteRefUpdate base,
 			final ObjectId newExpectedOldObjectId) throws IOException {
-		this(base.localDb, base.srcRef, base.remoteName, base.forceUpdate,
+		this(base.localDb, base.srcRef, base.newObjectId, base.remoteName,
+				base.forceUpdate,
 				(base.trackingRefUpdate == null ? null : base.trackingRefUpdate
-						.getLocalName()), newExpectedOldObjectId);
+						.getLocalName()),
+				base.fetchSpecs, newExpectedOldObjectId);
+	}
+
+	/**
+	 * Creates a "placeholder" update for the "matching" RefSpec ":".
+	 *
+	 * @param localDb
+	 *            local repository to push from
+	 * @param forceUpdate
+	 *            whether non-fast-forward updates shall be allowed
+	 * @param fetchSpecs
+	 *            The fetch {@link RefSpec}s to use when this placeholder is
+	 *            expanded to determine remote tracking branch updates
+	 */
+	RemoteRefUpdate(Repository localDb, boolean forceUpdate,
+			@NonNull Collection<RefSpec> fetchSpecs) {
+		this.localDb = localDb;
+		this.forceUpdate = forceUpdate;
+		this.fetchSpecs = fetchSpecs;
+		this.trackingRefUpdate = null;
+		this.srcRef = null;
+		this.remoteName = null;
+		this.newObjectId = null;
+		this.status = Status.NOT_ATTEMPTED;
+	}
+
+	/**
+	 * Tells whether this {@link RemoteRefUpdate} is a placeholder for a
+	 * "matching" {@link RefSpec}.
+	 *
+	 * @return {@code true} if this is a placeholder, {@code false} otherwise
+	 * @since 6.1
+	 */
+	public boolean isMatching() {
+		return fetchSpecs != null;
+	}
+
+	/**
+	 * Retrieves the fetch {@link RefSpec}s of this {@link RemoteRefUpdate}.
+	 *
+	 * @return the fetch {@link RefSpec}s, or {@code null} if
+	 *         {@code this.}{@link #isMatching()} {@code == false}
+	 */
+	Collection<RefSpec> getFetchSpecs() {
+		return fetchSpecs;
 	}
 
 	/**
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 1245f78a6e..0eab4434ed 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/Transport.java
@@ -589,6 +589,11 @@ public abstract class Transport implements AutoCloseable {
 		final Collection<RefSpec> procRefs = expandPushWildcardsFor(db, specs);
 
 		for (RefSpec spec : procRefs) {
+			if (spec.isMatching()) {
+				result.add(new RemoteRefUpdate(db, spec.isForceUpdate(),
+						fetchSpecs));
+				continue;
+			}
 			String srcSpec = spec.getSource();
 			final Ref srcRef = db.findRef(srcSpec);
 			if (srcRef != null)
@@ -659,7 +664,7 @@ public abstract class Transport implements AutoCloseable {
 
 		List<Ref> localRefs = null;
 		for (RefSpec spec : specs) {
-			if (spec.isWildcard()) {
+			if (!spec.isMatching() && spec.isWildcard()) {
 				if (localRefs == null) {
 					localRefs = db.getRefDatabase().getRefs();
 				}
@@ -675,7 +680,7 @@ public abstract class Transport implements AutoCloseable {
 		return procRefs;
 	}
 
-	private static String findTrackingRefName(final String remoteName,
+	static String findTrackingRefName(final String remoteName,
 			final Collection<RefSpec> fetchSpecs) {
 		// try to find matching tracking refs
 		for (RefSpec fetchSpec : fetchSpecs) {