From patchwork Sat Jul 29 21:36:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [2, of, 2, v2, sparse-ext, maybe-for-stable] sparse: treat paths as cwd-relative From: via Mercurial-devel X-Patchwork-Id: 22583 Message-Id: To: Kostia Balytskyi Cc: Mercurial-devel Date: Sat, 29 Jul 2017 14:36:09 -0700 On Jul 29, 2017 1:32 PM, "Kostia Balytskyi" wrote: # HG changeset patch # User Kostia Balytskyi # Date 1501360312 25200 # Sat Jul 29 13:31:52 2017 -0700 # Branch stable # Node ID d596185be467a35f5d915d63f94c3fead982c1c5 # Parent c9a99abd647235c4dfd167ffcb9fccd66f0e0e4b sparse: treat paths as cwd-relative This commit makes it so sparse treats passed paths as CWD-relative, not repo-root-realive. This is a more intuitive behavior in my (and some other FB people's) opinion. This is breaking change however. My hope here is that since sparse is experimental, it's ok to introduce BCs. I'd rather wait until after the freeze. That is also the reason why I am sending this during a freeze: the experimental status of the feature probably means that these two patches can just land on stable. Interesting thought. I think I'd prefer to not make big changes even to experimental features during the freeze. At least not generally. Maybe we could consider it on a case-by-case basis. The reason (glob)s are needed in the test is this: in these two cases we do not supply path together with slashes, but `os.path.join` adds them, which means that under Windows they can be backslashes. To demonstrate this behavior, one could remove the (glob)s and run `./run-tests.py test-sparse.t` from MinGW's terminal on Windows. + Verify commiting while sparse includes other files $ echo z > hide diff --git a/hgext/sparse.py b/hgext/sparse.py --- a/hgext/sparse.py +++ b/hgext/sparse.py @@ -155,7 +155,8 @@ def _clonesparsecmd(orig, ui, repo, *arg if include or exclude or enableprofile: def clonesparse(orig, self, node, overwrite, *args, **kwargs): sparse.updateconfig(self.unfiltered(), pat, {}, include=include, - exclude=exclude, enableprofile=enableprofile) + exclude=exclude, enableprofile=enableprofile, + usereporootpaths=True) return orig(self, node, overwrite, *args, **kwargs) extensions.wrapfunction(hg, 'updaterepo', clonesparse) return orig(ui, repo, *args, **opts) diff --git a/mercurial/sparse.py b/mercurial/sparse.py --- a/mercurial/sparse.py +++ b/mercurial/sparse.py @@ -616,7 +616,7 @@ def importfromfiles(repo, opts, paths, f def updateconfig(repo, pats, opts, include=False, exclude=False, reset=False, delete=False, enableprofile=False, disableprofile=False, - force=False): + force=False, usereporootpaths=False): """Perform a sparse config update. Only one of the actions may be performed. @@ -639,6 +639,11 @@ def updateconfig(repo, pats, opts, inclu if any(os.path.isabs(pat) for pat in pats): raise error.Abort(_('paths cannot be absolute')) + if not usereporootpaths: + # let's treat paths as relative to cwd + cwd = repo.getcwd() + pats = [os.path.join(cwd, pat) for pat in pats] I still think this is too naive. Add a test for "glob:*.ext" and another one for "path:.". I don't think those will do what most people would expect. + if include: newinclude.update(pats) elif exclude: diff --git a/tests/test-sparse.t b/tests/test-sparse.t --- a/tests/test-sparse.t +++ b/tests/test-sparse.t @@ -48,6 +48,27 @@ TODO: See if this can be made to fail th [255] #endif +Paths should be treated as cwd-relative, not repo-root-relative + $ mkdir subdir && cd subdir + $ hg debugsparse --include path + $ hg debugsparse + [include] + $TESTTMP/myrepo/hide + hide + subdir/path (glob) + + $ cd .. + $ echo hello > subdir/file2.ext + $ hg debugsparse --include subdir/*.ext # let us test globs Wasn't this supposed to test "--include '*.ext'" from within subdir? + $ hg debugsparse + [include] + $TESTTMP/myrepo/hide + hide + subdir/file2.ext + subdir/path (glob) + + $ rm -rf subdir