From 21483b259c4f63b27ce7a4ef7f8ef93174f289ab Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 5 Oct 2025 14:23:05 +0200 Subject: [PATCH 1/3] Rename AddFileInWorktree to AddFileInWorktreeOrSubmodule It works for submodules too. Also, pass file name and file content explicitly; the existing tests don't care about these, but when writing tests that do, it makes them easier to understand. --- pkg/integration/components/shell.go | 6 +++--- pkg/integration/tests/worktree/force_remove_worktree.go | 2 +- .../tests/worktree/remove_worktree_from_branch.go | 2 +- pkg/integration/tests/worktree/reset_window_tabs.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index 4c833f2ab..faa6fff13 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -428,11 +428,11 @@ func (self *Shell) AddWorktreeCheckout(base string, path string) *Shell { }) } -func (self *Shell) AddFileInWorktree(worktreePath string) *Shell { - self.CreateFile(filepath.Join(worktreePath, "content"), "content") +func (self *Shell) AddFileInWorktreeOrSubmodule(worktreePath string, filePath string, content string) *Shell { + self.CreateFile(filepath.Join(worktreePath, filePath), content) self.RunCommand([]string{ - "git", "-C", worktreePath, "add", "content", + "git", "-C", worktreePath, "add", filePath, }) return self diff --git a/pkg/integration/tests/worktree/force_remove_worktree.go b/pkg/integration/tests/worktree/force_remove_worktree.go index 23d0b9a88..0d3bf90cf 100644 --- a/pkg/integration/tests/worktree/force_remove_worktree.go +++ b/pkg/integration/tests/worktree/force_remove_worktree.go @@ -17,7 +17,7 @@ var ForceRemoveWorktree = NewIntegrationTest(NewIntegrationTestArgs{ shell.EmptyCommit("commit 2") shell.EmptyCommit("commit 3") shell.AddWorktree("mybranch", "../linked-worktree", "newbranch") - shell.AddFileInWorktree("../linked-worktree") + shell.AddFileInWorktreeOrSubmodule("../linked-worktree", "file", "content") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Worktrees(). diff --git a/pkg/integration/tests/worktree/remove_worktree_from_branch.go b/pkg/integration/tests/worktree/remove_worktree_from_branch.go index 7ef9d0ae9..f0b8de4e6 100644 --- a/pkg/integration/tests/worktree/remove_worktree_from_branch.go +++ b/pkg/integration/tests/worktree/remove_worktree_from_branch.go @@ -17,7 +17,7 @@ var RemoveWorktreeFromBranch = NewIntegrationTest(NewIntegrationTestArgs{ shell.EmptyCommit("commit 2") shell.EmptyCommit("commit 3") shell.AddWorktree("mybranch", "../linked-worktree", "newbranch") - shell.AddFileInWorktree("../linked-worktree") + shell.AddFileInWorktreeOrSubmodule("../linked-worktree", "file", "content") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Branches(). diff --git a/pkg/integration/tests/worktree/reset_window_tabs.go b/pkg/integration/tests/worktree/reset_window_tabs.go index c24373be8..12c4526eb 100644 --- a/pkg/integration/tests/worktree/reset_window_tabs.go +++ b/pkg/integration/tests/worktree/reset_window_tabs.go @@ -24,7 +24,7 @@ var ResetWindowTabs = NewIntegrationTest(NewIntegrationTestArgs{ shell.EmptyCommit("commit 2") shell.EmptyCommit("commit 3") shell.AddWorktree("mybranch", "../linked-worktree", "newbranch") - shell.AddFileInWorktree("../linked-worktree") + shell.AddFileInWorktreeOrSubmodule("../linked-worktree", "file", "content") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { // focus the remotes tab i.e. the second tab in the branches window From 0904bf9969804bbe4c380fbac0889400e5676cc4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 5 Oct 2025 14:52:28 +0200 Subject: [PATCH 2/3] Add test demonstrating the problem When dropping changes to the submodule, we expect it to get rolled back to the previous version; however, it is removed entirely instead. --- pkg/integration/components/shell.go | 14 +++++ .../tests/commit/discard_submodule_changes.go | 57 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 72 insertions(+) create mode 100644 pkg/integration/tests/commit/discard_submodule_changes.go diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index faa6fff13..b89e08d2b 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -170,6 +170,10 @@ func (self *Shell) Commit(message string) *Shell { return self.RunCommand([]string{"git", "commit", "-m", message}) } +func (self *Shell) CommitInWorktreeOrSubmodule(worktreePath string, message string) *Shell { + return self.RunCommand([]string{"git", "-C", worktreePath, "commit", "-m", message}) +} + func (self *Shell) EmptyCommit(message string) *Shell { return self.RunCommand([]string{"git", "commit", "--allow-empty", "-m", message}) } @@ -438,6 +442,16 @@ func (self *Shell) AddFileInWorktreeOrSubmodule(worktreePath string, filePath st return self } +func (self *Shell) UpdateFileInWorktreeOrSubmodule(worktreePath string, filePath string, content string) *Shell { + self.UpdateFile(filepath.Join(worktreePath, filePath), content) + + self.RunCommand([]string{ + "git", "-C", worktreePath, "add", filePath, + }) + + return self +} + func (self *Shell) MakeExecutable(path string) *Shell { // 0755 sets the executable permission for owner, and read/execute permissions for group and others err := os.Chmod(filepath.Join(self.dir, path), 0o755) diff --git a/pkg/integration/tests/commit/discard_submodule_changes.go b/pkg/integration/tests/commit/discard_submodule_changes.go new file mode 100644 index 000000000..df54dcd87 --- /dev/null +++ b/pkg/integration/tests/commit/discard_submodule_changes.go @@ -0,0 +1,57 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DiscardSubmoduleChanges = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Discarding changes to a submodule from an old commit.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("Initial commit") + shell.CloneIntoSubmodule("submodule", "submodule") + shell.Commit("Add submodule") + + shell.AddFileInWorktreeOrSubmodule("submodule", "file", "content") + shell.CommitInWorktreeOrSubmodule("submodule", "add file in submodule") + shell.GitAdd("submodule") + shell.Commit("Update submodule") + + shell.UpdateFileInWorktreeOrSubmodule("submodule", "file", "changed content") + shell.CommitInWorktreeOrSubmodule("submodule", "change file in submodule") + shell.GitAdd("submodule") + shell.Commit("Update submodule again") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("Update submodule again").IsSelected(), + Contains("Update submodule"), + Contains("Add submodule"), + Contains("Initial commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("M submodule").IsSelected(), + ). + Press(keys.Universal.Remove) + + t.ExpectPopup().Confirmation(). + Title(Equals("Discard file changes")). + Content(Contains("Are you sure you want to remove changes to the selected file(s) from this commit?")). + Confirm() + + t.Shell().RunCommand([]string{"git", "submodule", "update"}) + /* EXPECTED: + t.FileSystem().FileContent("submodule/file", Equals("content")) + ACTUAL: */ + t.FileSystem().PathNotPresent("submodule/file") + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6c36d9172..1abba20f6 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -119,6 +119,7 @@ var tests = []*components.IntegrationTest{ commit.CreateTag, commit.DisableCopyCommitMessageBody, commit.DiscardOldFileChanges, + commit.DiscardSubmoduleChanges, commit.DoNotShowBranchMarkerForHeadCommit, commit.FailHooksThenCommitNoHooks, commit.FindBaseCommitForFixup, From c1e52fc807689a12c2f05b1304391d6c95782aae Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 5 Oct 2025 10:51:55 +0200 Subject: [PATCH 3/3] 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 a49749084..152c88bbc 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -521,10 +521,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") }, })