motiejus
/
jgit
1
Fork 0
Code Releases Activity

RebaseCommand: fix commit message in "fixup" case 

JGit accumulated in MESSAGE_FIXUP commit messages of a fixup sequence,
just like it did in MESSAGE_SQUASH, and on the last step of a sequence
of fixups used that file, after stripping all comment lines, as the
commit message. That also stripped any lines from the original commit
message that happened to start with the comment character.

This is not how this is supposed to work. MESSAGE_FIXUP must contain
the original commit message of the base commit that is amended, and
the file contains the verbatim commit message for the final fixup.[1]

Change the implementation accordingly, and add new tests.

[1] https://github.com/git/git/blob/df3c41adeb/sequencer.c#L86 ff.

Bug: 513726
Change-Id: I885a2b7f10d6c74460a8693aa6cbf867ee0494a1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This commit is contained in:
Thomas Wolf 2022-01-18 00:37:33 +01:00 committed by Matthias Sohn
parent 92158af52e
commit 035d24097d
2 changed files with 57 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
View File

@ -2927,8 +2927,8 @@ public String modifyCommitMessage(String commit) {
}
}
@Test
public void testRebaseInteractiveFixupWithBlankLines() throws Exception {
private void simpleFixup(String firstMessage, String secondMessage)
throws Exception {
// create file1 on master
writeTrashFile(FILE1, FILE1);
git.add().addFilepattern(FILE1).call();
@ -2938,13 +2938,13 @@ public void testRebaseInteractiveFixupWithBlankLines() throws Exception {
// create file2 on master
writeTrashFile("file2", "file2");
git.add().addFilepattern("file2").call();
git.commit().setMessage("Add file2").call();
git.commit().setMessage(firstMessage).call();
assertTrue(new File(db.getWorkTree(), "file2").exists());
// update FILE1 on master
writeTrashFile(FILE1, "blah");
git.add().addFilepattern(FILE1).call();
git.commit().setMessage("updated file1 on master\n\nsome text").call();
git.commit().setMessage(secondMessage).call();
git.rebase().setUpstream("HEAD~2")
.runInteractively(new InteractiveHandler() {
@ -2968,9 +2968,31 @@ public String modifyCommitMessage(String commit) {
try (RevWalk walk = new RevWalk(db)) {
ObjectId headId = db.resolve(Constants.HEAD);
RevCommit headCommit = walk.parseCommit(headId);
assertEquals("Add file2",
headCommit.getFullMessage());
assertEquals(firstMessage, headCommit.getFullMessage());
}
}
@Test
public void testRebaseInteractiveFixupWithBlankLines() throws Exception {
simpleFixup("Add file2", "updated file1 on master\n\nsome text");
}
@Test
public void testRebaseInteractiveFixupWithBlankLines2() throws Exception {
simpleFixup("Add file2\n\nBody\n",
"updated file1 on master\n\nsome text");
}
@Test
public void testRebaseInteractiveFixupWithHash() throws Exception {
simpleFixup("#Add file2", "updated file1 on master");
}
@Test
public void testRebaseInteractiveFixupWithHash2() throws Exception {
simpleFixup("#Add file2\n\nHeader has hash\n",
"#updated file1 on master");
}
@Test(expected = InvalidRebaseStepException.class)

54
org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
View File

@ -466,12 +466,23 @@ private RebaseResult processStep(RebaseTodoLine step, boolean shouldPick)
resetSoftToParent();
List<RebaseTodoLine> steps = repo.readRebaseTodo(
rebaseState.getPath(GIT_REBASE_TODO), false);
RebaseTodoLine nextStep = steps.isEmpty() ? null : steps.get(0);
boolean isLast = steps.isEmpty();
if (!isLast) {
switch (steps.get(0).getAction()) {
case FIXUP:
case SQUASH:
break;
default:
isLast = true;
break;
}
}
File messageFixupFile = rebaseState.getFile(MESSAGE_FIXUP);
File messageSquashFile = rebaseState.getFile(MESSAGE_SQUASH);
if (isSquash && messageFixupFile.exists())
if (isSquash && messageFixupFile.exists()) {
messageFixupFile.delete();
newHead = doSquashFixup(isSquash, commitToPick, nextStep,
}
newHead = doSquashFixup(isSquash, commitToPick, isLast,
messageFixupFile, messageSquashFile);
}
return null;
@ -732,7 +743,7 @@ private void checkSteps(List<RebaseTodoLine> steps)
}
private RevCommit doSquashFixup(boolean isSquash, RevCommit commitToPick,
RebaseTodoLine nextStep, File messageFixup, File messageSquash)
boolean isLast, File messageFixup, File messageSquash)
throws IOException, GitAPIException {
if (!messageSquash.exists()) {
@ -742,24 +753,20 @@ private RevCommit doSquashFixup(boolean isSquash, RevCommit commitToPick,
initializeSquashFixupFile(MESSAGE_SQUASH,
previousCommit.getFullMessage());
if (!isSquash)
initializeSquashFixupFile(MESSAGE_FIXUP,
previousCommit.getFullMessage());
if (!isSquash) {
rebaseState.createFile(MESSAGE_FIXUP,
previousCommit.getFullMessage());
}
}
String currSquashMessage = rebaseState
.readFile(MESSAGE_SQUASH);
String currSquashMessage = rebaseState.readFile(MESSAGE_SQUASH);
int count = parseSquashFixupSequenceCount(currSquashMessage) + 1;
String content = composeSquashMessage(isSquash,
commitToPick, currSquashMessage, count);
rebaseState.createFile(MESSAGE_SQUASH, content);
if (messageFixup.exists())
rebaseState.createFile(MESSAGE_FIXUP, content);
return squashIntoPrevious(
!messageFixup.exists(),
nextStep);
return squashIntoPrevious(!messageFixup.exists(), isLast);
}
private void resetSoftToParent() throws IOException,
@ -781,29 +788,26 @@ private void resetSoftToParent() throws IOException,
}
private RevCommit squashIntoPrevious(boolean sequenceContainsSquash,
RebaseTodoLine nextStep)
boolean isLast)
throws IOException, GitAPIException {
RevCommit retNewHead;
String commitMessage = rebaseState
.readFile(MESSAGE_SQUASH);
String commitMessage;
if (!isLast || sequenceContainsSquash) {
commitMessage = rebaseState.readFile(MESSAGE_SQUASH);
} else {
commitMessage = rebaseState.readFile(MESSAGE_FIXUP);
}
try (Git git = new Git(repo)) {
if (nextStep == null || ((nextStep.getAction() != Action.FIXUP)
&& (nextStep.getAction() != Action.SQUASH))) {
// this is the last step in this sequence
if (isLast) {
if (sequenceContainsSquash) {
commitMessage = editCommitMessage(commitMessage,
CleanupMode.STRIP);
} else {
commitMessage = CommitConfig.cleanText(commitMessage,
CleanupMode.STRIP, '#');
}
retNewHead = git.commit()
.setMessage(commitMessage)
.setAmend(true).setNoVerify(true).call();
rebaseState.getFile(MESSAGE_SQUASH).delete();
rebaseState.getFile(MESSAGE_FIXUP).delete();
} else {
// Next step is either Squash or Fixup
retNewHead = git.commit().setMessage(commitMessage)