From f93ccb7fd4c619d5eeeb6fb46ca20fce73c597a0 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 13 Oct 2023 20:00:06 +0200 Subject: [PATCH] RebaseCommand: return correct status on stash apply conflicts Ensure that also the fast-forward cases return status STASH_APPLY_CONFLICTS when applying the stash produces conflicts. Bug: 582526 Change-Id: Ib989ff431dca6e301eb05156ca054a7115fa6ad5 Signed-off-by: Thomas Wolf --- .../eclipse/jgit/api/RebaseCommandTest.java | 53 ++++++++++++++++++- .../org/eclipse/jgit/api/RebaseCommand.java | 31 ++++++----- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java index 7f820b043..16f7cd1eb 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java @@ -2221,7 +2221,7 @@ public void testFastForwardRebaseWithAutoStash() throws Exception { checkoutBranch("refs/heads/master"); writeTrashFile(FILE1, "modified file1"); git.add().addFilepattern(FILE1).call(); - git.commit().setMessage("commit3").call(); + git.commit().setMessage("commit2").call(); // checkout topic branch / modify file0 checkoutBranch("refs/heads/topic"); @@ -2240,6 +2240,57 @@ public void testFastForwardRebaseWithAutoStash() throws Exception { assertEquals(RepositoryState.SAFE, db.getRepositoryState()); } + @Test + public void testFastForwardRebaseWithAutoStashConflict() throws Exception { + // create file0, add and commit + db.getConfig().setBoolean(ConfigConstants.CONFIG_REBASE_SECTION, null, + ConfigConstants.CONFIG_KEY_AUTOSTASH, true); + writeTrashFile("file0", "file0"); + git.add().addFilepattern("file0").call(); + git.commit().setMessage("commit0").call(); + // create file1, add and commit + writeTrashFile(FILE1, "file1"); + git.add().addFilepattern(FILE1).call(); + RevCommit commit = git.commit().setMessage("commit1").call(); + + // create topic branch + createBranch(commit, "refs/heads/topic"); + + // checkout master branch / modify file1, add and commit + checkoutBranch("refs/heads/master"); + writeTrashFile(FILE1, "modified file1"); + git.add().addFilepattern(FILE1).call(); + RevCommit master = git.commit().setMessage("commit2").call(); + + // checkout topic branch / modify file0 and file1 + checkoutBranch("refs/heads/topic"); + writeTrashFile("file0", "unstaged modified file0"); + writeTrashFile(FILE1, "unstaged modified file1"); + + // rebase + assertEquals(Status.STASH_APPLY_CONFLICTS, + git.rebase().setUpstream("refs/heads/master").call() + .getStatus()); + checkFile(new File(db.getWorkTree(), "file0"), + "unstaged modified file0"); + checkFile(new File(db.getWorkTree(), FILE1), + "<<<<<<< HEAD\n" + + "modified file1\n" + + "=======\n" + + "unstaged modified file1\n" + + ">>>>>>> stash\n"); + // If there is a merge conflict, the index is not reset, and thus file0 + // is staged here. This is the same behavior as in C git. + String expected = "[file0, mode:100644, content:unstaged modified file0]" + + "[file1, mode:100644, stage:1, content:file1]" + + "[file1, mode:100644, stage:2, content:modified file1]" + + "[file1, mode:100644, stage:3, content:unstaged modified file1]"; + assertEquals(expected, indexState(CONTENT)); + assertEquals(RepositoryState.SAFE, db.getRepositoryState()); + assertEquals(master, db.resolve(Constants.HEAD)); + assertEquals(master, db.resolve("refs/heads/topic")); + } + private List getStashedDiff() throws AmbiguousObjectException, IncorrectObjectTypeException, IOException, MissingObjectException { ObjectId stashId = db.resolve("stash@{0}"); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java index 859bf9384..0f7b8afe2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java @@ -289,13 +289,17 @@ public RebaseResult call() throws GitAPIException, NoHeadException, } } RebaseResult res = initFilesAndRewind(); - if (stopAfterInitialization) + if (stopAfterInitialization) { return RebaseResult.INTERACTIVE_PREPARED_RESULT; + } if (res != null) { - autoStashApply(); - if (rebaseState.getDir().exists()) + if (!autoStashApply()) { + res = RebaseResult.STASH_APPLY_CONFLICTS_RESULT; + } + if (rebaseState.getDir().exists()) { FileUtils.delete(rebaseState.getDir(), FileUtils.RECURSIVE); + } return res; } } @@ -381,7 +385,7 @@ private void autoStash() throws GitAPIException, IOException { } private boolean autoStashApply() throws IOException, GitAPIException { - boolean conflicts = false; + boolean success = true; if (rebaseState.getFile(AUTOSTASH).exists()) { String stash = rebaseState.readFile(AUTOSTASH); try (Git git = Git.wrap(repo)) { @@ -389,7 +393,7 @@ private boolean autoStashApply() throws IOException, GitAPIException { .ignoreRepositoryState(true).setStrategy(strategy) .call(); } catch (StashApplyFailureException e) { - conflicts = true; + success = false; try (RevWalk rw = new RevWalk(repo)) { ObjectId stashId = repo.resolve(stash); RevCommit commit = rw.parseCommit(stashId); @@ -398,7 +402,7 @@ private boolean autoStashApply() throws IOException, GitAPIException { } } } - return conflicts; + return success; } private void updateStashRef(ObjectId commitId, PersonIdent refLogIdent, @@ -723,13 +727,15 @@ private RebaseResult finishRebase(RevCommit finalHead, boolean lastStepIsForward) throws IOException, GitAPIException { String headName = rebaseState.readFile(HEAD_NAME); updateHead(headName, finalHead, upstreamCommit); - boolean stashConflicts = autoStashApply(); + boolean unstashSuccessful = autoStashApply(); getRepository().autoGC(monitor); FileUtils.delete(rebaseState.getDir(), FileUtils.RECURSIVE); - if (stashConflicts) + if (!unstashSuccessful) { return RebaseResult.STASH_APPLY_CONFLICTS_RESULT; - if (lastStepIsForward || finalHead == null) + } + if (lastStepIsForward || finalHead == null) { return RebaseResult.FAST_FORWARD_RESULT; + } return RebaseResult.OK_RESULT; } @@ -1149,7 +1155,7 @@ private RebaseResult initFilesAndRewind() throws IOException, if (!isInteractive() && walk.isMergedInto(upstream, headCommit)) return RebaseResult.UP_TO_DATE_RESULT; else if (!isInteractive() && walk.isMergedInto(headCommit, upstream)) { - // head is already merged into upstream, fast-foward + // head is already merged into upstream, fast-forward monitor.beginTask(MessageFormat.format( JGitText.get().resettingHead, upstreamCommit.getShortMessage()), ProgressMonitor.UNKNOWN); @@ -1447,13 +1453,14 @@ private RebaseResult abort(RebaseResult result) throws IOException, throw new JGitInternalException( JGitText.get().abortingRebaseFailed); } - boolean stashConflicts = autoStashApply(); + boolean unstashSuccessful = autoStashApply(); // cleanup the files FileUtils.delete(rebaseState.getDir(), FileUtils.RECURSIVE); repo.writeCherryPickHead(null); repo.writeMergeHeads(null); - if (stashConflicts) + if (!unstashSuccessful) { return RebaseResult.STASH_APPLY_CONFLICTS_RESULT; + } return result; } finally {