Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem
When user attempts to move a directory that contains symbolic links, it might result in an error without a proper cleanup.
How to reproduce
Sample code that produces the issue:
Sample input:
What happens
Running the above program against the given input leaves
diranddir1directories in the following state:$ tree dir1 dir dir1 ├── dir1 │ └── link1 └── file dir ├── dir1 └── link -> fileWhat happens here is the
fileis copied before thelinkand the exists function returns false because it follows symbolic links (docs reference). After that, the function fails without cleaning up half-moved directory. Also,link1after the copy holds the same data asfile(which it points to) instead of being a symbolic link. I think this is not an expected behavior as well.My fix
I fixed it by adding a
followfield tofile::CopyOptions(which defaults totrue) and set it tofalsewhen invoked from move and copy functions fromdirmodule. Setting this field prevents following symbolic links when moving or copying. Additionally, I fixed another small bug - settingoverwritetofalseinfile::CopyOptionswould still overwrite dangling symlinks, now checks are performed to prevent this.My solution breaks backward compatibility, if you don't want to introduce such changes to your repository now, let me know and I can try to adjust it to be backwards compatible.