This is an archive of the discontinued Mercurial Phabricator instance.

rebase: fix path auditing to audit path relative to repo root (issue5818)
ClosedPublic

Authored by martinvonz on Dec 3 2018, 5:10 PM.

Details

Summary

Before this patch, when rebasing a file called "foo/bar", we would
check e.g. if "/foo" (i.e. rooted at the file system root) was a
symlink.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Dec 3 2018, 5:10 PM
yuja added a subscriber: yuja.Dec 4 2018, 7:11 AM
Before this patch, when rebasing a file called "foo/bar", we would
check e.g. if "/foo" (i.e. rooted at the file system root) was a
symlink. That's clearly not what we meant to do. I haven't added a
test case because I don't know to test it.

This probably fixes the issue5818.

https://bz.mercurial-scm.org/show_bug.cgi?id=5818

    1. Test the other direction -- that this path from p2 isn't a directory
  • # in p1 (test that p1 doesn't any paths matching path/*).
  • match = matchmod.match('/', '', [path + '/'], default=b'relpath')

+ # in p1 (test that p1 doesn't have any paths matching path/*).
+ match = self.match(pats=[path + '/'], default=b'relpath')

'relpath' means relative to cwd. It should be default=b'path' or simply
scmutil.matchfiles().

In D5361#79583, @yuja wrote:
Before this patch, when rebasing a file called "foo/bar", we would
check e.g. if "/foo" (i.e. rooted at the file system root) was a
symlink. That's clearly not what we meant to do. I haven't added a
test case because I don't know to test it.

This probably fixes the issue5818.

Oh, I had not noticed that issue. I should be able to create a test case based on that. Thanks!

https://bz.mercurial-scm.org/show_bug.cgi?id=5818

    1. Test the other direction -- that this path from p2 isn't a directory
  • # in p1 (test that p1 doesn't any paths matching path/*).
  • match = matchmod.match('/', '', [path + '/'], default=b'relpath')

+ # in p1 (test that p1 doesn't have any paths matching path/*).
+ match = self.match(pats=[path + '/'], default=b'relpath')

'relpath' means relative to cwd. It should be default=b'path'

Good point. I'll fix that.

or simply
scmutil.matchfiles().

I don't think that will work because we want to match files in <path>/** here, not <path> itself.

martinvonz edited the summary of this revision. (Show Details)Dec 4 2018, 12:02 PM
martinvonz retitled this revision from rebase: fix path auditing to audit path relative to repo root to rebase: fix path auditing to audit path relative to repo root (issue5818).
martinvonz updated this revision to Diff 12699.
This revision was automatically updated to reflect the committed changes.