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

Fix error when overwrite_all option is set to true #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

catuhana
Copy link

@catuhana catuhana commented Sep 7, 2024

dircpy still did not overwrite the files even though CopyBuilder::overwrite was set to true.

dircpy still did not overwrite the files even though `CopyBuilder::overwrite` was set to true.
@woelper
Copy link
Owner

woelper commented Sep 8, 2024

Hi!
Thank you so much for the PR!

I've added a test here for a copy operation with overwrite set to true, but it passes:

fn copy_overwrite() {

Are you adding any other options? Could you please give an example where this fails, or perhaps add a failing test? On which operating system are you?

@catuhana
Copy link
Author

catuhana commented Sep 8, 2024

Hi there.

My code for using dircpy can be seen here: https://github.com/catuhana/nue/blob/failing-dircpy/src/types/node/release.rs#L73

Using CopyBuilder::overwrite option always throws this error without this PR. Other options, overwrite_if_newer and such, do not fail.
image

I'm on Ubuntu WSL.

@woelper
Copy link
Owner

woelper commented Sep 8, 2024

Hi!

Could you try running the test in the dircpy repo with cargo test copy_overwrite and see if it fails for you please?

It would be great if you had a very simple case to reproduce the error as I would like to know the root cause. This could also be a permission issue or something else - from your error message it seems like the operation fails because the file exists - perhaps that is an issue which could be addressed differently.

@catuhana
Copy link
Author

catuhana commented Sep 8, 2024

Hi again.

The test case does not fail for me at all, but I've created a repository for reproducing my issue. It has two features, dircpy_upstream which uses this repo directly and fails, and dircpy_fork which uses my fork (this PR specifically) and works.

The first run is always a success, since the target directory does not exist yet, so please run it twice to see the error.

@woelper
Copy link
Owner

woelper commented Sep 8, 2024

Amazing, I'll try that out!

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

Successfully merging this pull request may close these issues.

2 participants