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

Failed to symlink if file already exists #132

Closed
starzation opened this issue Dec 30, 2023 · 13 comments
Closed

Failed to symlink if file already exists #132

starzation opened this issue Dec 30, 2023 · 13 comments

Comments

@starzation
Copy link

starzation commented Dec 30, 2023

Like the tittle said if the file is already exists (in destination folder) it will throw 'symlink error file exists'

There already a quick patch in vinegarhq/vinegar#322, Should the patch belong here or theres?

Issue (For log and more info): vinegarhq/vinegar#321
Code: vinegarhq/vinegar/internal/dirs/copy.go

@otiai10
Copy link
Owner

otiai10 commented Jan 9, 2024

Regarding this vinegarhq/vinegar#322 and the change, it seems you're trying to copy Overlay to dest. Is it right?

We assume that users can check existence or properties of the root argument, the first argument the users give to cp.Copy func, by theirselves.

OnError or OnSymlink is to define behavior to handle something inside the folders, not the root folders.

Therefore, if you are talking about the root folders, you should separate the check before calling cp.Copy.


If my quick view above doesn't answer your question, let me ask you some clarification questions:

Given what 1, what 2 do you expect?
What 3 did you see?
What 4 is your proposal?

@starzation
Copy link
Author

Regarding this vinegarhq/vinegar#322 and the change, it seems you're trying to copy Overlay to dest. Is it right?
Yes that right.

Look like this is not the lib issue right? This mean we need to check if the file is already exists first if not then we call cp.Copy right? Since by default cp.Copy doesn't check if the file already exists and trying to copy the file

For more information:
Overlay and dest is root folder and we trying to copy file inside Overlay into dest folder.

@otiai10
Copy link
Owner

otiai10 commented Jan 12, 2024

I think you understand it right.

In other words: as long as the user knows the names of what to be compared, the user should compare them by the user-side.

The methods "OnXxx" are provided because some cases happening during copying must be controlled by the user even the user couldn't anticipate the names of entries which would happen the cases.

Let me know if anything still unclear for you. I'm happy to hear to improve this pkg.

@starzation
Copy link
Author

Thank you for your clarification. Also thank you for your assistance.

@apprehensions
Copy link
Contributor

If i read copy behavior right, copy copies file no matter if the files exist in the destination directory, so why is the same not done for symlinks?

I expected copy to copy files from a source directory to a destination directory recursively, no matter if files are present or not in the destination.

@otiai10
Copy link
Owner

otiai10 commented Feb 8, 2024

@apprehensions Thank you for your comment.

Out discussion point is about "root" and "children".
When we talk about "root", it's users' responsibility to check the validity.

When we talk about "children" under the root, we need a basic philosophy and some options to let users control the behavior.

@apprehensions
Copy link
Contributor

Why are symlinks an exception?

@otiai10
Copy link
Owner

otiai10 commented Feb 9, 2024

Clarification:
Are we talking about this?

copy/copy.go

Lines 314 to 325 in 1b0f255

func lcopy(src, dest string) error {
orig, err := os.Readlink(src)
// @See https://github.com/otiai10/copy/issues/111
// TODO: This might be controlled by Options in the future.
if err != nil {
if os.IsNotExist(err) { // Copy symlink even if not existing
return os.Symlink(src, dest)
}
return err
}
return os.Symlink(orig, dest)
}

@apprehensions
Copy link
Contributor

Yes, it seems copying the source symlinks even if it's links to somewhere that does not exist. It would hr better if the symlinks gets copied to the destination regardless if it does not exist much like regular files

@otiai10
Copy link
Owner

otiai10 commented Feb 9, 2024

I think I got your point:

  • We are talking about "children" entries, not about "root" entries.
  • copy.Copy should not return error even if the destination of symlink already exists. (Update the symlink as of the source)

Correct me if I'm understanding wrong.

@apprehensions
Copy link
Contributor

Yes, the children (destination, copy) symlink should have the same behavior as copying regular files.

I apologize if I was explaining incorrectly.

@otiai10
Copy link
Owner

otiai10 commented Feb 13, 2024

Can you please open another issue for this?

@apprehensions
Copy link
Contributor

@starzation can you confirm #145 fixes your problem?

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

No branches or pull requests

3 participants