From 7f59cfe14354a19a38ccc5920c2ce5e49de9ae0d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sat, 12 Aug 2017 08:25:16 -0700 Subject: [PATCH] Support symbolic references in ReceiveCommand Allow creating symbolic references with link, and deleting them or switching to ObjectId with unlink. How this happens is up to the individual RefDatabase. The default implementation detaches RefUpdate if a symbolic reference is involved, supporting these command instances on RefDirectory. Unfortunately the packed-refs file does not support storing symrefs, so atomic transactions involving more than one symref command are failed early. Updating InMemoryRepository is deferred until reftable lands, as I plan to switch InMemoryRepository to use reftable for its internal storage representation. Change-Id: Ibcae068b17a2fc6d958f767f402a570ad88d9151 Signed-off-by: Minh Thai Signed-off-by: Terry Parker --- .../storage/reftree/RefTreeDatabaseTest.java | 3 +- .../eclipse/jgit/internal/JGitText.properties | 3 + .../org/eclipse/jgit/internal/JGitText.java | 3 + .../storage/file/PackedBatchRefUpdate.java | 23 +- .../internal/storage/reftree/Command.java | 17 +- .../jgit/transport/ReceiveCommand.java | 241 ++++++++++++++++-- 6 files changed, 268 insertions(+), 22 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/RefTreeDatabaseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/RefTreeDatabaseTest.java index 9aef94369..1684afa4e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/RefTreeDatabaseTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/RefTreeDatabaseTest.java @@ -657,7 +657,8 @@ public boolean apply(ObjectReader reader, RefTree tree) Ref old = tree.exactRef(reader, name); Command n; try (RevWalk rw = new RevWalk(repo)) { - n = new Command(old, Command.toRef(rw, id, name, true)); + n = new Command(old, + Command.toRef(rw, id, null, name, true)); } return tree.apply(Collections.singleton(n)); } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 43dd9482d..f586aee93 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -22,6 +22,7 @@ atLeastOnePatternIsRequired=At least one pattern is required. atLeastTwoFiltersNeeded=At least two filters needed. atomicPushNotSupported=Atomic push not supported. atomicRefUpdatesNotSupported=Atomic ref updates not supported +atomicSymRefNotSupported=Atomic symref not supported authenticationNotSupported=authentication not supported badBase64InputCharacterAt=Bad Base64 input character at {0} : {1} (decimal) badEntryDelimiter=Bad entry delimiter @@ -41,6 +42,7 @@ blameNotCommittedYet=Not Committed Yet blobNotFound=Blob not found: {0} blobNotFoundForPath=Blob not found: {0} for path: {1} blockSizeNotPowerOf2=blockSize must be a power of 2 +bothRefTargetsMustNotBeNull=both old and new ref targets must not be null. branchNameInvalid=Branch name {0} is not allowed buildingBitmaps=Building bitmaps cachedPacksPreventsIndexCreation=Using cached packs prevents index creation @@ -434,6 +436,7 @@ month=month months=months monthsAgo={0} months ago multipleMergeBasesFor=Multiple merge bases for:\n {0}\n {1} found:\n {2}\n {3} +nameMustNotBeNullOrEmpty=Ref name must not be null or empty. need2Arguments=Need 2 arguments needPackOut=need packOut needsAtLeastOneEntry=Needs at least one entry diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 6b3631601..c41fc51bd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -81,6 +81,7 @@ public static JGitText get() { /***/ public String atLeastTwoFiltersNeeded; /***/ public String atomicPushNotSupported; /***/ public String atomicRefUpdatesNotSupported; + /***/ public String atomicSymRefNotSupported; /***/ public String authenticationNotSupported; /***/ public String badBase64InputCharacterAt; /***/ public String badEntryDelimiter; @@ -100,6 +101,7 @@ public static JGitText get() { /***/ public String blobNotFound; /***/ public String blobNotFoundForPath; /***/ public String blockSizeNotPowerOf2; + /***/ public String bothRefTargetsMustNotBeNull; /***/ public String branchNameInvalid; /***/ public String buildingBitmaps; /***/ public String cachedPacksPreventsIndexCreation; @@ -493,6 +495,7 @@ public static JGitText get() { /***/ public String months; /***/ public String monthsAgo; /***/ public String multipleMergeBasesFor; + /***/ public String nameMustNotBeNullOrEmpty; /***/ public String need2Arguments; /***/ public String needPackOut; /***/ public String needsAtLeastOneEntry; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java index b328eb83e..ad2500059 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java @@ -47,6 +47,7 @@ import static org.eclipse.jgit.transport.ReceiveCommand.Result.LOCK_FAILURE; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_NONFASTFORWARD; +import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON; import java.io.IOException; import java.text.MessageFormat; @@ -142,6 +143,12 @@ public void execute(RevWalk walk, ProgressMonitor monitor, super.execute(walk, monitor, options); return; } + if (containsSymrefs(pending)) { + // packed-refs file cannot store symrefs + reject(pending.get(0), REJECTED_OTHER_REASON, + JGitText.get().atomicSymRefNotSupported, pending); + return; + } // Required implementation details copied from super.execute. if (!blockUntilTimestamps(MAX_WAIT)) { @@ -209,6 +216,15 @@ public void execute(RevWalk walk, ProgressMonitor monitor, writeReflog(pending); } + private static boolean containsSymrefs(List commands) { + for (ReceiveCommand cmd : commands) { + if (cmd.getOldSymref() != null || cmd.getNewSymref() != null) { + return true; + } + } + return false; + } + private boolean checkConflictingNames(List commands) throws IOException { Set takenNames = new HashSet<>(); @@ -510,7 +526,12 @@ private static void lockFailure(ReceiveCommand cmd, private static void reject(ReceiveCommand cmd, ReceiveCommand.Result result, List commands) { - cmd.setResult(result); + reject(cmd, result, null, commands); + } + + private static void reject(ReceiveCommand cmd, ReceiveCommand.Result result, + String why, List commands) { + cmd.setResult(result, why); for (ReceiveCommand c2 : commands) { if (c2.getResult() == ReceiveCommand.Result.OK) { // Undo OK status so ReceiveCommand#abort aborts it. Assumes this method diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java index dd08375f2..92cfe3d89 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java @@ -61,6 +61,7 @@ import org.eclipse.jgit.lib.ObjectIdRef; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.SymbolicRef; import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevTag; import org.eclipse.jgit.revwalk.RevWalk; @@ -153,14 +154,20 @@ public Command(@Nullable Ref oldRef, @Nullable Ref newRef) { */ public Command(RevWalk rw, ReceiveCommand cmd) throws MissingObjectException, IOException { - this.oldRef = toRef(rw, cmd.getOldId(), cmd.getRefName(), false); - this.newRef = toRef(rw, cmd.getNewId(), cmd.getRefName(), true); + this.oldRef = toRef(rw, cmd.getOldId(), cmd.getOldSymref(), + cmd.getRefName(), false); + this.newRef = toRef(rw, cmd.getNewId(), cmd.getNewSymref(), + cmd.getRefName(), true); this.cmd = cmd; } - static Ref toRef(RevWalk rw, ObjectId id, String name, - boolean mustExist) throws MissingObjectException, IOException { - if (ObjectId.zeroId().equals(id)) { + static Ref toRef(RevWalk rw, ObjectId id, @Nullable String target, + String name, boolean mustExist) + throws MissingObjectException, IOException { + if (target != null) { + return new SymbolicRef(name, + new ObjectIdRef.Unpeeled(NETWORK, target, id)); + } else if (ObjectId.zeroId().equals(id)) { return null; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java index e9681b34c..374df6a67 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java @@ -52,6 +52,7 @@ import java.util.Collection; import java.util.List; +import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.AnyObjectId; @@ -196,8 +197,8 @@ public static void abort(Iterable commands) { * * @param cmd * command. - * @return whether the command failed due to transaction aborted, as in {@link - * #abort(Iterable)}. + * @return whether the command failed due to transaction aborted, as in + * {@link #abort(Iterable)}. * @since 4.9 */ public static boolean isTransactionAborted(ReceiveCommand cmd) { @@ -205,14 +206,71 @@ public static boolean isTransactionAborted(ReceiveCommand cmd) { && cmd.getMessage().equals(JGitText.get().transactionAborted); } + /** + * Create a command to switch a reference from object to symbolic. + * + * @param oldId + * expected oldId. May be {@code zeroId} to create. + * @param newTarget + * new target; must begin with {@code "refs/"}. + * @param name + * name of the reference to make symbolic. + * @return command instance. + * @since 4.10 + */ + public static ReceiveCommand link(@NonNull ObjectId oldId, + @NonNull String newTarget, @NonNull String name) { + return new ReceiveCommand(oldId, newTarget, name); + } + + /** + * Create a command to switch a symbolic reference's target. + * + * @param oldTarget + * expected old target. May be null to create. + * @param newTarget + * new target; must begin with {@code "refs/"}. + * @param name + * name of the reference to make symbolic. + * @return command instance. + * @since 4.10 + */ + public static ReceiveCommand link(@Nullable String oldTarget, + @NonNull String newTarget, @NonNull String name) { + return new ReceiveCommand(oldTarget, newTarget, name); + } + + /** + * Create a command to switch a reference from symbolic to object. + * + * @param oldTarget + * expected old target. + * @param newId + * new object identifier. May be {@code zeroId()} to delete. + * @param name + * name of the reference to convert from symbolic. + * @return command instance. + * @since 4.10 + */ + public static ReceiveCommand unlink(@NonNull String oldTarget, + @NonNull ObjectId newId, @NonNull String name) { + return new ReceiveCommand(oldTarget, newId, name); + } + private final ObjectId oldId; + private final String oldSymref; + private final ObjectId newId; + private final String newSymref; + private final String name; private Type type; + private boolean typeIsCorrect; + private Ref ref; private Result status = Result.NOT_ATTEMPTED; @@ -227,8 +285,6 @@ public static boolean isTransactionAborted(ReceiveCommand cmd) { private Boolean forceRefLog; - private boolean typeIsCorrect; - /** * Create a new command for {@link BaseReceivePack}. * @@ -244,13 +300,21 @@ public static boolean isTransactionAborted(ReceiveCommand cmd) { public ReceiveCommand(final ObjectId oldId, final ObjectId newId, final String name) { if (oldId == null) { - throw new IllegalArgumentException(JGitText.get().oldIdMustNotBeNull); + throw new IllegalArgumentException( + JGitText.get().oldIdMustNotBeNull); } if (newId == null) { - throw new IllegalArgumentException(JGitText.get().newIdMustNotBeNull); + throw new IllegalArgumentException( + JGitText.get().newIdMustNotBeNull); + } + if (name == null || name.isEmpty()) { + throw new IllegalArgumentException( + JGitText.get().nameMustNotBeNullOrEmpty); } this.oldId = oldId; + this.oldSymref = null; this.newId = newId; + this.newSymref = null; this.name = name; type = Type.UPDATE; @@ -275,19 +339,28 @@ public ReceiveCommand(final ObjectId oldId, final ObjectId newId, * name of the ref being affected. * @param type * type of the command. Must be {@link Type#CREATE} if {@code - * oldId} is zero, or {@link Type#DELETE} if {@code newId} is zero. + * oldId} is zero, or {@link Type#DELETE} if {@code newId} is + * zero. * @since 2.0 */ public ReceiveCommand(final ObjectId oldId, final ObjectId newId, final String name, final Type type) { if (oldId == null) { - throw new IllegalArgumentException(JGitText.get().oldIdMustNotBeNull); + throw new IllegalArgumentException( + JGitText.get().oldIdMustNotBeNull); } if (newId == null) { - throw new IllegalArgumentException(JGitText.get().newIdMustNotBeNull); + throw new IllegalArgumentException( + JGitText.get().newIdMustNotBeNull); + } + if (name == null || name.isEmpty()) { + throw new IllegalArgumentException( + JGitText.get().nameMustNotBeNullOrEmpty); } this.oldId = oldId; + this.oldSymref = null; this.newId = newId; + this.newSymref = null; this.name = name; switch (type) { case CREATE: @@ -311,21 +384,144 @@ public ReceiveCommand(final ObjectId oldId, final ObjectId newId, } break; default: - throw new IllegalStateException(JGitText.get().enumValueNotSupported0); + throw new IllegalStateException( + JGitText.get().enumValueNotSupported0); } this.type = type; } + /** + * Create a command to switch a reference from object to symbolic. + * + * @param oldId + * the old object id; must not be null. Use + * {@link ObjectId#zeroId()} to indicate a ref creation. + * @param newSymref + * new target, must begin with {@code "refs/"}. Use {@code null} + * to indicate a ref deletion. + * @param name + * name of the reference to make symbolic. + * @since 4.10 + */ + private ReceiveCommand(ObjectId oldId, String newSymref, String name) { + if (oldId == null) { + throw new IllegalArgumentException( + JGitText.get().oldIdMustNotBeNull); + } + if (name == null || name.isEmpty()) { + throw new IllegalArgumentException( + JGitText.get().nameMustNotBeNullOrEmpty); + } + this.oldId = oldId; + this.oldSymref = null; + this.newId = ObjectId.zeroId(); + this.newSymref = newSymref; + this.name = name; + if (AnyObjectId.equals(ObjectId.zeroId(), oldId)) { + type = Type.CREATE; + } else if (newSymref != null) { + type = Type.UPDATE; + } else { + type = Type.DELETE; + } + typeIsCorrect = true; + } + + /** + * Create a command to switch a reference from symbolic to object. + * + * @param oldSymref + * expected old target. Use {@code null} to indicate a ref + * creation. + * @param newId + * the new object id; must not be null. Use + * {@link ObjectId#zeroId()} to indicate a ref deletion. + * @param name + * name of the reference to convert from symbolic. + * @since 4.10 + */ + private ReceiveCommand(String oldSymref, ObjectId newId, String name) { + if (newId == null) { + throw new IllegalArgumentException( + JGitText.get().newIdMustNotBeNull); + } + if (name == null || name.isEmpty()) { + throw new IllegalArgumentException( + JGitText.get().nameMustNotBeNullOrEmpty); + } + this.oldId = ObjectId.zeroId(); + this.oldSymref = oldSymref; + this.newId = newId; + this.newSymref = null; + this.name = name; + if (oldSymref == null) { + type = Type.CREATE; + } else if (!AnyObjectId.equals(ObjectId.zeroId(), newId)) { + type = Type.UPDATE; + } else { + type = Type.DELETE; + } + typeIsCorrect = true; + } + + /** + * Create a command to switch a symbolic reference's target. + * + * @param oldTarget + * expected old target. Use {@code null} to indicate a ref + * creation. + * @param newTarget + * new target. Use {@code null} to indicate a ref deletion. + * @param name + * name of the reference to make symbolic. + * @since 4.10 + */ + private ReceiveCommand(@Nullable String oldTarget, String newTarget, String name) { + if (name == null || name.isEmpty()) { + throw new IllegalArgumentException( + JGitText.get().nameMustNotBeNullOrEmpty); + } + this.oldId = ObjectId.zeroId(); + this.oldSymref = oldTarget; + this.newId = ObjectId.zeroId(); + this.newSymref = newTarget; + this.name = name; + if (oldTarget == null) { + if (newTarget == null) { + throw new IllegalArgumentException( + JGitText.get().bothRefTargetsMustNotBeNull); + } + type = Type.CREATE; + } else if (newTarget != null) { + type = Type.UPDATE; + } else { + type = Type.DELETE; + } + typeIsCorrect = true; + } + /** @return the old value the client thinks the ref has. */ public ObjectId getOldId() { return oldId; } + /** @return expected old target for a symbolic reference. */ + @Nullable + public String getOldSymref() { + return oldSymref; + } + /** @return the requested new value for this ref. */ public ObjectId getNewId() { return newId; } + /** @return requested new target for a symbolic reference. */ + @Nullable + public String getNewSymref() { + return newSymref; + } + /** @return the name of the ref being updated. */ public String getRefName() { return name; @@ -452,8 +648,8 @@ public boolean isRefLogIncludingResult() { /** * Check whether the reflog should be written regardless of repo defaults. * - * @return whether force writing is enabled; null if {@code - * #setForceRefLog(boolean)} was never called. + * @return whether force writing is enabled; {@code null} if + * {@code #setForceRefLog(boolean)} was never called. * @since 4.9 */ @Nullable @@ -525,7 +721,18 @@ public void updateType(RevWalk walk) throws IOException { */ public void execute(final BaseReceivePack rp) { try { - final RefUpdate ru = rp.getRepository().updateRef(getRefName()); + String expTarget = getOldSymref(); + boolean detach = getNewSymref() != null + || (type == Type.DELETE && expTarget != null); + RefUpdate ru = rp.getRepository().updateRef(getRefName(), detach); + if (expTarget != null) { + if (!ru.getRef().isSymbolic() || !ru.getRef().getTarget() + .getName().equals(expTarget)) { + setResult(Result.LOCK_FAILURE); + return; + } + } + ru.setRefLogIdent(rp.getRefLogIdent()); ru.setRefLogMessage(refLogMessage, refLogIncludeResult); switch (getType()) { @@ -546,9 +753,13 @@ public void execute(final BaseReceivePack rp) { case UPDATE_NONFASTFORWARD: ru.setForceUpdate(rp.isAllowNonFastForwards()); ru.setExpectedOldObjectId(getOldId()); - ru.setNewObjectId(getNewId()); ru.setRefLogMessage("push", true); //$NON-NLS-1$ - setResult(ru.update(rp.getRevWalk())); + if (getNewSymref() != null) { + setResult(ru.link(getNewSymref())); + } else { + ru.setNewObjectId(getNewId()); + setResult(ru.update(rp.getRevWalk())); + } break; } } catch (IOException err) {