Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions copier/copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ func Mkdir(root string, directory string, options MkdirOptions) error {
type RemoveOptions struct {
All bool // if Directory is a directory, remove its contents as well
AllowNotFound bool // don't return an error if the item is already not present
AllowWildcard bool // expand the path as a glob pattern, removing each match. All must be set to remove matched non-empty directories
}

// Remove removes the specified directory or item, traversing any intermediate
Expand Down Expand Up @@ -2297,20 +2298,30 @@ func copierHandlerRemove(req request) *response {
errorResponse := func(fmtspec string, args ...any) *response {
return &response{Error: fmt.Sprintf(fmtspec, args...), Remove: removeResponse{}}
}
resolvedTarget, err := resolvePath(req.Root, req.Directory, false, nil)
if err != nil {
return errorResponse("copier: remove: %v", err)
}
if req.RemoveOptions.All {
err = os.RemoveAll(resolvedTarget)
} else {
err = os.Remove(resolvedTarget)
if req.RemoveOptions.AllowNotFound && errors.Is(err, os.ErrNotExist) {
err = nil
targets := []string{req.Directory}
if req.RemoveOptions.AllowWildcard {
var err error
targets, err = extendedGlob(req.Directory)
if err != nil {
return errorResponse("copier: remove: glob %q: %v", req.Directory, err)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to be catching cases where the returned set of targets has len() == 0 when AllowNotFound isn't set, and returning an error, as Glob() will only return an error for an invalid pattern.
AllowWildcard doesn't always mean that there's a wildcard being used, and I don't know that we want to effectively ignore not-found errors when it's set.

}
if err != nil {
return errorResponse("copier: remove %q: %v", req.Directory, err)
for _, target := range targets {
resolvedTarget, err := resolvePath(req.Root, target, false, nil)
if err != nil {
return errorResponse("copier: remove: %v", err)
}
if req.RemoveOptions.All {
err = os.RemoveAll(resolvedTarget)
} else {
err = os.Remove(resolvedTarget)
if req.RemoveOptions.AllowNotFound && errors.Is(err, os.ErrNotExist) {
err = nil
}
}
if err != nil {
return errorResponse("copier: remove %q: %v", target, err)
}
}
return &response{Error: "", Remove: removeResponse{}}
}
Expand Down
70 changes: 69 additions & 1 deletion copier/copier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,7 @@ func testRemove(t *testing.T) {
remove string
all bool
allowNotFound bool
allowWildcard bool
fail bool
removed []string
}
Expand Down Expand Up @@ -2095,6 +2096,73 @@ func testRemove(t *testing.T) {
all: true,
removed: []string{"subdir-a/subdir-e", "subdir-a/subdir-e/subdir-f"},
},
{
name: "wildcard-files",
remove: "subdir-a/file-*",
allowWildcard: true,
removed: []string{"subdir-a/file-a", "subdir-a/file-b"},
},
{
name: "wildcard-all-in-dir",
remove: "subdir-a/subdir-b/*",
allowWildcard: true,
all: true,
removed: []string{
"subdir-a/subdir-b/subdir-c",
"subdir-a/subdir-b/subdir-c/parent",
"subdir-a/subdir-b/subdir-c/link-b",
"subdir-a/subdir-b/subdir-c/root",
},
},
{
name: "wildcard-no-match",
remove: "subdir-a/nonexistent-*",
allowWildcard: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without allowNotFound and all set (i.e., both false), should an error be returned, causing this test to fail because fail wasn't set?

},
{
name: "wildcard-allow-not-found",
remove: "subdir-a/file-*",
allowWildcard: true,
allowNotFound: true,
removed: []string{"subdir-a/file-a", "subdir-a/file-b"},
},
{
name: "wildcard-dir-without-all",
remove: "subdir-a/subdir-*",
allowWildcard: true,
all: false,
fail: true,
},
{
name: "wildcard-empty-dir",
remove: "subdir-a/subdir-d",
allowWildcard: true,
all: false,
removed: []string{"subdir-a/subdir-d"},
},
{
name: "wildcard-literal-path",
remove: "subdir-a/file-a",
allowWildcard: true,
removed: []string{"subdir-a/file-a"},
},
{
name: "wildcard-literal-missing",
remove: "subdir-a/file-nonexistent",
allowWildcard: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without allowNotFound and all set (i.e., both false), should an error be returned, causing this test to fail because fail wasn't set?

},
{
name: "wildcard-char-class",
remove: "subdir-a/file-[ab]",
allowWildcard: true,
removed: []string{"subdir-a/file-a", "subdir-a/file-b"},
},
{
name: "wildcard-question-mark",
remove: "subdir-a/file-?",
allowWildcard: true,
removed: []string{"subdir-a/file-a", "subdir-a/file-b"},
},
},
},
}
Expand All @@ -2105,7 +2173,7 @@ func testRemove(t *testing.T) {
dir, err := makeContextFromArchive(t, makeArchive(testArchives[i].headers, nil), "")
require.NoErrorf(t, err, "error creating context from archive %q, topdir=%q", testArchives[i].name, "")
root := dir
options := RemoveOptions{All: testCase.all, AllowNotFound: testCase.allowNotFound}
options := RemoveOptions{All: testCase.all, AllowNotFound: testCase.allowNotFound, AllowWildcard: testCase.allowWildcard}
beforeNames := make(map[string]struct{})
err = filepath.WalkDir(dir, func(path string, _ fs.DirEntry, err error) error {
if err != nil {
Expand Down
Loading