Skip to content

repository: fix left-over file for symlink check - #5149

Closed
pks-t wants to merge 1 commit into
libgit2:masterfrom
pks-t:pks/win32-symlink-leftover
Closed

repository: fix left-over file for symlink check#5149
pks-t wants to merge 1 commit into
libgit2:masterfrom
pks-t:pks/win32-symlink-leftover

Conversation

@pks-t

@pks-t pks-t commented Jun 28, 2019

Copy link
Copy Markdown
Member

To determine whether the host we're running on supports symlinks, we
try to create a temporary symlink in the repo's working directory. If
creation succeeded, then symlinks are supported. This works just fine on
non-Windows hosts, but on Windows hosts it does leave behind the test
file without deleting it correctly.

The reason for this is that on Windows, one has to delete symlinks to
files with DeleteFile, but symlinks to directories with
RemoveDirectory. As we are creating the symlink with a non-existing
target of "testing", the symlink will be a directory symlink, but we
still use p_unlink which will map to DeleteFile and thus deletion
fails. Note: in the case where the target repo has a file "testing"
inside of its working directory, then we'd actually succeed to unlink
the file.

The issue can be fixed by creating another file and link to that one.
One might think of easier alternatives, e.g. linking to ".git" and then
just using p_rmdir on Windows. But this would fall flat for e.g.
submodules, which have a ".git" file, and for cases where the workdir
and gitdir live in different places.

Furthermore it's debatable whether p_unlink should emulate unlink(3P)
more closely. But this would require us to check what kind of file we
want to unlink, which could potentially have a non-negligible impact on
performance.


Fixes #5147

To determine whether the host we're running on supports symlinks, we
try to create a temporary symlink in the repo's working directory. If
creation succeeded, then symlinks are supported. This works just fine on
non-Windows hosts, but on Windows hosts it does leave behind the test
file without deleting it correctly.

The reason for this is that on Windows, one has to delete symlinks to
files with `DeleteFile`, but symlinks to directories with
`RemoveDirectory`. As we are creating the symlink with a non-existing
target of "testing", the symlink will be a directory symlink, but we
still use `p_unlink` which will map to `DeleteFile` and thus deletion
fails. Note: in the case where the target repo has a file "testing"
inside of its working directory, then we'd actually succeed to unlink
the file.

The issue can be fixed by creating another file and link to that one.
One might think of easier alternatives, e.g. linking to ".git" and then
just using `p_rmdir` on Windows. But this would fall flat for e.g.
submodules, which have a ".git" file, and for cases where the workdir
and gitdir live in different places.

Furthermore it's debatable whether `p_unlink` should emulate unlink(3P)
more closely. But this would require us to check what kind of file we
want to unlink, which could potentially have a non-negligible impact on
performance.
@pks-t

pks-t commented Jun 29, 2019

Copy link
Copy Markdown
Member Author

Closing in favour of #5151

@pks-t pks-t closed this Jun 29, 2019
@pks-t
pks-t deleted the pks/win32-symlink-leftover branch July 11, 2019 19:06
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.

Create repository has symlink left on Windows

1 participant