containerd / containerd/continuity

[Darwin] clonefile() fails if the target already exists

Open
#226 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
153
Forks
74
PR merge metrics
No merged PRs in 30d

Description

This breaks the expected `copyFile` semantics, which should truncate an existing file instead of throwing an error.

We could pro-actively remove the target:

```diff
func copyFile(target, source string) error {
+ if err := os.Remove(target); err != nil {
+ if !errors.Is(err, unix.ENOENT) {
+ return fmt.Errorf("removing copyFile target failed: %w", err)
+ }
+ }
if err := unix.Clonefile(source, target, unix.CLONE_NOFOLLOW); err != nil {
if !errors.Is(err, unix.ENOTSUP) {
return fmt.Errorf("clonefile failed: %w", err)
```

But I'm not sure if this could become a problem when `clonefile` is not supported, or when the copy operation is cross-device, and we still fall back to `copyFile`.

I'm happy to create a PR with the patch above, but would like confirmation first that this would indeed be the correct approach.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.