[PATCH 2 of 2 v2 sparse-ext maybe-for-stable] sparse: treat paths as cwd-relative

[Gregory Szorc]

On Sat, Jul 29, 2017 at 2:36 PM, Martin von Zweigbergk via Mercurial-devel <
mercurial-devel at mercurial-scm.org> wrote:

>
>
> On Jul 29, 2017 1:32 PM, "Kostia Balytskyi" <ikostia at fb.com> wrote:
>
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # 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.
>

This affects the user-visible interface to sparse, which I consider to be
the most experimental aspect of the sparse feature from the perspective of
core Mercurial. On one hand, I could be convinced to queue. On the other,
the user-facing bits will continue to undergo many BC changes before sparse
is non-experimental. So it's not like beating the freeze accomplishes much
since it's all going to change anyway.


>
>
> 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.
>
> 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
> +
>  Verify commiting while sparse includes other files
>
>    $ echo z > hide
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170730/b5bbcd18/attachment-0002.html>