Fix dropping submodule changes from a commit (#4937)

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.

Fixes #4932.

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
 			}

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),

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})
 }
@@ -428,11 +432,21 @@ 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
+}
+
+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

pkg/integration/tests/commit/discard_submodule_changes.go

@@ -0,0 +1,54 @@
+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"})
+		t.FileSystem().FileContent("submodule/file", Equals("content"))
+	},
+})

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,

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().

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().

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