D785: context: also consider path conflicts when clearing unknown files
mbthomas added inline comments. INLINE COMMENTS > swhitaker wrote in context.py:1941 > 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): 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 https://phab.mercurial-scm.org/D1157 to fix it. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 To: mbthomas, #hg-reviewers, ryanmce Cc: swhitaker, ryanmce, kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
swhitaker added inline comments. INLINE COMMENTS > context.py:1941 > +if wvfs.isfileorlink(p): > +wvfs.unlink(p) > +break 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): REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 To: mbthomas, #hg-reviewers, ryanmce Cc: swhitaker, ryanmce, kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
This revision was automatically updated to reflect the committed changes. Closed by commit rHG33acbd671554: context: also consider path conflicts when clearing unknown files (authored by mbthomas, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D785?vs=2362=2556 REVISION DETAIL https://phab.mercurial-scm.org/D785 AFFECTED FILES mercurial/context.py tests/test-pathconflicts-basic.t CHANGE DETAILS diff --git a/tests/test-pathconflicts-basic.t b/tests/test-pathconflicts-basic.t --- a/tests/test-pathconflicts-basic.t +++ b/tests/test-pathconflicts-basic.t @@ -35,6 +35,7 @@ [1] $ hg update --clean . 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ rm a~853701544ac3 Basic update - local directory conflicts with remote file @@ -46,13 +47,19 @@ abort: untracked files in working directory differ from files in requested revision [255] $ hg up --clean file - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file) + +Repo state is ok -Repo is in a very bad state now - recover manually - - $ rm -r a - $ hg up -q --clean 0 + $ hg sum + parent: 1:853701544ac3 + file + branch: default + bookmarks: *file + commit: (clean) + update: 2 new changesets (update) + phases: 4 draft Basic update - untracked file conflicts with remote directory @@ -62,6 +69,9 @@ a: replacing untracked file 1 files updated, 0 files merged, 0 files removed, 0 files unresolved (activating bookmark dir) + $ cat a.orig + untracked + $ rm -f a.orig Basic clean update - local directory conflicts with changed remote file @@ -73,11 +83,17 @@ abort: *: '$TESTTMP/repo/a' (glob) [255] $ hg up --clean file2 - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file2) + +Repo state is ok -Repo is in a very bad state now - recover manually + $ hg sum + parent: 2:f64e09fac717 + file2 + branch: default + bookmarks: *file2 + commit: (clean) + update: 1 new changesets, 2 branch heads (merge) + phases: 4 draft - $ rm -r a - $ hg up -q --clean 0 - diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1933,8 +1933,13 @@ ``write()`` can be called successfully. """ wvfs = self._repo.wvfs -if wvfs.isdir(self._path) and not wvfs.islink(self._path): -wvfs.removedirs(self._path) +f = self._path +if wvfs.isdir(f) and not wvfs.islink(f): +wvfs.rmtree(f, forcibly=True) +for p in reversed(list(util.finddirs(f))): +if wvfs.isfileorlink(p): +wvfs.unlink(p) +break def setflags(self, l, x): self._repo.wvfs.setflags(self._path, l, x) To: mbthomas, #hg-reviewers, ryanmce Cc: ryanmce, kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
ryanmce accepted this revision. ryanmce added a comment. Updated version looks good to me REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 To: mbthomas, #hg-reviewers, ryanmce Cc: ryanmce, kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
mbthomas marked 2 inline comments as done. mbthomas added inline comments. INLINE COMMENTS > mbthomas wrote in test-pathconflicts-basic.t:38 > I will strip the `+` characters. I didn't know these caused problems on > Windows. Now stripping '+' symbols in https://phab.mercurial-scm.org/D784. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 To: mbthomas, #hg-reviewers Cc: kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
mbthomas updated this revision to Diff 2362. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D785?vs=2221=2362 REVISION DETAIL https://phab.mercurial-scm.org/D785 AFFECTED FILES mercurial/context.py tests/test-pathconflicts-basic.t CHANGE DETAILS diff --git a/tests/test-pathconflicts-basic.t b/tests/test-pathconflicts-basic.t --- a/tests/test-pathconflicts-basic.t +++ b/tests/test-pathconflicts-basic.t @@ -35,6 +35,7 @@ [1] $ hg update --clean . 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ rm a~853701544ac3 Basic update - local directory conflicts with remote file @@ -46,13 +47,19 @@ abort: untracked files in working directory differ from files in requested revision [255] $ hg up --clean file - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file) + +Repo state is ok -Repo is in a very bad state now - recover manually - - $ rm -r a - $ hg up -q --clean 0 + $ hg sum + parent: 1:853701544ac3 + file + branch: default + bookmarks: *file + commit: (clean) + update: 2 new changesets (update) + phases: 4 draft Basic update - untracked file conflicts with remote directory @@ -62,6 +69,9 @@ a: replacing untracked file 1 files updated, 0 files merged, 0 files removed, 0 files unresolved (activating bookmark dir) + $ cat a.orig + untracked + $ rm -f a.orig Basic clean update - local directory conflicts with changed remote file @@ -73,11 +83,17 @@ abort: *: '$TESTTMP/repo/a' (glob) [255] $ hg up --clean file2 - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file2) + +Repo state is ok -Repo is in a very bad state now - recover manually + $ hg sum + parent: 2:f64e09fac717 + file2 + branch: default + bookmarks: *file2 + commit: (clean) + update: 1 new changesets, 2 branch heads (merge) + phases: 4 draft - $ rm -r a - $ hg up -q --clean 0 - diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1971,8 +1971,13 @@ ``write()`` can be called successfully. """ wvfs = self._repo.wvfs -if wvfs.isdir(self._path) and not wvfs.islink(self._path): -wvfs.removedirs(self._path) +f = self._path +if wvfs.isdir(f) and not wvfs.islink(f): +wvfs.rmtree(f, forcibly=True) +for p in reversed(list(util.finddirs(f))): +if wvfs.isfileorlink(p): +wvfs.unlink(p) +break def setflags(self, l, x): self._repo.wvfs.setflags(self._path, l, x) To: mbthomas, #hg-reviewers Cc: kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
mbthomas added inline comments. INLINE COMMENTS > kiilerix wrote in test-pathconflicts-basic.t:38 > I don't know about these `~hash` files, but including the `+` in the name > definitely seems wrong. I will strip the `+` characters. I didn't know these caused problems on Windows. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 To: mbthomas, #hg-reviewers Cc: kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
kiilerix added inline comments. INLINE COMMENTS > test-pathconflicts-basic.t:38 >1 files updated, 0 files merged, 1 files removed, 0 files unresolved > + $ rm a~853701544ac3+ > I don't know about these `~hash` files, but including the `+` in the name definitely seems wrong. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 To: mbthomas, #hg-reviewers Cc: kiilerix, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
mbthomas updated this revision to Diff 2221. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D785?vs=2003=2221 REVISION DETAIL https://phab.mercurial-scm.org/D785 AFFECTED FILES mercurial/context.py tests/test-pathconflicts-basic.t CHANGE DETAILS diff --git a/tests/test-pathconflicts-basic.t b/tests/test-pathconflicts-basic.t --- a/tests/test-pathconflicts-basic.t +++ b/tests/test-pathconflicts-basic.t @@ -35,6 +35,7 @@ [1] $ hg update --clean . 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ rm a~853701544ac3+ Basic update - local directory conflicts with remote file @@ -46,13 +47,19 @@ abort: untracked files in working directory differ from files in requested revision [255] $ hg up --clean file - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file) + +Repo state is ok -Repo is in a very bad state now - recover manually - - $ rm -r a - $ hg up -q --clean 0 + $ hg sum + parent: 1:853701544ac3 + file + branch: default + bookmarks: *file + commit: (clean) + update: 2 new changesets (update) + phases: 4 draft Basic update - untracked file conflicts with remote directory @@ -62,6 +69,9 @@ a: replacing untracked file 1 files updated, 0 files merged, 0 files removed, 0 files unresolved (activating bookmark dir) + $ cat a.orig + untracked + $ rm -f a.orig Basic clean update - local directory conflicts with changed remote file @@ -73,11 +83,17 @@ abort: *: '$TESTTMP/repo/a' (glob) [255] $ hg up --clean file2 - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file2) + +Repo state is ok -Repo is in a very bad state now - recover manually + $ hg sum + parent: 2:f64e09fac717 + file2 + branch: default + bookmarks: *file2 + commit: (clean) + update: 1 new changesets, 2 branch heads (merge) + phases: 4 draft - $ rm -r a - $ hg up -q --clean 0 - diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1971,8 +1971,13 @@ ``write()`` can be called successfully. """ wvfs = self._repo.wvfs -if wvfs.isdir(self._path) and not wvfs.islink(self._path): -wvfs.removedirs(self._path) +f = self._path +if wvfs.isdir(f) and not wvfs.islink(f): +wvfs.rmtree(f, forcibly=True) +for p in reversed(list(util.finddirs(f))): +if wvfs.isfileorlink(p): +wvfs.unlink(p) +break def setflags(self, l, x): self._repo.wvfs.setflags(self._path, l, x) To: mbthomas, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D785: context: also consider path conflicts when clearing unknown files
mbthomas created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY When clearing unknown files to remove path conflicts, also delete files that conflict with the target file's path. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D785 AFFECTED FILES mercurial/context.py tests/test-pathconflicts-basic.t CHANGE DETAILS diff --git a/tests/test-pathconflicts-basic.t b/tests/test-pathconflicts-basic.t --- a/tests/test-pathconflicts-basic.t +++ b/tests/test-pathconflicts-basic.t @@ -35,6 +35,7 @@ [1] $ hg update --clean . 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ rm a~853701544ac3+ Basic update - local directory conflicts with remote file @@ -46,13 +47,19 @@ abort: untracked files in working directory differ from files in requested revision [255] $ hg up --clean file - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file) + +Repo state is ok -Repo is in a very bad state now - recover manually - - $ rm -r a - $ hg up -q --clean 0 + $ hg sum + parent: 1:853701544ac3 + file + branch: default + bookmarks: *file + commit: (clean) + update: 2 new changesets (update) + phases: 4 draft Basic clean update - local directory conflicts with changed remote file @@ -64,11 +71,17 @@ abort: *: '$TESTTMP/repo/a' (glob) [255] $ hg up --clean file2 - abort: *: '$TESTTMP/repo/a' (glob) - [255] + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + (activating bookmark file2) + +Repo state is ok -Repo is in a very bad state now - recover manually + $ hg sum + parent: 2:f64e09fac717 + file2 + branch: default + bookmarks: *file2 + commit: (clean) + update: 1 new changesets, 2 branch heads (merge) + phases: 4 draft - $ rm -r a - $ hg up -q --clean 0 - diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1971,8 +1971,13 @@ ``write()`` can be called successfully. """ wvfs = self._repo.wvfs -if wvfs.isdir(self._path) and not wvfs.islink(self._path): -wvfs.removedirs(self._path) +f = self._path +if wvfs.isdir(f) and not wvfs.islink(f): +wvfs.rmtree(f, forcibly=True) +for p in util.finddirs(f): +if wvfs.isfile(p): +wvfs.unlink(p) +break def setflags(self, l, x): self._repo.wvfs.setflags(self._path, l, x) To: mbthomas, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
