diff --git a/internal/chezmoi/sourcestate.go b/internal/chezmoi/sourcestate.go index 48c0d9d26f9..343086e22e2 100644 --- a/internal/chezmoi/sourcestate.go +++ b/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 diff --git a/internal/cmd/applycmd_test.go b/internal/cmd/applycmd_test.go index 94041d531ba..c65be9c0497 100644 --- a/internal/cmd/applycmd_test.go +++ b/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(), + ), + ) + }) +} diff --git a/internal/cmd/testdata/scripts/issue3703.txtar b/internal/cmd/testdata/scripts/issue3703.txtar new file mode 100644 index 00000000000..313f283f64c --- /dev/null +++ b/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