This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new f4c184991f fix commit during a merge aborting the merge, fixes #8001
(#8004)
f4c184991f is described below
commit f4c184991fd2dc8e86a74f68dd00ef95526a793d
Author: Bart Maertens <[email protected]>
AuthorDate: Wed Aug 19 15:55:45 2026 +0200
fix commit during a merge aborting the merge, fixes #8001 (#8004)
The commit perspective reset the whole index before staging the checked
files. JGit's ResetCommand clears MERGE_HEAD for any mode other than SOFT,
so resolving a merge conflict and committing recorded an ordinary
single-parent commit: git no longer considered the branch merged and
replayed everything on the next merge. Cherry-pick and revert state were
cleared the same way.
Move the staging into UIGit.commitPaths(), which unstages only the paths
which have to stay out of the commit, and leave the index alone entirely
while a merge, cherry-pick or revert is in progress. The commit
perspective refuses a partial commit there, the way git does.
---
.../org/apache/hop/git/GitCommitPerspective.java | 59 ++++++----
.../main/java/org/apache/hop/git/model/UIGit.java | 68 +++++++++++
.../hop/git/messages/messages_en_US.properties | 4 +
.../java/org/apache/hop/git/model/UIGitTest.java | 124 +++++++++++++++++++++
4 files changed, 231 insertions(+), 24 deletions(-)
diff --git
a/plugins/misc/git/src/main/java/org/apache/hop/git/GitCommitPerspective.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/GitCommitPerspective.java
index df397808ca..14d4cdb2b4 100644
---
a/plugins/misc/git/src/main/java/org/apache/hop/git/GitCommitPerspective.java
+++
b/plugins/misc/git/src/main/java/org/apache/hop/git/GitCommitPerspective.java
@@ -61,9 +61,6 @@ import
org.apache.hop.ui.hopgui.perspective.HopPerspectivePlugin;
import org.apache.hop.ui.hopgui.perspective.IHopPerspective;
import org.apache.hop.ui.hopgui.perspective.TabItemHandler;
import org.apache.hop.ui.hopgui.perspective.explorer.ExplorerPerspective;
-import org.eclipse.jgit.api.AddCommand;
-import org.eclipse.jgit.api.Git;
-import org.eclipse.jgit.api.ResetCommand;
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.swt.SWT;
@@ -848,31 +845,26 @@ public class GitCommitPerspective implements
IHopPerspective {
String message = wMessage.getText();
boolean amend = wAmend.getSelection();
- Git git = uiGit.getGit();
-
- // Reset all staged files
- git.reset().setMode(ResetCommand.ResetType.MIXED).call();
-
- // Add only selected files
- AddCommand addCommand = git.add();
- for (UIFile file : filesToCommit) {
- addCommand.addFilepattern(file.getName());
+ // A merge, cherry-pick or revert has to be committed as a whole: the
commit records the
+ // complete result, and for a merge it also records the second parent.
Say so instead of
+ // dropping the unchecked files from the commit behind the user's back.
+ //
+ String pendingOperation = getPendingOperation(uiGit);
+ if (pendingOperation != null && !filesToIgnore.isEmpty()) {
+ showStatus(
+ GuiResource.getInstance().getImageError(),
+ BaseMessages.getString(
+ PKG, "GitCommitPerspective.Error.PartialCommit.Message",
pendingOperation));
+ return;
}
- addCommand.call();
- // Commit selected files
- uiGit.commit(authorName, message, amend);
+ // Stage and commit the checked files, keeping the staged files which
were unchecked out of
+ // the commit
+ //
+ uiGit.commitPaths(
+ filesToCommit.stream().map(UIFile::getName).toList(), authorName,
message, amend);
String commitId = uiGit.getCommitId(Constants.HEAD);
- // Restore unselected staged files
- if (!filesToIgnore.isEmpty()) {
- AddCommand restoreCommand = git.add();
- for (UIFile file : filesToIgnore) {
- restoreCommand.addFilepattern(file.getName());
- }
- restoreCommand.call();
- }
-
GitGuiPlugin.getInstance().beforeRefresh();
refresh();
@@ -915,6 +907,25 @@ public class GitCommitPerspective implements
IHopPerspective {
}
}
+ /**
+ * The name of the operation git is in the middle of, to name it in a
message. Git refuses to
+ * commit part of the index while one of these is in progress.
+ *
+ * @param uiGit the repository to check
+ * @return the name of the operation, or null when git is not in the middle
of one
+ */
+ private String getPendingOperation(UIGit uiGit) {
+ return switch (uiGit.getRepositoryState()) {
+ case MERGING, MERGING_RESOLVED ->
+ BaseMessages.getString(PKG,
"GitCommitPerspective.Operation.Merge.Label");
+ case CHERRY_PICKING, CHERRY_PICKING_RESOLVED ->
+ BaseMessages.getString(PKG,
"GitCommitPerspective.Operation.CherryPick.Label");
+ case REVERTING, REVERTING_RESOLVED ->
+ BaseMessages.getString(PKG,
"GitCommitPerspective.Operation.Revert.Label");
+ default -> null;
+ };
+ }
+
/**
* Show a line of feedback next to the commit buttons. It is wiped by the
next {@link
* #updateGui()} so it lives exactly as long as the user's next action.
diff --git a/plugins/misc/git/src/main/java/org/apache/hop/git/model/UIGit.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/model/UIGit.java
index 728b6d28e9..c460cc35ea 100644
--- a/plugins/misc/git/src/main/java/org/apache/hop/git/model/UIGit.java
+++ b/plugins/misc/git/src/main/java/org/apache/hop/git/model/UIGit.java
@@ -51,6 +51,7 @@ import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.ui.core.dialog.EnterSelectionDialog;
import org.apache.hop.ui.core.dialog.ErrorDialog;
import org.apache.hop.ui.hopgui.HopGui;
+import org.eclipse.jgit.api.AddCommand;
import org.eclipse.jgit.api.CloneCommand;
import org.eclipse.jgit.api.DiffCommand;
import org.eclipse.jgit.api.Git;
@@ -61,6 +62,7 @@ import org.eclipse.jgit.api.MergeResult.MergeStatus;
import org.eclipse.jgit.api.PushCommand;
import org.eclipse.jgit.api.RemoteAddCommand;
import org.eclipse.jgit.api.RemoteRemoveCommand;
+import org.eclipse.jgit.api.ResetCommand;
import org.eclipse.jgit.api.ResetCommand.ResetType;
import org.eclipse.jgit.api.RevertCommand;
import org.eclipse.jgit.api.Status;
@@ -351,6 +353,72 @@ public class UIGit extends VCS {
}
}
+ /**
+ * The state git is in: whether it is in the middle of a merge, a
cherry-pick or a revert. A
+ * commit has to record the whole index while one of those is in progress,
so nothing may be left
+ * out of it.
+ *
+ * @return the state of the repository
+ */
+ public RepositoryState getRepositoryState() {
+ return git.getRepository().getRepositoryState();
+ }
+
+ /**
+ * Stage the given paths and commit them, so the commit records those paths
and nothing else.
+ *
+ * <p>Anything else which was staged is taken back out of the index one path
at a time. Resetting
+ * the whole index is the shorter way to write that, but it also clears
MERGE_HEAD: the commit
+ * would record a merge as an ordinary commit and git would no longer
consider the branch merged.
+ *
+ * <p>Nothing is unstaged while a merge, cherry-pick or revert is in
progress. Git commits the
+ * whole index there, so a selection cannot be honoured and the caller is
expected to offer all of
+ * it. See {@link #getRepositoryState()}.
+ *
+ * @param pathsToCommit the paths to stage and commit
+ * @param authorName the author of the commit, as "name <email>"
+ * @param message the commit message
+ * @param amend whether the commit should amend the previous commit
+ * @return true if the commit is successful; otherwise, false.
+ * @throws HopException if staging or the commit itself fails
+ */
+ public boolean commitPaths(
+ List<String> pathsToCommit, String authorName, String message, boolean
amend)
+ throws HopException {
+ try {
+ Set<String> commitPaths = new HashSet<>();
+ for (String path : pathsToCommit) {
+ commitPaths.add(normalizePathForJGit(path));
+ }
+
+ // What is staged right now, rather than what the caller last saw: a
file staged in the
+ // meantime must not be swept into this commit either.
+ //
+ if (getRepositoryState() == RepositoryState.SAFE) {
+ List<String> pathsToUnstage =
+ getStagedFiles().stream()
+ .map(UIFile::getName)
+ .filter(name -> !commitPaths.contains(name))
+ .toList();
+ if (!pathsToUnstage.isEmpty()) {
+ ResetCommand resetCommand = git.reset();
+ pathsToUnstage.forEach(resetCommand::addPath);
+ resetCommand.call();
+ }
+ }
+
+ if (!commitPaths.isEmpty()) {
+ AddCommand addCommand = git.add();
+ commitPaths.forEach(addCommand::addFilepattern);
+ addCommand.call();
+ }
+ } catch (Exception e) {
+ throw new HopException("Error staging the files to commit", e);
+ }
+
+ return commit(authorName, message, amend);
+ }
+
public List<ObjectRevision> getRevisions() {
return getRevisions(null);
}
diff --git
a/plugins/misc/git/src/main/resources/org/apache/hop/git/messages/messages_en_US.properties
b/plugins/misc/git/src/main/resources/org/apache/hop/git/messages/messages_en_US.properties
index 863dc8a946..9fb7f822ed 100644
---
a/plugins/misc/git/src/main/resources/org/apache/hop/git/messages/messages_en_US.properties
+++
b/plugins/misc/git/src/main/resources/org/apache/hop/git/messages/messages_en_US.properties
@@ -141,6 +141,10 @@ GitCommitPerspective.Menu.ShowTextDiff.Text=Show text diff
GitCommitPerspective.Menu.ShowGraphDiff.Text=Show visual diff
GitCommitPerspective.Menu.Delete.Text=Delete...
GitCommitPerspective.Error.NoFilesChecked.Message=Please check the box in
front of the files you want to commit
+GitCommitPerspective.Error.PartialCommit.Message=Cannot do a partial commit
during a {0}. Check all staged files to continue.
+GitCommitPerspective.Operation.Merge.Label=merge
+GitCommitPerspective.Operation.CherryPick.Label=cherry-pick
+GitCommitPerspective.Operation.Revert.Label=revert
GitCommitPerspective.Status.Committed.Message=Committed {0} file(s) {1}
GitCommitPerspective.Status.CommittedAndPushed.Message=Committed and pushed
{0} file(s) {1}
GitCommitPerspective.Status.CommitId.Label=as {0}
diff --git
a/plugins/misc/git/src/test/java/org/apache/hop/git/model/UIGitTest.java
b/plugins/misc/git/src/test/java/org/apache/hop/git/model/UIGitTest.java
index 1e0b14e391..38849baf3f 100644
--- a/plugins/misc/git/src/test/java/org/apache/hop/git/model/UIGitTest.java
+++ b/plugins/misc/git/src/test/java/org/apache/hop/git/model/UIGitTest.java
@@ -57,6 +57,7 @@ import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.RepositoryState;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.RemoteConfig;
@@ -1009,6 +1010,129 @@ public class UIGitTest extends RepositoryTestCase {
message.getValue().contains("Please commit or revert your changes
before you merge"));
}
+ /**
+ * A merge has to be recorded as a merge. Committing the resolved conflict
used to reset the whole
+ * index first, which cleared MERGE_HEAD and left an ordinary commit behind:
git no longer
+ * considered the branch merged and replayed everything on the next merge.
+ */
+ @Test
+ public void testCommitPathsAfterAMergeRecordsBothParents() throws Exception {
+ RevCommit base = initialCommit();
+ commitOnBranch("develop", "Test.txt", "Hello from develop");
+ RevCommit develop =
git.getRepository().parseCommit(git.getRepository().resolve("develop"));
+
+ // Let master change the same file, so merging develop conflicts
+ //
+ git.checkout().setName(Constants.MASTER).call();
+ writeTrashFile("Test.txt", "Hello from master");
+ git.add().addFilepattern("Test.txt").call();
+ RevCommit master = git.commit().setMessage("master commit").call();
+
+ assertTrue(uiGit.mergeBranch("develop", MergeStrategy.RECURSIVE));
+ assertEquals(RepositoryState.MERGING, uiGit.getRepositoryState());
+
+ // Resolve the conflict the way the commit perspective does: accept a
side, then commit
+ //
+ uiGit.add("Test.txt.ours");
+ assertEquals(RepositoryState.MERGING_RESOLVED, uiGit.getRepositoryState());
+
+ assertTrue(
+ uiGit.commitPaths(List.of("Test.txt"), "John Doe <[email protected]>",
"Merged", false));
+
+ RevCommit merged =
git.getRepository().parseCommit(git.getRepository().resolve(Constants.HEAD));
+ assertEquals("The merge has to be recorded as a merge commit", 2,
merged.getParentCount());
+ assertEquals(master, merged.getParent(0));
+ assertEquals(develop, merged.getParent(1));
+ assertEquals(RepositoryState.SAFE, uiGit.getRepositoryState());
+ assertNotEquals(base, merged);
+ }
+
+ /**
+ * The commit records the paths which were asked for and nothing else,
whether or not the caller
+ * knew about everything that was staged.
+ */
+ @Test
+ public void testCommitPathsCommitsOnlyTheGivenPaths() throws Exception {
+ initialCommit();
+
+ writeTrashFile("Committed.txt", "in the commit");
+ writeTrashFile("Unchecked.txt", "left out of the commit");
+ git.add().addFilepattern("Committed.txt").call();
+ git.add().addFilepattern("Unchecked.txt").call();
+
+ // Staged after the caller read its file list, so it is not in the
selection either
+ //
+ writeTrashFile("StagedInTheMeantime.txt", "staged behind the GUI's back");
+ git.add().addFilepattern("StagedInTheMeantime.txt").call();
+
+ assertTrue(
+ uiGit.commitPaths(
+ List.of("Committed.txt"), "John Doe <[email protected]>", "One file
only", false));
+
+ String head = uiGit.getCommitId(Constants.HEAD);
+ List<UIFile> committed =
uiGit.getStagedFiles(uiGit.getParentCommitId(head), head);
+ assertEquals(1, committed.size());
+ assertEquals("Committed.txt", committed.get(0).getName());
+
+ // Both of the others are out of the index and still on disk, nothing was
thrown away
+ //
+ Status status = git.status().call();
+ assertTrue(status.getUntracked().contains("Unchecked.txt"));
+ assertTrue(status.getUntracked().contains("StagedInTheMeantime.txt"));
+ assertTrue(new File(db.getWorkTree(), "Unchecked.txt").exists());
+ assertTrue(new File(db.getWorkTree(), "StagedInTheMeantime.txt").exists());
+ }
+
+ /**
+ * Committing part of a merge is not possible, so the whole index goes in
and the resolution the
+ * user staged is never quietly dropped.
+ */
+ @Test
+ public void testCommitPathsDuringAMergeKeepsTheRestOfTheIndexStaged() throws
Exception {
+ initialCommit();
+ commitOnBranch("develop", "Test.txt", "Hello from develop");
+
+ git.checkout().setName(Constants.MASTER).call();
+ writeTrashFile("Test.txt", "Hello from master");
+ git.add().addFilepattern("Test.txt").call();
+ git.commit().setMessage("master commit").call();
+
+ assertTrue(uiGit.mergeBranch("develop", MergeStrategy.RECURSIVE));
+ uiGit.add("Test.txt.ours");
+
+ // Another file staged during the merge has to survive into the merge
commit
+ //
+ writeTrashFile("AlsoResolved.txt", "resolved as well");
+ git.add().addFilepattern("AlsoResolved.txt").call();
+
+ assertTrue(
+ uiGit.commitPaths(List.of("Test.txt"), "John Doe <[email protected]>",
"Merged", false));
+
+ String head = uiGit.getCommitId(Constants.HEAD);
+ List<UIFile> committed =
uiGit.getStagedFiles(uiGit.getParentCommitId(head), head);
+ assertTrue(committed.stream().anyMatch(file ->
file.getName().equals("AlsoResolved.txt")));
+ assertTrue(uiGit.isClean());
+ }
+
+ /**
+ * A commit that fails must leave the files it was asked to commit staged,
so nothing has to be
+ * staged a second time to retry.
+ */
+ @Test
+ public void testCommitPathsLeavesTheSelectionStagedWhenTheCommitFails()
throws Exception {
+ initialCommit();
+
+ writeTrashFile("Staged.txt", "staged content");
+
+ // A malformed author name (no e-mail address) makes the commit itself fail
+ //
+ assertThrows(
+ Exception.class,
+ () -> uiGit.commitPaths(List.of("Staged.txt"), "no email address",
"Nope", false));
+
+ assertTrue(git.status().call().getAdded().contains("Staged.txt"));
+ }
+
private void commitOnBranch(String branch, String file, String content)
throws Exception {
git.branchCreate().setName(branch).call();
git.checkout().setName(branch).call();
