Skip to content

Commit 30169bd

Browse files
authored
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.
2 parents 8e00bbd + c1e52fc commit 30169bd

File tree

8 files changed

+87
-11
lines changed

8 files changed

+87
-11
lines changed

pkg/commands/git_commands/rebase.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -521,10 +521,17 @@ func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, comm
521521
}
522522

523523
for _, filePath := range filePaths {
524-
// check if file exists in previous commit (this command returns an error if the file doesn't exist)
525-
cmdArgs := NewGitCmd("cat-file").Arg("-e", "HEAD^:"+filePath).ToArgv()
526-
527-
if err := self.cmd.New(cmdArgs).Run(); err != nil {
524+
doesFileExistInPreviousCommit := false
525+
if commitIndex < len(commits)-1 {
526+
// check if file exists in previous commit (this command returns an empty string if the file doesn't exist)
527+
cmdArgs := NewGitCmd("ls-tree").Arg("--name-only", "HEAD^", "--", filePath).ToArgv()
528+
output, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
529+
if err != nil {
530+
return err
531+
}
532+
doesFileExistInPreviousCommit = strings.TrimRight(output, "\n") == filePath
533+
}
534+
if !doesFileExistInPreviousCommit {
528535
if err := self.os.Remove(filePath); err != nil {
529536
return err
530537
}

pkg/commands/git_commands/rebase_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
139139
fileName: []string{"test999.txt"},
140140
runner: oscommands.NewFakeRunner(t).
141141
ExpectGitArgs([]string{"rebase", "--interactive", "--autostash", "--keep-empty", "--no-autosquash", "--rebase-merges", "abcdef"}, "", nil).
142-
ExpectGitArgs([]string{"cat-file", "-e", "HEAD^:test999.txt"}, "", nil).
142+
ExpectGitArgs([]string{"ls-tree", "--name-only", "HEAD^", "--", "test999.txt"}, "test999.txt\n", nil).
143143
ExpectGitArgs([]string{"checkout", "HEAD^", "--", "test999.txt"}, "", nil).
144144
ExpectGitArgs([]string{"commit", "--amend", "--no-edit", "--allow-empty", "--allow-empty-message"}, "", nil).
145145
ExpectGitArgs([]string{"rebase", "--continue"}, "", nil),

pkg/integration/components/shell.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ func (self *Shell) Commit(message string) *Shell {
170170
return self.RunCommand([]string{"git", "commit", "-m", message})
171171
}
172172

173+
func (self *Shell) CommitInWorktreeOrSubmodule(worktreePath string, message string) *Shell {
174+
return self.RunCommand([]string{"git", "-C", worktreePath, "commit", "-m", message})
175+
}
176+
173177
func (self *Shell) EmptyCommit(message string) *Shell {
174178
return self.RunCommand([]string{"git", "commit", "--allow-empty", "-m", message})
175179
}
@@ -428,11 +432,21 @@ func (self *Shell) AddWorktreeCheckout(base string, path string) *Shell {
428432
})
429433
}
430434

431-
func (self *Shell) AddFileInWorktree(worktreePath string) *Shell {
432-
self.CreateFile(filepath.Join(worktreePath, "content"), "content")
435+
func (self *Shell) AddFileInWorktreeOrSubmodule(worktreePath string, filePath string, content string) *Shell {
436+
self.CreateFile(filepath.Join(worktreePath, filePath), content)
437+
438+
self.RunCommand([]string{
439+
"git", "-C", worktreePath, "add", filePath,
440+
})
441+
442+
return self
443+
}
444+
445+
func (self *Shell) UpdateFileInWorktreeOrSubmodule(worktreePath string, filePath string, content string) *Shell {
446+
self.UpdateFile(filepath.Join(worktreePath, filePath), content)
433447

434448
self.RunCommand([]string{
435-
"git", "-C", worktreePath, "add", "content",
449+
"git", "-C", worktreePath, "add", filePath,
436450
})
437451

438452
return self
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package commit
2+
3+
import (
4+
"github.com/jesseduffield/lazygit/pkg/config"
5+
. "github.com/jesseduffield/lazygit/pkg/integration/components"
6+
)
7+
8+
var DiscardSubmoduleChanges = NewIntegrationTest(NewIntegrationTestArgs{
9+
Description: "Discarding changes to a submodule from an old commit.",
10+
ExtraCmdArgs: []string{},
11+
Skip: false,
12+
SetupConfig: func(config *config.AppConfig) {},
13+
SetupRepo: func(shell *Shell) {
14+
shell.EmptyCommit("Initial commit")
15+
shell.CloneIntoSubmodule("submodule", "submodule")
16+
shell.Commit("Add submodule")
17+
18+
shell.AddFileInWorktreeOrSubmodule("submodule", "file", "content")
19+
shell.CommitInWorktreeOrSubmodule("submodule", "add file in submodule")
20+
shell.GitAdd("submodule")
21+
shell.Commit("Update submodule")
22+
23+
shell.UpdateFileInWorktreeOrSubmodule("submodule", "file", "changed content")
24+
shell.CommitInWorktreeOrSubmodule("submodule", "change file in submodule")
25+
shell.GitAdd("submodule")
26+
shell.Commit("Update submodule again")
27+
},
28+
Run: func(t *TestDriver, keys config.KeybindingConfig) {
29+
t.Views().Commits().
30+
Focus().
31+
Lines(
32+
Contains("Update submodule again").IsSelected(),
33+
Contains("Update submodule"),
34+
Contains("Add submodule"),
35+
Contains("Initial commit"),
36+
).
37+
PressEnter()
38+
39+
t.Views().CommitFiles().
40+
IsFocused().
41+
Lines(
42+
Equals("M submodule").IsSelected(),
43+
).
44+
Press(keys.Universal.Remove)
45+
46+
t.ExpectPopup().Confirmation().
47+
Title(Equals("Discard file changes")).
48+
Content(Contains("Are you sure you want to remove changes to the selected file(s) from this commit?")).
49+
Confirm()
50+
51+
t.Shell().RunCommand([]string{"git", "submodule", "update"})
52+
t.FileSystem().FileContent("submodule/file", Equals("content"))
53+
},
54+
})

pkg/integration/tests/test_list.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ var tests = []*components.IntegrationTest{
119119
commit.CreateTag,
120120
commit.DisableCopyCommitMessageBody,
121121
commit.DiscardOldFileChanges,
122+
commit.DiscardSubmoduleChanges,
122123
commit.DoNotShowBranchMarkerForHeadCommit,
123124
commit.FailHooksThenCommitNoHooks,
124125
commit.FindBaseCommitForFixup,

pkg/integration/tests/worktree/force_remove_worktree.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ var ForceRemoveWorktree = NewIntegrationTest(NewIntegrationTestArgs{
1717
shell.EmptyCommit("commit 2")
1818
shell.EmptyCommit("commit 3")
1919
shell.AddWorktree("mybranch", "../linked-worktree", "newbranch")
20-
shell.AddFileInWorktree("../linked-worktree")
20+
shell.AddFileInWorktreeOrSubmodule("../linked-worktree", "file", "content")
2121
},
2222
Run: func(t *TestDriver, keys config.KeybindingConfig) {
2323
t.Views().Worktrees().

pkg/integration/tests/worktree/remove_worktree_from_branch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ var RemoveWorktreeFromBranch = NewIntegrationTest(NewIntegrationTestArgs{
1717
shell.EmptyCommit("commit 2")
1818
shell.EmptyCommit("commit 3")
1919
shell.AddWorktree("mybranch", "../linked-worktree", "newbranch")
20-
shell.AddFileInWorktree("../linked-worktree")
20+
shell.AddFileInWorktreeOrSubmodule("../linked-worktree", "file", "content")
2121
},
2222
Run: func(t *TestDriver, keys config.KeybindingConfig) {
2323
t.Views().Branches().

pkg/integration/tests/worktree/reset_window_tabs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ var ResetWindowTabs = NewIntegrationTest(NewIntegrationTestArgs{
2424
shell.EmptyCommit("commit 2")
2525
shell.EmptyCommit("commit 3")
2626
shell.AddWorktree("mybranch", "../linked-worktree", "newbranch")
27-
shell.AddFileInWorktree("../linked-worktree")
27+
shell.AddFileInWorktreeOrSubmodule("../linked-worktree", "file", "content")
2828
},
2929
Run: func(t *TestDriver, keys config.KeybindingConfig) {
3030
// focus the remotes tab i.e. the second tab in the branches window

0 commit comments

Comments
 (0)