From e2899b81badce39a90a306559bad2c06b8e25d4f Mon Sep 17 00:00:00 2001
From: Tom Payne <twpayne@gmail.com>
Date: Fri, 17 Dec 2021 21:37:48 +0100
Subject: [PATCH] feat: Improve inconsistent state error with removes

---
 internal/chezmoi/sourcestate.go             | 10 +++++++--
 internal/chezmoi/sourcestate_test.go        |  3 +++
 internal/chezmoi/sourcestateentry.go        |  3 ++-
 internal/cmd/testdata/scripts/edgecases.txt | 23 +++++++++++++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/internal/chezmoi/sourcestate.go b/internal/chezmoi/sourcestate.go
index 1c1fa2b2ab5..fb2a415f26d 100644
--- a/internal/chezmoi/sourcestate.go
+++ b/internal/chezmoi/sourcestate.go
@@ -411,7 +411,10 @@ DESTABSPATH:
 				return nil
 			}
 			sourceRelPath := sourceStateEntry.SourceRelPath()
-			sourceRoot.Set(sourceRelPath.RelPath(), &SourceStateRemove{})
+			sourceRoot.Set(sourceRelPath.RelPath(), &SourceStateRemove{
+				sourceRelPath: sourceRelPath,
+				targetRelPath: targetRelPath,
+			})
 			update := sourceUpdate{
 				destAbsPath: s.destDirAbsPath.Join(targetRelPath),
 				entryState: &EntryState{
@@ -784,6 +787,7 @@ func (s *SourceState) Read(ctx context.Context, options *ReadOptions) error {
 			for _, match := range matches {
 				targetRelPath := targetParentRelPath.JoinString(match)
 				sourceStateEntry := &SourceStateRemove{
+					sourceRelPath: sourceRelPath,
 					targetRelPath: targetRelPath,
 				}
 				allSourceStateEntries[targetRelPath] = append(allSourceStateEntries[targetRelPath], sourceStateEntry)
@@ -892,7 +896,8 @@ func (s *SourceState) Read(ctx context.Context, options *ReadOptions) error {
 			continue
 		}
 
-		switch sourceStateDir, ok := sourceStateEntries[0].(*SourceStateDir); {
+		sourceStateDir, ok := sourceStateEntries[0].(*SourceStateDir)
+		switch {
 		case !ok:
 			continue
 		case !sourceStateDir.Attr.Exact:
@@ -914,6 +919,7 @@ func (s *SourceState) Read(ctx context.Context, options *ReadOptions) error {
 					continue
 				}
 				allSourceStateEntries[destEntryRelPath] = append(allSourceStateEntries[destEntryRelPath], &SourceStateRemove{
+					sourceRelPath: sourceStateDir.sourceRelPath,
 					targetRelPath: destEntryRelPath,
 				})
 			}
diff --git a/internal/chezmoi/sourcestate_test.go b/internal/chezmoi/sourcestate_test.go
index 98b47123d5e..d31d270bc51 100644
--- a/internal/chezmoi/sourcestate_test.go
+++ b/internal/chezmoi/sourcestate_test.go
@@ -1141,6 +1141,7 @@ func TestSourceStateRead(t *testing.T) {
 						},
 					},
 					NewRelPath("dir/file2"): &SourceStateRemove{
+						sourceRelPath: NewSourceRelDirPath("exact_dir"),
 						targetRelPath: NewRelPath("dir/file2"),
 					},
 				}),
@@ -1162,6 +1163,7 @@ func TestSourceStateRead(t *testing.T) {
 			expectedSourceState: NewSourceState(
 				withEntries(map[RelPath]SourceStateEntry{
 					NewRelPath("file"): &SourceStateRemove{
+						sourceRelPath: NewSourceRelPath(".chezmoiremove"),
 						targetRelPath: NewRelPath("file"),
 					},
 				}),
@@ -1182,6 +1184,7 @@ func TestSourceStateRead(t *testing.T) {
 			expectedSourceState: NewSourceState(
 				withEntries(map[RelPath]SourceStateEntry{
 					NewRelPath("file1"): &SourceStateRemove{
+						sourceRelPath: NewSourceRelPath(".chezmoiremove"),
 						targetRelPath: NewRelPath("file1"),
 					},
 				}),
diff --git a/internal/chezmoi/sourcestateentry.go b/internal/chezmoi/sourcestateentry.go
index b4e4fe1181b..0e799ddb062 100644
--- a/internal/chezmoi/sourcestateentry.go
+++ b/internal/chezmoi/sourcestateentry.go
@@ -39,6 +39,7 @@ type SourceStateFile struct {
 
 // A SourceStateRemove represents that an entry should be removed.
 type SourceStateRemove struct {
+	sourceRelPath SourceRelPath
 	targetRelPath RelPath
 }
 
@@ -146,7 +147,7 @@ func (s *SourceStateRemove) Order() ScriptOrder {
 
 // Origin returns s's origin.
 func (s *SourceStateRemove) Origin() string {
-	return ""
+	return s.sourceRelPath.String()
 }
 
 // SourceRelPath returns s's source relative path.
diff --git a/internal/cmd/testdata/scripts/edgecases.txt b/internal/cmd/testdata/scripts/edgecases.txt
index 4b6c10dc291..40a1a75e663 100644
--- a/internal/cmd/testdata/scripts/edgecases.txt
+++ b/internal/cmd/testdata/scripts/edgecases.txt
@@ -35,7 +35,30 @@ mkdir $HOME/.dir
 chezmoi add $HOME${/}.dir/
 ! exists $CHEZMOISOURCEDIR/dot_dir/dot_dir
 
+chhome home3/user
+
+# test that chezmoi reports an inconsistent state error when a file should be both removed and present
+! chezmoi apply
+stderr 'chezmoi: \.file: inconsistent state \(\.chezmoiremove, dot_file\)'
+
+skip 'FIXME make the following test pass'
+chhome home4/user
+
+# test that chezmoi reports an inconsistent state error when a file should be both removed and present, even if the file is not already present
+! chezmoi apply
+stderr 'chezmoi: \.file: inconsistent state \(\.chezmoiremove, dot_file\)'
+
 -- golden/.file --
 # contents of .file
 -- home2/user/.local/share/chezmoi/dot_file --
 # contents of .file
+-- home3/user/.file --
+# contents of .file
+-- home3/user/.local/share/chezmoi/.chezmoiremove --
+.file
+-- home3/user/.local/share/chezmoi/dot_file --
+# contents of .file
+-- home4/user/.local/share/chezmoi/.chezmoiremove --
+.file
+-- home4/user/.local/share/chezmoi/dot_file --
+# contents of .file