This is an archive of the discontinued Mercurial Phabricator instance.

context: also consider path conflicts when clearing unknown files
ClosedPublic

Authored by mbthomas on Sep 22 2017, 5:28 AM.

Details

Summary

When clearing unknown files to remove path conflicts, also delete files that
conflict with the target file's path.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mbthomas created this revision.Sep 22 2017, 5:28 AM
mbthomas updated this revision to Diff 2221.Oct 1 2017, 5:33 AM
kiilerix added inline comments.
tests/test-pathconflicts-basic.t
38

I don't know about these ~hash files, but including the + in the name definitely seems wrong.

mbthomas added inline comments.Oct 2 2017, 5:12 PM
tests/test-pathconflicts-basic.t
38

I will strip the + characters. I didn't know these caused problems on Windows.

mbthomas updated this revision to Diff 2362.Oct 2 2017, 5:15 PM
mbthomas marked 2 inline comments as done.Oct 5 2017, 12:49 PM
mbthomas added inline comments.
tests/test-pathconflicts-basic.t
38

Now stripping '+' symbols in D784.

mbthomas marked 2 inline comments as done.Oct 5 2017, 12:49 PM
ryanmce accepted this revision.Oct 5 2017, 12:53 PM
ryanmce added a subscriber: ryanmce.

Updated version looks good to me

This revision was automatically updated to reflect the committed changes.
swhitaker added inline comments.
mercurial/context.py
1979

This breaks test-audit-path.t on macOS. In the test "attack /tmp/test", we call this codepath with f == '/tmp/test'. util.finddirs finds '/tmp', which on macOS is a symlink to /private/tmp, so L1940 is true and on L1941 we try to unlink /tmp.

@mbthomas Is it intentional that we try to unlink symlinks to directories here? If not, we can fix this with:

-                if wvfs.isfileorlink(p):
+                if wvfs.isfileorlink(p) and not wvfs.isdir(p):
mbthomas added inline comments.Oct 17 2017, 11:16 AM
mercurial/context.py
1979

It's intentional to unlink symlinks, but it should not be doing anything outside the vfs. This is missing a vfs.audit() call. I've created D1157 to fix it.