fix: Remove unmanaged files from exact_ directories containing extern…

…al files

internal/chezmoi/sourcestate.go

@@ -1108,6 +1108,14 @@ func (s *SourceState) Read(ctx context.Context, options *ReadOptions) error {
 		allSourceStateEntries[targetRelPath] = append(allSourceStateEntries[targetRelPath], sourceStateEntry)
 	}
 
+	// Where there are multiple SourceStateEntries for a single target, replace
+	// them with a single canonical SourceStateEntry if possible.
+	for targetRelPath, sourceStateEntries := range allSourceStateEntries {
+		if sourceStateEntry, ok := canonicalSourceStateEntry(sourceStateEntries); ok {
+			allSourceStateEntries[targetRelPath] = []SourceStateEntry{sourceStateEntry}
+		}
+	}
+
 	// Generate SourceStateRemoves for exact directories.
 	for targetRelPath, sourceStateEntries := range allSourceStateEntries {
 		if len(sourceStateEntries) != 1 {
@@ -1231,11 +1239,6 @@ func (s *SourceState) Read(ctx context.Context, options *ReadOptions) error {
 			continue
 		}
 
-		// Allow duplicate equivalent source entries for directories.
-		if allEquivalentDirs(sourceStateEntries) {
-			continue
-		}
-
 		origins := make([]string, 0, len(sourceStateEntries))
 		for _, sourceStateEntry := range sourceStateEntries {
 			origins = append(origins, sourceStateEntry.Origin().OriginString())
@@ -2793,9 +2796,12 @@ func (e *External) OriginString() string {
 	return e.URL + " defined in " + e.sourceAbsPath.String()
 }
 
-// allEquivalentDirs returns if sourceStateEntries are all equivalent
-// directories.
-func allEquivalentDirs(sourceStateEntries []SourceStateEntry) bool {
+// canonicalSourceStateEntry returns the canonical SourceStateEntry for the
+// given sourceStateEntries.
+//
+// This only applies to directories, where SourceStateImplicitDirs are
+// considered equivalent to all SourceStateDirs.
+func canonicalSourceStateEntry(sourceStateEntries []SourceStateEntry) (SourceStateEntry, bool) {
 	// Find all directories to check for equivalence.
 	var firstSourceStateDir *SourceStateDir
 	sourceStateDirs := make([]SourceStateEntry, 0, len(sourceStateEntries))
@@ -2807,29 +2813,37 @@ func allEquivalentDirs(sourceStateEntries []SourceStateEntry) bool {
 		case *SourceStateImplicitDir:
 			sourceStateDirs = append(sourceStateDirs, sourceStateEntry)
 		default:
-			return false
+			return nil, false
 		}
 	}
 
-	// If there are no SourceStateDirs then there are no equivalent directories.
-	if len(sourceStateDirs) == 0 {
-		return false
-	}
-
-	// Check for equivalence.
-	for _, sourceStateDir := range sourceStateDirs {
-		switch sourceStateDir := sourceStateDir.(type) {
-		case *SourceStateDir:
-			if sourceStateDir.Attr != firstSourceStateDir.Attr {
-				return false
+	switch len(sourceStateDirs) {
+	case 0:
+		// If there are no SourceStateDirs then there are no equivalent directories.
+		return nil, false
+	case 1:
+		return sourceStateDirs[0], true
+	default:
+		// Check for equivalence.
+		for _, sourceStateDir := range sourceStateDirs {
+			switch sourceStateDir := sourceStateDir.(type) {
+			case *SourceStateDir:
+				if sourceStateDir.Attr != firstSourceStateDir.Attr {
+					return nil, false
+				}
+			case *SourceStateImplicitDir:
+				// SourceStateImplicitDirs are considered equivalent to all other
+				// directories.
 			}
-		case *SourceStateImplicitDir:
-			// SourceStateImplicitDirs are considered equivalent to all other
-			// directories.
 		}
+		// If all directories are equivalent then return the first real
+		// *SourceStateDir, if it exists.
+		if firstSourceStateDir != nil {
+			return firstSourceStateDir, true
+		}
+		// Otherwise, return the first entry which is a *SourceStateImplicitDir.
+		return sourceStateDirs[0], true
 	}
-
-	return true
 }
 
 // isAppleDoubleFile returns true if the file looks like and has the

internal/cmd/applycmd_test.go

@@ -2,6 +2,8 @@ package cmd
 
 import (
 	"io/fs"
+	"net/http"
+	"net/http/httptest"
 	"path/filepath"
 	"testing"
 
@@ -264,3 +266,39 @@ func TestIssue3216(t *testing.T) {
 		assert.NoError(t, newTestConfig(t, fileSystem).execute([]string{"apply"}))
 	})
 }
+
+func TestIssue3703(t *testing.T) {
+	httpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte("contents of file\n"))
+		assert.NoError(t, err)
+	}))
+	defer httpServer.Close()
+
+	chezmoitest.WithTestFS(t, map[string]any{
+		"/home/user": map[string]any{
+			".local": map[string]any{
+				"bin": map[string]any{
+					"unmanaged": "",
+				},
+				"share/chezmoi": map[string]any{
+					".chezmoiexternal.toml.tmpl": chezmoitest.JoinLines(
+						`[".local/bin/file"]`,
+						`    type = "file"`,
+						`    url = "`+httpServer.URL+`/file"`,
+					),
+					"dot_local/exact_bin/.keep": "",
+				},
+			},
+		},
+	}, func(fileSystem vfs.FS) {
+		assert.NoError(t, newTestConfig(t, fileSystem).execute([]string{"apply"}))
+		vfst.RunTests(t, fileSystem, ".local/bin",
+			vfst.TestPath("/home/user/.local/bin/file",
+				vfst.TestContentsString("contents of file\n"),
+			),
+			vfst.TestPath("/home/user/.local/bin/unmanaged",
+				vfst.TestDoesNotExist(),
+			),
+		)
+	})
+}

internal/cmd/testdata/scripts/issue3703.txtar

@@ -0,0 +1,16 @@
+httpd www
+
+# test that chezmoi apply removes unmanaged files from an exact_ directory containing an external
+exists $HOME/.local/bin/unmanaged
+exec chezmoi apply --debug
+cmp $HOME/.local/bin/file www/file
+! exists $HOME/.local/bin/unmanaged
+
+-- home/user/.local/bin/unmanaged --
+-- home/user/.local/share/chezmoi/.chezmoiexternal.toml.tmpl --
+[".local/bin/file"]
+    type = "file"
+    url = "{{ env "HTTPD_URL" }}/file"
+-- home/user/.local/share/chezmoi/dot_local/exact_bin/.keep --
+-- www/file --
+# contents of file