[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>
This commit is contained in:
Thomas Wolf 2022-02-20 18:11:49 +01:00 committed by Matthias Sohn
parent 90df7c123e
commit 8a2c769417
7 changed files with 342 additions and 75 deletions

View File

@ -10,6 +10,7 @@
package org.eclipse.jgit.api;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
@ -392,28 +393,64 @@ public void testPushDefaultMatching() throws Exception {
git.add().addFilepattern("f").call();
RevCommit commit = git.commit().setMessage("adding f").call();
git.checkout().setName("also-pushed").setCreateBranch(true).call();
git.checkout().setName("not-pushed").setCreateBranch(true).call();
git.checkout().setName("branchtopush").setCreateBranch(true).call();
assertEquals(null,
git2.getRepository().resolve("refs/heads/branchtopush"));
assertEquals(null,
git2.getRepository().resolve("refs/heads/also-pushed"));
git2.getRepository().resolve("refs/heads/not-pushed"));
assertEquals(null,
git2.getRepository().resolve("refs/heads/master"));
git.push().setRemote("test").setPushDefault(PushDefault.MATCHING)
// push master and branchtopush
git.push().setRemote("test").setRefSpecs(
new RefSpec("refs/heads/master:refs/heads/master"),
new RefSpec(
"refs/heads/branchtopush:refs/heads/branchtopush"))
.call();
assertEquals(commit.getId(),
git2.getRepository().resolve("refs/heads/branchtopush"));
assertEquals(commit.getId(),
git2.getRepository().resolve("refs/heads/also-pushed"));
assertEquals(commit.getId(),
git2.getRepository().resolve("refs/heads/master"));
assertEquals(commit.getId(), git.getRepository()
.resolve("refs/remotes/origin/branchtopush"));
assertEquals(commit.getId(), git.getRepository()
.resolve("refs/remotes/origin/also-pushed"));
assertEquals(commit.getId(),
git2.getRepository().resolve("refs/heads/branchtopush"));
assertEquals(null,
git2.getRepository().resolve("refs/heads/not-pushed"));
// Create two different commits on these two branches
writeTrashFile("b", "on branchtopush");
git.add().addFilepattern("b").call();
RevCommit bCommit = git.commit().setMessage("on branchtopush")
.call();
git.checkout().setName("master").call();
writeTrashFile("m", "on master");
git.add().addFilepattern("m").call();
RevCommit mCommit = git.commit().setMessage("on master").call();
// Now push with mode "matching": should push both branches.
Iterable<PushResult> result = git.push().setRemote("test")
.setPushDefault(PushDefault.MATCHING)
.call();
int n = 0;
for (PushResult r : result) {
n++;
assertEquals(1, n);
assertEquals(2, r.getRemoteUpdates().size());
for (RemoteRefUpdate update : r.getRemoteUpdates()) {
assertFalse(update.isMatching());
assertTrue(update.getSrcRef()
.equals("refs/heads/branchtopush")
|| update.getSrcRef().equals("refs/heads/master"));
assertEquals(RemoteRefUpdate.Status.OK, update.getStatus());
}
}
assertEquals(bCommit.getId(),
git2.getRepository().resolve("refs/heads/branchtopush"));
assertEquals(null,
git2.getRepository().resolve("refs/heads/not-pushed"));
assertEquals(mCommit.getId(),
git2.getRepository().resolve("refs/heads/master"));
assertEquals(bCommit.getId(), git.getRepository()
.resolve("refs/remotes/origin/branchtopush"));
assertEquals(null, git.getRepository()
.resolve("refs/remotes/origin/not-pushed"));
assertEquals(mCommit.getId(),
git.getRepository().resolve("refs/remotes/origin/master"));
}
}

View File

@ -466,4 +466,18 @@ public void onlyWildCard() {
assertTrue(a.matchSource("refs/heads/master"));
assertNull(a.getDestination());
}
@Test
public void matching() {
RefSpec a = new RefSpec(":");
assertTrue(a.isMatching());
assertFalse(a.isForceUpdate());
}
@Test
public void matchingForced() {
RefSpec a = new RefSpec("+:");
assertTrue(a.isMatching());
assertTrue(a.isForceUpdate());
}
}

View File

@ -230,7 +230,7 @@ private void determineDefaultRefSpecs(Config config)
refSpecs.add(new RefSpec(getCurrentBranch()));
break;
case MATCHING:
setPushAll();
refSpecs.add(new RefSpec(":")); //$NON-NLS-1$
break;
case NOTHING:
throw new InvalidRefNameException(

View File

@ -26,6 +26,7 @@
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 @@ PushResult execute(ProgressMonitor monitor)
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 @@ private Map<String, RemoteRefUpdate> prepareRemoteUpdates()
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 @@ private Map<String, RemoteRefUpdate> prepareRemoteUpdates()
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) {

View File

@ -12,11 +12,11 @@
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 static boolean isWildcard(String s) {
/** 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 enum WildcardMode {
*/
ALLOW_MISMATCH
}
/** Whether a wildcard is allowed on one side but not the other. */
private WildcardMode allowMismatchedWildcards;
@ -87,6 +91,7 @@ public enum WildcardMode {
* 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 RefSpec(String spec, WildcardMode mode) {
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 RefSpec(String spec, WildcardMode mode) {
}
srcName = checkValid(s);
}
matching = matchPushSpec;
}
/**
@ -195,6 +209,7 @@ public RefSpec(String spec) {
}
private RefSpec(RefSpec p) {
matching = false;
force = p.isForceUpdate();
wildcard = p.isWildcard();
srcName = p.getSource();
@ -202,6 +217,17 @@ private RefSpec(RefSpec p) {
allowMismatchedWildcards = p.allowMismatchedWildcards;
}
/**
* 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.
*
@ -220,6 +246,7 @@ public boolean isForceUpdate() {
*/
public RefSpec setForceUpdate(boolean forceUpdate) {
final RefSpec r = new RefSpec(this);
r.matching = matching;
r.force = forceUpdate;
return r;
}
@ -541,37 +568,36 @@ public boolean equals(Object obj) {
if (!(obj instanceof RefSpec))
return false;
final RefSpec b = (RefSpec) obj;
if (isForceUpdate() != b.isForceUpdate())
if (isForceUpdate() != b.isForceUpdate()) {
return false;
if (isWildcard() != b.isWildcard())
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();
}

View File

@ -12,7 +12,9 @@
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;
@ -115,6 +117,12 @@ public enum Status {
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
@ -259,24 +267,38 @@ public RemoteRefUpdate(final Repository localDb, final String srcRef,
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 @@ else if (srcId != null && !srcId.equals(ObjectId.zeroId()))
? localUpdate.getOldObjectId()
: ObjectId.zeroId(),
newObjectId);
} else
} else {
trackingRefUpdate = null;
}
this.localDb = localDb;
this.expectedOldObjectId = expectedOldObjectId;
this.status = Status.NOT_ATTEMPTED;
@ -318,9 +341,55 @@ else if (srcId != null && !srcId.equals(ObjectId.zeroId()))
*/
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;
}
/**

View File

@ -589,6 +589,11 @@ public static Collection<RemoteRefUpdate> findRemoteRefUpdatesFor(
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 @@ private static Collection<RefSpec> expandPushWildcardsFor(
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 @@ private static Collection<RefSpec> expandPushWildcardsFor(
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) {