diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RenameBranchCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RenameBranchCommandTest.java index 5c437ac5a..9facce9ac 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RenameBranchCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RenameBranchCommandTest.java @@ -49,14 +49,18 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import org.eclipse.jgit.api.errors.RefAlreadyExistsException; import org.eclipse.jgit.junit.RepositoryTestCase; import org.eclipse.jgit.lib.BranchConfig.BranchRebaseMode; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; /** * Unit tests of {@link RenameBranchCommand} @@ -69,6 +73,9 @@ public class RenameBranchCommandTest extends RepositoryTestCase { private Git git; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Override @Before public void setUp() throws Exception { @@ -80,6 +87,45 @@ public class RenameBranchCommandTest extends RepositoryTestCase { assertNotNull(head); } + @Test + public void renameToExisting() throws Exception { + assertNotNull(git.branchCreate().setName("foo").call()); + thrown.expect(RefAlreadyExistsException.class); + git.branchRename().setOldName("master").setNewName("foo").call(); + } + + @Test + public void renameToTag() throws Exception { + Ref ref = git.tag().setName("foo").call(); + assertNotNull(ref); + assertEquals("Unexpected tag name", Constants.R_TAGS + "foo", + ref.getName()); + ref = git.branchRename().setNewName("foo").call(); + assertNotNull(ref); + assertEquals("Unexpected ref name", Constants.R_HEADS + "foo", + ref.getName()); + // Check that we can rename it back + ref = git.branchRename().setOldName("foo").setNewName(Constants.MASTER) + .call(); + assertNotNull(ref); + assertEquals("Unexpected ref name", + Constants.R_HEADS + Constants.MASTER, ref.getName()); + } + + @Test + public void renameToStupidName() throws Exception { + Ref ref = git.branchRename().setNewName(Constants.R_HEADS + "foo") + .call(); + assertEquals("Unexpected ref name", + Constants.R_HEADS + Constants.R_HEADS + "foo", + ref.getName()); + // And check that we can rename it back to a sane name + ref = git.branchRename().setNewName("foo").call(); + assertNotNull(ref); + assertEquals("Unexpected ref name", Constants.R_HEADS + "foo", + ref.getName()); + } + @Test public void renameBranchNoConfigValues() throws Exception { StoredConfig config = git.getRepository().getConfig(); 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 6ee144ff6..1366031be 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -590,8 +590,9 @@ remoteConfigHasNoURIAssociated=Remote config "{0}" has no URIs associated remoteDoesNotHaveSpec=Remote does not have {0} available for fetch. remoteDoesNotSupportSmartHTTPPush=remote does not support smart HTTP push remoteHungUpUnexpectedly=remote hung up unexpectedly -remoteNameCantBeNull=Remote name can't be null. -renameBranchFailedBecauseTag=Can not rename as Ref {0} is a tag +remoteNameCannotBeNull=Remote name cannot be null. +renameBranchFailedAmbiguous=Cannot rename branch {0}; name is ambiguous: {1} or {2} +renameBranchFailedNotABranch=Cannot rename {0}: this is not a branch renameBranchFailedUnknownReason=Rename failed with unknown reason renameBranchUnexpectedResult=Unexpected rename result {0} renameCancelled=Rename detection was cancelled diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java index 24d9dd401..7e8c33c8a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java @@ -94,25 +94,38 @@ public class RenameBranchCommand extends GitCommand { RefAlreadyExistsException, DetachedHeadException { checkCallable(); - if (newName == null) + if (newName == null) { throw new InvalidRefNameException(MessageFormat.format(JGitText .get().branchNameInvalid, "")); //$NON-NLS-1$ - + } try { String fullOldName; String fullNewName; - if (repo.findRef(newName) != null) - throw new RefAlreadyExistsException(MessageFormat.format( - JGitText.get().refAlreadyExists1, newName)); if (oldName != null) { - Ref ref = repo.findRef(oldName); - if (ref == null) - throw new RefNotFoundException(MessageFormat.format( - JGitText.get().refNotResolved, oldName)); - if (ref.getName().startsWith(Constants.R_TAGS)) - throw new RefNotFoundException(MessageFormat.format( - JGitText.get().renameBranchFailedBecauseTag, - oldName)); + // Don't just rely on findRef -- if there are local and remote + // branches with the same name, and oldName is a short name, it + // does not uniquely identify the ref and we might end up + // renaming the wrong branch or finding a tag instead even + // if a unique branch for the name exists! + // + // OldName may be a either a short or a full name. + Ref ref = repo.exactRef(oldName); + if (ref == null) { + ref = repo.exactRef(Constants.R_HEADS + oldName); + Ref ref2 = repo.exactRef(Constants.R_REMOTES + oldName); + if (ref != null && ref2 != null) { + throw new RefNotFoundException(MessageFormat.format( + JGitText.get().renameBranchFailedAmbiguous, + oldName, ref.getName(), ref2.getName())); + } else if (ref == null) { + if (ref2 != null) { + ref = ref2; + } else { + throw new RefNotFoundException(MessageFormat.format( + JGitText.get().refNotResolved, oldName)); + } + } + } fullOldName = ref.getName(); } else { fullOldName = repo.getFullBranch(); @@ -124,26 +137,34 @@ public class RenameBranchCommand extends GitCommand { throw new DetachedHeadException(); } - if (fullOldName.startsWith(Constants.R_REMOTES)) + if (fullOldName.startsWith(Constants.R_REMOTES)) { fullNewName = Constants.R_REMOTES + newName; - else { + } else if (fullOldName.startsWith(Constants.R_HEADS)) { fullNewName = Constants.R_HEADS + newName; + } else { + throw new RefNotFoundException(MessageFormat.format( + JGitText.get().renameBranchFailedNotABranch, + fullOldName)); } - if (!Repository.isValidRefName(fullNewName)) + if (!Repository.isValidRefName(fullNewName)) { throw new InvalidRefNameException(MessageFormat.format(JGitText .get().branchNameInvalid, fullNewName)); - + } + if (repo.exactRef(fullNewName) != null) { + throw new RefAlreadyExistsException(MessageFormat + .format(JGitText.get().refAlreadyExists1, fullNewName)); + } RefRename rename = repo.renameRef(fullOldName, fullNewName); Result renameResult = rename.rename(); setCallable(false); - if (Result.RENAMED != renameResult) + if (Result.RENAMED != renameResult) { throw new JGitInternalException(MessageFormat.format(JGitText .get().renameBranchUnexpectedResult, renameResult .name())); - + } if (fullNewName.startsWith(Constants.R_HEADS)) { String shortOldName = fullOldName.substring(Constants.R_HEADS .length()); @@ -154,8 +175,9 @@ public class RenameBranchCommand extends GitCommand { String[] values = repoConfig.getStringList( ConfigConstants.CONFIG_BRANCH_SECTION, shortOldName, name); - if (values.length == 0) + if (values.length == 0) { continue; + } // Keep any existing values already configured for the // new branch name String[] existing = repoConfig.getStringList( @@ -180,10 +202,11 @@ public class RenameBranchCommand extends GitCommand { repoConfig.save(); } - Ref resultRef = repo.findRef(newName); - if (resultRef == null) + Ref resultRef = repo.exactRef(fullNewName); + if (resultRef == null) { throw new JGitInternalException( JGitText.get().renameBranchFailedUnknownReason); + } return resultRef; } catch (IOException ioe) { throw new JGitInternalException(ioe.getMessage(), ioe); @@ -191,7 +214,13 @@ public class RenameBranchCommand extends GitCommand { } /** - * Set the new name of the branch + * Sets the new short name of the branch. + *

+ * The full name is constructed using the prefix of the branch to be renamed + * defined by either {@link #setOldName(String)} or HEAD. If that old branch + * is a local branch, the renamed branch also will be, and if the old branch + * is a remote branch, so will be the renamed branch. + *

* * @param newName * the new name @@ -204,7 +233,11 @@ public class RenameBranchCommand extends GitCommand { } /** - * Set the old name of the branch + * Sets the old name of the branch. + *

+ * {@code oldName} may be a short or a full name. Using a full name is + * recommended to unambiguously identify the branch to be renamed. + *

* * @param oldName * the name of the branch to rename; if not set, the currently 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 bd91fe0f8..7dce43a71 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -651,8 +651,9 @@ public class JGitText extends TranslationBundle { /***/ public String remoteDoesNotHaveSpec; /***/ public String remoteDoesNotSupportSmartHTTPPush; /***/ public String remoteHungUpUnexpectedly; - /***/ public String remoteNameCantBeNull; - /***/ public String renameBranchFailedBecauseTag; + /***/ public String remoteNameCannotBeNull; + /***/ public String renameBranchFailedAmbiguous; + /***/ public String renameBranchFailedNotABranch; /***/ public String renameBranchFailedUnknownReason; /***/ public String renameBranchUnexpectedResult; /***/ public String renameCancelled; 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 9a67f0f8f..c34e3b8e6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteRefUpdate.java @@ -293,7 +293,7 @@ public class RemoteRefUpdate { final boolean forceUpdate, final String localName, final ObjectId expectedOldObjectId) throws IOException { if (remoteName == null) - throw new IllegalArgumentException(JGitText.get().remoteNameCantBeNull); + throw new IllegalArgumentException(JGitText.get().remoteNameCannotBeNull); if (srcId == null && srcRef != null) throw new IOException(MessageFormat.format( JGitText.get().sourceRefDoesntResolveToAnyObject, srcRef));