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
dir
anddir1
directories in the following state:$ tree dir1 dir dir1 ├── dir1 │ └── link1 └── file dir ├── dir1 └── link -> file
What happens here is the
file
is copied before thelink
and the exists function returns false because it follows symbolic links (docs reference). After that, the function fails without cleaning up half-moved directory. Also,link1
after 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
follow
field tofile::CopyOptions
(which defaults totrue
) and set it tofalse
when invoked from move and copy functions fromdir
module. Setting this field prevents following symbolic links when moving or copying. Additionally, I fixed another small bug - settingoverwrite
tofalse
infile::CopyOptions
would 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.