This is an archive of the discontinued Mercurial Phabricator instance.

Differential D225

sparse: use correct cwd-prepending with patterns
ClosedPublic

Authored by ikostia on Aug 4 2017, 6:16 AM.
Tags
None
Subscribers
martinvonz

Details

Reviewers
mjpieters
durham
Group Reviewers
Restricted Project
Commits
rFBHGXa257a93a45b3: sparse: use correct cwd-prepending with patterns
Summary

Change fb-hgext's sparse to treat patterns correctly (this was brought to
my attention by @martinvonz on the mailing list).

This patch makes sure that:

reporoot/subdir$ hg --include **/file

results in subdir/**/file being included rather than subdir/.

Test Plan

rt

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Branch
default
Lint
Lint OK
Unit
Unit Test Errors

Event Timeline

ikostia created this revision.Aug 4 2017, 6:16 AM
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 4 2017, 6:16 AM
ikostia updated this revision to Diff 533.Aug 4 2017, 6:43 AM

resubmitting to avoid UT problems

ikostia updated this revision to Diff 534.Aug 4 2017, 6:45 AM

resubmitting to avoid UT problems 2

mjpieters requested changes to this revision.Aug 4 2017, 7:02 AM
mjpieters added inline comments.
hgext3rd/sparse.py
779–780

Nit: don't use backslash for line continuations when you could use (...) parentheses instead:

ap = ((kind + ':' if kind else '') +
         pathutil.canonpath(root, cwd, pat))

You also have a logic error here. You are filtering out patterns that are already absolute; if pats = ['glob:*.py', 'path:cwd/path'] you only retain ['glob:cwd/*.py'].

This revision now requires changes to proceed.Aug 4 2017, 7:02 AM
ikostia edited edge metadata.Aug 4 2017, 8:28 AM
ikostia updated this revision to Diff 535.

fix logic error

ikostia marked an inline comment as done.Aug 4 2017, 8:37 AM
mjpieters accepted this revision.Aug 4 2017, 9:02 AM
mjpieters added inline comments.
hgext3rd/sparse.py
779–783

Nit: just assign back to kindpat and drop the else: (un-indenting that line):

for kindpat in pats:
    kind, pat = matchmod._patsplit(kindpat, None)
    if kind in cwdrealtivepatkinds or kind is None:
        kindpat = ((kind + ':' if kind else '') +
                   pathutil.canonpath(root, cwd, pat))
    abspats.append(kindpat)
This revision is now accepted and ready to land.Aug 4 2017, 9:02 AM
Closed by commit rFBHGXa257a93a45b3: sparse: use correct cwd-prepending with patterns (authored by ikostia). · Explain WhyAug 4 2017, 9:20 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext3rd/sparse.py (27 lines)
Mtests/test-sparse.t (8 lines)

Diff 533

hgext3rd/sparse.py

# sparse.py - allow sparse checkouts of the working directory # sparse.py - allow sparse checkouts of the working directory
# #
# Copyright 2014 Facebook, Inc. # Copyright 2014 Facebook, Inc.
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
"""allow sparse checkouts of the working directory """allow sparse checkouts of the working directory
""" """
from mercurial import util, cmdutil, extensions, context, dirstate, commands from mercurial import util, cmdutil, extensions, context, dirstate, commands
from mercurial import localrepo, error, hg, registrar, scmutil from mercurial import localrepo, error, hg, pathutil, registrar
from mercurial import match as matchmod from mercurial import match as matchmod
from mercurial import merge as mergemod from mercurial import merge as mergemod
from mercurial.node import nullid from mercurial.node import nullid
from mercurial.i18n import _ from mercurial.i18n import _
import os, collections, hashlib import os, collections, hashlib
cmdtable = {} cmdtable = {}
command = registrar.command(cmdtable) command = registrar.command(cmdtable)
testedwith = 'ships-with-fb-hgext' testedwith = 'ships-with-fb-hgext'
cwdrealtivepatkinds = ('glob', 'relpath')
def uisetup(ui): def uisetup(ui):
_setupupdates(ui) _setupupdates(ui)
_setupcommit(ui) _setupcommit(ui)
def extsetup(ui): def extsetup(ui):
_setupclone(ui) _setupclone(ui)
_setuplog(ui) _setuplog(ui)
_setupadd(ui) _setupadd(ui)
newinclude = set(oldinclude) newinclude = set(oldinclude)
newexclude = set(oldexclude) newexclude = set(oldexclude)
newprofiles = set(oldprofiles) newprofiles = set(oldprofiles)
if any(os.path.isabs(pat) for pat in pats): if any(os.path.isabs(pat) for pat in pats):
err = _('paths cannot be absolute') err = _('paths cannot be absolute')
raise error.Abort(err) raise error.Abort(err)
if (not ui.configbool('sparse', 'includereporootpaths', False) adjustpats = ((include or exclude or delete) and
and (include or exclude or delete)): not ui.configbool('sparse', 'includereporootpaths', False))
# supplied file patterns should be treated as relative adjustpats |= ((enableprofile or disableprofile) and
# to current working dir, so we need to convert them first not ui.configbool('sparse', 'enablereporootpaths', True))
pats = scmutil.match(repo['.'], pats).files() if adjustpats:
if (not ui.configbool('sparse', 'enablereporootpaths', True)
and (enableprofile or disableprofile)):
# supplied file patterns should be treated as relative # supplied file patterns should be treated as relative
# to current working dir, so we need to convert them first # to current working dir, so we need to convert them first
pats = scmutil.match(repo['.'], pats).files() root, cwd = repo.root, repo.getcwd()
abspats = []
for kindpat in pats:
kind, pat = matchmod._patsplit(kindpat, None)
if kind in cwdrealtivepatkinds or kind is None:
ap = (kind + ':' if kind else '') +\
pathutil.canonpath(root, cwd, pat)
mjpietersUnsubmitted
Done

Nit: don't use backslash for line continuations when you could use (...) parentheses instead:

ap = ((kind + ':' if kind else '') +
         pathutil.canonpath(root, cwd, pat))

You also have a logic error here. You are filtering out patterns that are already absolute; if pats = ['glob:*.py', 'path:cwd/path'] you only retain ['glob:cwd/*.py'].

mjpieters: Nit: don't use backslash for line continuations when you could use `(...)` parentheses instead…
abspats.append(ap)
pats = abspats
mjpietersUnsubmitted
Not Done

Nit: just assign back to kindpat and drop the else: (un-indenting that line):

for kindpat in pats:
    kind, pat = matchmod._patsplit(kindpat, None)
    if kind in cwdrealtivepatkinds or kind is None:
        kindpat = ((kind + ':' if kind else '') +
                   pathutil.canonpath(root, cwd, pat))
    abspats.append(kindpat)
mjpieters: Nit: just assign back to `kindpat` and drop the `else:` (un-indenting that line)…
oldstatus = repo.status() oldstatus = repo.status()
if include: if include:
newinclude.update(pats) newinclude.update(pats)
elif exclude: elif exclude:
newexclude.update(pats) newexclude.update(pats)
elif enableprofile: elif enableprofile:
newprofiles.update(pats) newprofiles.update(pats)
elif disableprofile: elif disableprofile:

tests/test-sparse.t

$ hg sparse --include '/root' $ hg sparse --include '/root'
abort: paths cannot be absolute abort: paths cannot be absolute
[255] [255]
Repo root-relaive vs. cwd-relative includes Repo root-relaive vs. cwd-relative includes
$ mkdir subdir $ mkdir subdir
$ cd subdir $ cd subdir
$ hg sparse --config sparse.includereporootpaths=on --include reporoot/path $ hg sparse --config sparse.includereporootpaths=on --include reporoot/path
$ hg sparse --config sparse.includereporootpaths=off --include cwd/path $ hg sparse --config sparse.includereporootpaths=off --include **/path
$ hg sparse $ hg sparse
[include] [include]
$TESTTMP/myrepo/hide $TESTTMP/myrepo/hide
hide hide
reporoot/path reporoot/path
subdir/cwd/path subdir/**/path
[exclude] [exclude]
$ cd .. $ cd ..
$ rm -rf subdir $ rm -rf subdir
Verify deleting uses relative paths Verify deleting uses relative paths
$ mkdir subdir && echo foo > subdir/foo $ mkdir subdir && echo foo > subdir/foo
$ hg sparse $ hg sparse
[include] [include]
$TESTTMP/myrepo/hide $TESTTMP/myrepo/hide
hide hide
reporoot/path reporoot/path
subdir/cwd/path subdir/**/path
[exclude] [exclude]
$ cd subdir $ cd subdir
$ hg sparse --delete cwd/path $ hg sparse --delete **/path
$ hg sparse $ hg sparse
[include] [include]
$TESTTMP/myrepo/hide $TESTTMP/myrepo/hide
hide hide
reporoot/path reporoot/path
[exclude] [exclude]