From 33d4b40ef29e282186b5f36822f10b39d55ef9b6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 5 Oct 2025 10:51:55 +0200 Subject: [PATCH] Fix dropping submodule changes from a commit Our logic to decide if a file needs to be checked out from the previous commit or deleted because it didn't exist in the previous commit didn't work for submodules. We were using `git cat-file -e` to ask whether the file existed, but this returns an error for submodules, so we were always deleting those instead of reverting them back to their previous state. Switch to using `git ls-tree -- file` instead, which works for both files and submodules. --- pkg/commands/git_commands/rebase.go | 15 +++++++++++---- pkg/commands/git_commands/rebase_test.go | 2 +- .../tests/commit/discard_submodule_changes.go | 3 --- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index d882d9b1a..a7b1dd215 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -519,10 +519,17 @@ func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, comm } for _, filePath := range filePaths { - // check if file exists in previous commit (this command returns an error if the file doesn't exist) - cmdArgs := NewGitCmd("cat-file").Arg("-e", "HEAD^:"+filePath).ToArgv() - - if err := self.cmd.New(cmdArgs).Run(); err != nil { + doesFileExistInPreviousCommit := false + if commitIndex < len(commits)-1 { + // check if file exists in previous commit (this command returns an empty string if the file doesn't exist) + cmdArgs := NewGitCmd("ls-tree").Arg("--name-only", "HEAD^", "--", filePath).ToArgv() + output, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() + if err != nil { + return err + } + doesFileExistInPreviousCommit = strings.TrimRight(output, "\n") == filePath + } + if !doesFileExistInPreviousCommit { if err := self.os.Remove(filePath); err != nil { return err } diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index 40385bf09..46f1fcc1c 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -139,7 +139,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { fileName: []string{"test999.txt"}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"rebase", "--interactive", "--autostash", "--keep-empty", "--no-autosquash", "--rebase-merges", "abcdef"}, "", nil). - ExpectGitArgs([]string{"cat-file", "-e", "HEAD^:test999.txt"}, "", nil). + ExpectGitArgs([]string{"ls-tree", "--name-only", "HEAD^", "--", "test999.txt"}, "test999.txt\n", nil). ExpectGitArgs([]string{"checkout", "HEAD^", "--", "test999.txt"}, "", nil). ExpectGitArgs([]string{"commit", "--amend", "--no-edit", "--allow-empty", "--allow-empty-message"}, "", nil). ExpectGitArgs([]string{"rebase", "--continue"}, "", nil), diff --git a/pkg/integration/tests/commit/discard_submodule_changes.go b/pkg/integration/tests/commit/discard_submodule_changes.go index df54dcd87..b457a3a41 100644 --- a/pkg/integration/tests/commit/discard_submodule_changes.go +++ b/pkg/integration/tests/commit/discard_submodule_changes.go @@ -49,9 +49,6 @@ var DiscardSubmoduleChanges = NewIntegrationTest(NewIntegrationTestArgs{ Confirm() t.Shell().RunCommand([]string{"git", "submodule", "update"}) - /* EXPECTED: t.FileSystem().FileContent("submodule/file", Equals("content")) - ACTUAL: */ - t.FileSystem().PathNotPresent("submodule/file") }, })