Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] callback func onDirExist does not effect #131

Open
MrKrisYu opened this issue Dec 26, 2023 · 1 comment
Open

[Bug] callback func onDirExist does not effect #131

MrKrisYu opened this issue Dec 26, 2023 · 1 comment

Comments

@MrKrisYu
Copy link

[occurrence]

when testing the conflict with the same directory, the onDirExists callback function doest not effect, code as following:

func TestRename(t *testing.T) {
	targetDir := "./test/dir1"
	newDir := "./test/dir2/dir1"
	err := cp.Copy(targetDir, newDir, cp.Options{
		OnDirExists: func(src, dest string) cp.DirExistsAction {
			fmt.Printf("Directory Exists, do nothing.\n")
			return cp.Untouchable
		},
		PreserveTimes: true,
		PreserveOwner: true,
	})
	if err != nil {
		fmt.Printf("Failed to rename: %s \n", err.Error())
		return
	}
	fmt.Printf("Success to rename \n")
}

working directory tree is as following:
image

[personal opinio]

I 'm confused about the conditions under which the onDirExist callback function is executed.

func onDirExists(opt Options, srcdir, destdir string) (bool, error) {
	_, err := os.Stat(destdir)
	if err == nil && opt.OnDirExists != nil && destdir != opt.intent.dest {
		switch opt.OnDirExists(srcdir, destdir) {
		case Replace:
			if err := os.RemoveAll(destdir); err != nil {
				return false, err
			}
		case Untouchable:
			return true, nil
		} // case "Merge" is default behaviour. Go through.
	} else if err != nil && !os.IsNotExist(err) {
		return true, err // Unwelcome error type...!
	}
	return false, nil
}

we know that variable destdir is always equal to opt.intent.dest, if we use default options as following:

// part of default options
func getDefaultOptions(src, dest string) Options {
	return Options{
		// ignore the extra code .....
		intent:            intent{src, dest, nil, nil},
	}
}


// part of merge options 
func assureOptions(src, dest string, opts ...Options) Options {
	defopt := getDefaultOptions(src, dest)
	// ignore the extra code .....
	opts[0].intent.src = defopt.intent.src
	opts[0].intent.dest = defopt.intent.dest
	return opts[0]
}

so, how is it possible that the switch-case code of onDirExists callback func executes.

@otiai10
Copy link
Owner

otiai10 commented Dec 26, 2023

that's expected behavior, though I'm happy to discuss.

Since you're trying to copy "dir1" to "dir2/dir1" intentionally, you can check if the destination root already exists or not by yourself.

That's why it works even you return "Untouchable".

On this case, "Untouchable" means it doesn't do anything when some child directories exist under the root destination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants