From 318f3d464307e3efd8342852310c17e71a7282fe Mon Sep 17 00:00:00 2001 From: Tomasz Zarna Date: Mon, 29 Oct 2012 16:21:59 +0100 Subject: [PATCH] Add support for --no-ff while merging Bug: 394432 Change-Id: I373128c0ba949f9b24248874f77f3d68b50ccfd1 Signed-off-by: Chris Aniszczyk --- .../tst/org/eclipse/jgit/pgm/MergeTest.java | 59 +++++++++++++- .../org/eclipse/jgit/pgm/CLIText.properties | 3 + .../src/org/eclipse/jgit/pgm/CLIText.java | 3 + .../src/org/eclipse/jgit/pgm/Merge.java | 59 +++++++++++++- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/api/MergeCommand.java | 76 ++++++++++++++++--- .../src/org/eclipse/jgit/api/MergeResult.java | 14 ++++ .../org/eclipse/jgit/internal/JGitText.java | 1 + 8 files changed, 200 insertions(+), 16 deletions(-) diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeTest.java index 87a2c8553..28a107bcb 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/MergeTest.java @@ -42,8 +42,8 @@ */ package org.eclipse.jgit.pgm; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.CLIRepositoryTestCase; @@ -82,7 +82,8 @@ public class MergeTest extends CLIRepositoryTestCase { git.commit().setMessage("commit").call(); git.checkout().setName("side").call(); - assertEquals("Fast-forward", execute("git merge master")[0]); + assertArrayEquals(new String[] { "Updating 6fd41be..26a81a1", + "Fast-forward", "" }, execute("git merge master")); } @Test @@ -120,4 +121,58 @@ public class MergeTest extends CLIRepositoryTestCase { "" }, execute("git merge master --squash")); } + + @Test + public void testNoFastForward() throws Exception { + git.branchCreate().setName("side").call(); + writeTrashFile("file", "master"); + git.add().addFilepattern("file").call(); + git.commit().setMessage("commit").call(); + git.checkout().setName("side").call(); + + assertEquals("Merge made by the 'recursive' strategy.", + execute("git merge master --no-ff")[0]); + assertArrayEquals(new String[] { + "commit 6db23724012376e8407fc24b5da4277a9601be81", // + "Author: GIT_COMMITTER_NAME ", // + "Date: Sat Aug 15 20:12:58 2009 -0330", // + "", // + " Merge branch 'master' into side", // + "", // + "commit 6fd41be26b7ee41584dd997f665deb92b6c4c004", // + "Author: GIT_COMMITTER_NAME ", // + "Date: Sat Aug 15 20:12:58 2009 -0330", // + "", // + " initial commit", // + "", // + "commit 26a81a1c6a105551ba703a8b6afc23994cacbae1", // + "Author: GIT_COMMITTER_NAME ", // + "Date: Sat Aug 15 20:12:58 2009 -0330", // + "", // + " commit", // + "", // + "" + }, execute("git log")); + } + + @Test + public void testNoFastForwardAndSquash() throws Exception { + assertEquals("fatal: You cannot combine --squash with --no-ff.", + execute("git merge master --no-ff --squash")[0]); + } + + @Test + public void testFastForwardOnly() throws Exception { + git.branchCreate().setName("side").call(); + writeTrashFile("file", "master"); + git.add().addFilepattern("file").call(); + git.commit().setMessage("commit#1").call(); + git.checkout().setName("side").call(); + writeTrashFile("file", "side"); + git.add().addFilepattern("file").call(); + git.commit().setMessage("commit#2").call(); + + assertEquals("fatal: Not possible to fast-forward, aborting.", + execute("git merge master --ff-only")[0]); + } } diff --git a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties index 6f5c8eb68..cb1574f26 100644 --- a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties +++ b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/CLIText.properties @@ -18,6 +18,7 @@ branchNotFound=branch '{0}' not found. cacheTreePathInfo="{0}": {1} entries, {2} children cannotBeRenamed={0} cannot be renamed cannotChekoutNoHeadsAdvertisedByRemote=cannot checkout; no HEAD advertised by remote +cannotCombineSquashWithNoff=You cannot combine --squash with --no-ff. cannotCreateCommand=Cannot create command {0} cannotCreateOutputStream=cannot create output stream cannotDeatchHEAD=Cannot detach HEAD @@ -55,6 +56,7 @@ failedToLockTag=Failed to lock tag {0}: {1} fatalError=fatal: {0} fatalThisProgramWillDestroyTheRepository=fatal: This program will destroy the repository\nfatal:\nfatal:\nfatal: {0}\nfatal:\nfatal: To continue, add {1} to the command line\nfatal: fileIsRequired=argument file is required +ffNotPossibleAborting=Not possible to fast-forward, aborting. forcedUpdate=forced update fromURI=From {0} initializedEmptyGitRepositoryIn=Initialized empty Git repository in {0} @@ -160,6 +162,7 @@ unknownMergeStrategy=unknown merge strategy {0} specified unmergedPaths=Unmerged paths: unsupportedOperation=Unsupported operation: {0} untrackedFiles=Untracked files: +updating=Updating {0}..{1} usage_Blame=Show what revision and author last modified each line usage_CommandLineClientForamazonsS3Service=Command line client for Amazon's S3 service usage_CommitAll=commit all modified and deleted files diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/CLIText.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/CLIText.java index c1cac71cf..5a0b8b287 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/CLIText.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/CLIText.java @@ -86,6 +86,7 @@ public class CLIText extends TranslationBundle { /***/ public String configFileNotFound; /***/ public String cannotBeRenamed; /***/ public String cannotChekoutNoHeadsAdvertisedByRemote; + /***/ public String cannotCombineSquashWithNoff; /***/ public String cannotCreateCommand; /***/ public String cannotCreateOutputStream; /***/ public String cannotDeatchHEAD; @@ -122,6 +123,7 @@ public class CLIText extends TranslationBundle { /***/ public String fatalError; /***/ public String fatalThisProgramWillDestroyTheRepository; /***/ public String fileIsRequired; + /***/ public String ffNotPossibleAborting; /***/ public String forcedUpdate; /***/ public String fromURI; /***/ public String initializedEmptyGitRepositoryIn; @@ -221,5 +223,6 @@ public class CLIText extends TranslationBundle { /***/ public String unmergedPaths; /***/ public String unsupportedOperation; /***/ public String untrackedFiles; + /***/ public String updating; /***/ public String warningNoCommitGivenOnCommandLine; } diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java index 728866d18..e79a8ff0a 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java @@ -43,14 +43,22 @@ package org.eclipse.jgit.pgm; +import java.io.IOException; import java.text.MessageFormat; import java.util.Map; import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.MergeCommand; import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.api.MergeCommand.FastForwardMode; +import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.merge.MergeStrategy; import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; @@ -68,8 +76,23 @@ class Merge extends TextBuiltin { @Argument(required = true) private String ref; + @Option(name = "--ff") + private FastForwardMode ff = FastForwardMode.FF; + + @Option(name = "--no-ff") + void noff(@SuppressWarnings("unused") final boolean ignored) { + ff = FastForwardMode.NO_FF; + } + + @Option(name = "--ff-only") + void ffonly(@SuppressWarnings("unused") final boolean ignored) { + ff = FastForwardMode.FF_ONLY; + } + @Override protected void run() throws Exception { + if (squash && ff == FastForwardMode.NO_FF) + throw die(CLIText.get().cannotCombineSquashWithNoff); // determine the merge strategy if (strategyName != null) { mergeStrategy = MergeStrategy.get(strategyName); @@ -79,14 +102,21 @@ class Merge extends TextBuiltin { } // determine the other revision we want to merge with HEAD + final Ref srcRef = db.getRef(ref); final ObjectId src = db.resolve(ref + "^{commit}"); if (src == null) throw die(MessageFormat.format( CLIText.get().refDoesNotExistOrNoCommit, ref)); + Ref oldHead = db.getRef(Constants.HEAD); Git git = new Git(db); - MergeResult result = git.merge().setStrategy(mergeStrategy) - .setSquash(squash).include(src).call(); + MergeCommand mergeCmd = git.merge().setStrategy(mergeStrategy) + .setSquash(squash).setFastForward(ff); + if (srcRef != null) + mergeCmd.include(srcRef); + else + mergeCmd.include(src); + MergeResult result = mergeCmd.call(); switch (result.getMergeStatus()) { case ALREADY_UP_TO_DATE: @@ -95,6 +125,10 @@ class Merge extends TextBuiltin { outw.println(CLIText.get().alreadyUpToDate); break; case FAST_FORWARD: + ObjectId oldHeadId = oldHead.getObjectId(); + outw.println(MessageFormat.format(CLIText.get().updating, oldHeadId + .abbreviate(7).name(), result.getNewHead().abbreviate(7) + .name())); outw.println(result.getMergeStatus().toString()); break; case CONFLICTING: @@ -119,15 +153,32 @@ class Merge extends TextBuiltin { } break; case MERGED: - outw.println(MessageFormat.format(CLIText.get().mergeMadeBy, - mergeStrategy.getName())); + String name; + if (!isMergedInto(oldHead, src)) + name = mergeStrategy.getName(); + else + name = "recursive"; + outw.println(MessageFormat.format(CLIText.get().mergeMadeBy, name)); break; case MERGED_SQUASHED: outw.println(CLIText.get().mergedSquashed); break; + case ABORTED: + throw die(CLIText.get().ffNotPossibleAborting); case NOT_SUPPORTED: outw.println(MessageFormat.format( CLIText.get().unsupportedOperation, result.toString())); } } + + private boolean isMergedInto(Ref oldHead, AnyObjectId src) + throws IOException { + RevWalk revWalk = new RevWalk(db); + ObjectId oldHeadObjectId = oldHead.getPeeledObjectId(); + if (oldHeadObjectId == null) + oldHeadObjectId = oldHead.getObjectId(); + RevCommit oldHeadCommit = revWalk.lookupCommit(oldHeadObjectId); + RevCommit srcCommit = revWalk.lookupCommit(src); + return revWalk.isMergedInto(oldHeadCommit, srcCommit); + } } 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 8018ab9ba..70f1313a4 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -31,6 +31,7 @@ branchNameInvalid=Branch name {0} is not allowed cachedPacksPreventsIndexCreation=Using cached packs prevents index creation cannotBeCombined=Cannot be combined. cannotBeRecursiveWhenTreesAreIncluded=TreeWalk shouldn't be recursive when tree objects are included. +cannotCombineSquashWithNoff=Cannot combine --squash with --no-ff. cannotCombineTreeFilterWithRevFilter=Cannot combine TreeFilter {0} with RevFilter {1}. cannotCommitOnARepoWithState=Cannot commit on a repo with state: {0} cannotCommitWriteTo=Cannot commit write to {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java index 3ca861c06..a884c70cb 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java @@ -99,6 +99,30 @@ public class MergeCommand extends GitCommand { private boolean squash; + private FastForwardMode fastForwardMode = FastForwardMode.FF; + + /** + * The modes available for fast forward merges (corresponding to the --ff, + * --no-ff and --ff-only options). + */ + public enum FastForwardMode { + /** + * Corresponds to the default --ff option (for a fast forward update the + * branch pointer only). + */ + FF, + /** + * Corresponds to the --no-ff option (create a merge commit even for a + * fast forward). + */ + NO_FF, + /** + * Corresponds to the --ff-only option (abort unless the merge is a fast + * forward). + */ + FF_ONLY; + } + /** * @param repo */ @@ -118,14 +142,7 @@ public class MergeCommand extends GitCommand { ConcurrentRefUpdateException, CheckoutConflictException, InvalidMergeHeadsException, WrongRepositoryStateException, NoMessageException { checkCallable(); - - if (commits.size() != 1) - throw new InvalidMergeHeadsException( - commits.isEmpty() ? JGitText.get().noMergeHeadSpecified - : MessageFormat.format( - JGitText.get().mergeStrategyDoesNotSupportHeads, - mergeStrategy.getName(), - Integer.valueOf(commits.size()))); + checkParameters(); RevWalk revWalk = null; DirCacheCheckout dco = null; @@ -179,7 +196,8 @@ public class MergeCommand extends GitCommand { return new MergeResult(headCommit, srcCommit, new ObjectId[] { headCommit, srcCommit }, MergeStatus.ALREADY_UP_TO_DATE, mergeStrategy, null, null); - } else if (revWalk.isMergedInto(headCommit, srcCommit)) { + } else if (revWalk.isMergedInto(headCommit, srcCommit) + && fastForwardMode == FastForwardMode.FF) { // FAST_FORWARD detected: skip doing a real merge but only // update HEAD refLogMessage.append(": " + MergeStatus.FAST_FORWARD); @@ -210,6 +228,11 @@ public class MergeCommand extends GitCommand { headCommit, srcCommit }, mergeStatus, mergeStrategy, null, msg); } else { + if (fastForwardMode == FastForwardMode.FF_ONLY) { + return new MergeResult(headCommit, srcCommit, + new ObjectId[] { headCommit, srcCommit }, + MergeStatus.ABORTED, mergeStrategy, null, null); + } String mergeMessage = ""; if (!squash) { mergeMessage = new MergeMessageFormatter().format( @@ -241,7 +264,10 @@ public class MergeCommand extends GitCommand { } else noProblems = merger.merge(headCommit, srcCommit); refLogMessage.append(": Merge made by "); - refLogMessage.append(mergeStrategy.getName()); + if (!revWalk.isMergedInto(headCommit, srcCommit)) + refLogMessage.append(mergeStrategy.getName()); + else + refLogMessage.append("recursive"); refLogMessage.append('.'); if (noProblems) { dco = new DirCacheCheckout(repo, @@ -305,6 +331,21 @@ public class MergeCommand extends GitCommand { } } + private void checkParameters() throws InvalidMergeHeadsException { + if (squash && fastForwardMode == FastForwardMode.NO_FF) { + throw new JGitInternalException( + JGitText.get().cannotCombineSquashWithNoff); + } + + if (commits.size() != 1) + throw new InvalidMergeHeadsException( + commits.isEmpty() ? JGitText.get().noMergeHeadSpecified + : MessageFormat.format( + JGitText.get().mergeStrategyDoesNotSupportHeads, + mergeStrategy.getName(), + Integer.valueOf(commits.size()))); + } + private void updateHead(StringBuilder refLogMessage, ObjectId newHeadId, ObjectId oldHeadID) throws IOException, ConcurrentRefUpdateException { @@ -392,4 +433,19 @@ public class MergeCommand extends GitCommand { this.squash = squash; return this; } + + /** + * Sets the fast forward mode. + * + * @param fastForwardMode + * corresponds to the --ff/--no-ff/--ff-only options. --ff is the + * default option. + * @return {@code this} + * @since 2.2 + */ + public MergeCommand setFastForward(FastForwardMode fastForwardMode) { + checkCallable(); + this.fastForwardMode = fastForwardMode; + return this; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java index 484039e70..8485a1544 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java @@ -152,6 +152,20 @@ public class MergeResult { return false; } }, + /** + * @since 2.2 + */ + ABORTED { + @Override + public String toString() { + return "Aborted"; + } + + @Override + public boolean isSuccessful() { + return false; + } + }, /** */ NOT_SUPPORTED { @Override 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 f6c48c92c..e115a73ce 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -91,6 +91,7 @@ public class JGitText extends TranslationBundle { /***/ public String cachedPacksPreventsIndexCreation; /***/ public String cannotBeCombined; /***/ public String cannotBeRecursiveWhenTreesAreIncluded; + /***/ public String cannotCombineSquashWithNoff; /***/ public String cannotCombineTreeFilterWithRevFilter; /***/ public String cannotCommitOnARepoWithState; /***/ public String cannotCommitWriteTo;