Skip to content
Draft
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
13 changes: 13 additions & 0 deletions all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"testing"
"time"

"github.com/otiai10/copy/plugins/copyonwrite"
. "github.com/otiai10/mint"
)

Expand Down Expand Up @@ -476,3 +477,15 @@ func TestOptions_RenameDestination(t *testing.T) {
_, err = os.Stat("test/data.copy/case20/foo/control.txt")
Expect(t, err).ToBe(nil)
}

func TestOptions_FileCopyFunc_OnMacOS(t *testing.T) {
if runtime.GOOS != "darwin" {
t.Skip("This test is only for macOS")
}
// You can use your own file copy function
opt := Options{
FileCopyFunc: copyonwrite.CopyOnWrite,
}
err := Copy("test/data/case21", "test/data.copy/case21", opt)
Expect(t, err).ToBe(nil)
}
63 changes: 41 additions & 22 deletions copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,46 @@ func copyNextOrSkip(src, dest string, info os.FileInfo, opt Options) error {
// and file permission.
func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) {

if err = os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil {
return
}

if opt.FileCopyFunc != nil {
if err = opt.FileCopyFunc(src, dest); err != nil {
return err
}
} else {
if err = fcopyByReadAndWrite(src, dest, info, opt); err != nil {
return err
}
}

chmodfunc, err := opt.PermissionControl(info, dest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question for my own reference: the point where the permissions are changed doesn't matter, as long as it happens?

Example: after file creation, or after the file is finished writing

Copy link
Owner Author

Choose a reason for hiding this comment

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

I doesn't matter

if err != nil {
return err
}
chmodfunc(&err)
if os.IsNotExist(err) {
// See https://github.com/otiai10/copy/issues/72
// Gracefully handle files/symlinks/dirs deleted while copy
return nil
}

if opt.PreserveOwner {
if err := preserveOwner(src, dest, info); err != nil {
return err
}
}
if opt.PreserveTimes {
if err := preserveTimes(info, dest); err != nil {
return err
}
}

return
}

func fcopyByReadAndWrite(src, dest string, info os.FileInfo, opt Options) (err error) {
var readcloser io.ReadCloser
if opt.FS != nil {
readcloser, err = opt.FS.Open(src)
Expand All @@ -101,22 +141,12 @@ func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) {
}
defer fclose(readcloser, &err)

if err = os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil {
return
}

f, err := os.Create(dest)
if err != nil {
return
}
defer fclose(f, &err)

chmodfunc, err := opt.PermissionControl(info, dest)
if err != nil {
return err
}
chmodfunc(&err)

var buf []byte = nil
var w io.Writer = f
var r io.Reader = readcloser
Expand All @@ -141,18 +171,7 @@ func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) {
err = f.Sync()
}

if opt.PreserveOwner {
if err := preserveOwner(src, dest, info); err != nil {
return err
}
}
if opt.PreserveTimes {
if err := preserveTimes(info, dest); err != nil {
return err
}
}

return
return err
}

// dcopy is for a directory,
Expand Down
5 changes: 5 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ type Options struct {
// e.g., You can use embed.FS to copy files from embedded filesystem.
FS fs.FS

// FileCopyFunc is a function to copy an actual file.
// If nil, it uses the default file copying function with `io.Copy`.
// e.g., You can use `CopyOnWrite` to copy files with copy-on-write.
FileCopyFunc func(src, dest string) error
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, that's a clever way of handling it.

A few questions, though:

  • How does this work when the source is a fs.FS. If it's not supposed to, how can that be communicated to the user? Documentation? Error?

  • Similarly, what about also using the Sync option or WrapReader option?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better not to give the user full control over the function but instead let them pick between pre-made options.


// NumOfWorkers represents the number of workers used for
// concurrent copying contents of directories.
// If 0 or 1, it does not use goroutine for copying directories.
Expand Down
3 changes: 3 additions & 0 deletions plugins/copyonwrite/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Support CopyOnWrite of macOS

// TODO: Write more description
9 changes: 9 additions & 0 deletions plugins/copyonwrite/copyonwrite_darwin copy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build darwin

package copyonwrite

import "golang.org/x/sys/unix"

func CopyOnWrite(src, dst string) error {
return unix.Clonefile(src, dst, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be able to gracefully handle EEXIST in order to be compatible with the read and write copy.

Read and write copy will just overwrite the file, but clonefile will fail if the destination already exists.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will also fail if:

  • The filesystem doesn't support copy-on-write.
  • The source file is on a different filesystem than the destination.

You can check for both of these ahead of time for every file, but it's more platform-specific logic and overhead. It wouldn't be possible to only do a single check on the original source and destination given to Copy either, since a directory can contain mount points for other filesystems.

I think falling back to read and write copy on error is going to be the most compatible approach.

}
9 changes: 9 additions & 0 deletions plugins/copyonwrite/copyonwrite_x.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build !darwin

package copyonwrite

import "errors"

func CopyOnWrite(src, dst string) error {
return errors.New("copy-on-write is not supported on this platform")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to fall back to read and write copy here. Having it fail with an error is going to cause problems if people write code on one platform but never test it on any others.

In this proposal, there also isn't any way to check if the platform does support copy-on-write.

}
1 change: 1 addition & 0 deletions test/data/case21/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# CopyOnWrite plugin