Patchwork [8,of,8] fileset: add a 'status(...)' predicate to control evaluation context

login
register
mail settings
Submitter Matt Harbison
Date March 5, 2017, 5:20 a.m.
Message ID <op.ywl3r6079lwrgf@envy>
Download mbox | patch
Permalink /patch/18910/
State Not Applicable
Delegated to: Augie Fackler
Headers show

Comments

Matt Harbison - March 5, 2017, 5:20 a.m.
On Fri, 03 Mar 2017 23:35:04 -0500, Matt Harbison <mharbison72@gmail.com>  
wrote:

> On Fri, 03 Mar 2017 08:40:45 -0500, Pierre-Yves David  
> <pierre-yves.david@ens-lyon.org> wrote:
>
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1488546500 -3600
>> #      Fri Mar 03 14:08:20 2017 +0100
>> # Node ID 3e95bf7ed38189f6f56de89571fffc667280bb56
>> # Parent  9d6e733046b9aa7e2ded8c4207625fedcc2a8c04
>> # EXP-Topic filesetrev-func
>> # Available At  
>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>> #              hg pull  
>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r  
>> 3e95bf7ed381
>> fileset: add a 'status(...)' predicate to control evaluation context
>
> This is awesome!  It unlocks the "what merge conflicts might I run  
> into?" pre update/rebase test that I mentioned last week.  I did a test  
> to see if the files listed for the 13bbcd56c57a merge agreed with  
> TortoiseHg:
>
> $ ../hg files --rev 'ancestor(6483e49204ee, a91c62752d08)'  
> "set:status('ancestor(6483e49204ee, a91c62752d08)', 6483e49204ee,  
> added()+modified()+removed()) & status('ancestor(6483e49204ee,  
> a91c62752d08)', a91c62752d08, added()+modified()+removed())"
> ..\mercurial\ui.py
> ..\mercurial\worker.py
>
> It does, but that's quite a mouthful.  Granted, doing this for real  
> pre-merge would likely use the branch names, but not necessarily.   
> (Consider an anonymous branch after pulling.)
>
> I wonder if the parameters to this can have default values, to make it  
> more concise, and more like the status command.  For example, leaving  
> out the 3rd parameter implies added()+modified()+removed()+missing()  
> (status doesn't list clean or ignored by default).  It seems like it  
> would be convenient if one of the revisions (base?) defaulted to what  
> --rev specified for the command.  The second --rev for status defaults  
> to wdir().
>
> I know this is meant for aggregating files _across_ revisions, but then  
> `hg status` and `hg files 'set:status()'` would agree, without ORing  
> together the MAR! predicates.
>
> I did run into an oddity when testing with subrepos:
>
> diff --git a/tests/test-subrepo-deep-nested-change.t  
> b/tests/test-subrepo-deep-nested-change.t
> --- a/tests/test-subrepo-deep-nested-change.t
> +++ b/tests/test-subrepo-deep-nested-change.t
> @@ -455,6 +455,17 @@
>     sub1/sub2/sub2 (glob)
>     sub1/sub2/test.txt (glob)
>
> +  $ hg files 'set:status(0, "wdir()", added()+modified()+removed())'
> +  .hgsubstate
> +  foo\bar\abc
> +
> +  $ hg files -S 'set:status(0, tip, added()+modified()+removed())'
> +  .hgsubstate
> +  foo\bar\abc
> +  sub1\.hgsubstate
> +  sub1\foo
> +  sub1\sub2\folder\bar
> +
>     $ hg files sub1
>     sub1/.hgsub (glob)
>     sub1/.hgsubstate (glob)
>
> I'd expect .hgsub to show up in the main repo.

My misunderstanding on this one.  `hg status --rev 0` agrees with this  
output.  I read the doc saying "added from 3 to 7" as [3, 7], not (3, 7]  
as it actually is.  .hgsub was only changed in rev 0.  When I changed to  
null or 0000, it showed as expected.  (Using -1 here raises a parse error,  
and using "-1" causes nothing to match.)

> There may be some more missing in the subrepos for the second command,  
> but I don't have any more time to investigate tonight.  I'll see if I  
> can figure it out over the weekend.

Here's a test case:


The fileset gets fully populated with what `hg status` shows.  But if you  
drop the "-r 2" arg to `hg files`, the two *.txt files don't show.  What's  
happening is the command is walking 'wdir()', and since those files don't  
exist, they aren't tested against the set.  IOW, there's an implicit &  
with the files in the revision of the command against the fileset.  That  
makes sense, but it initially surprised me.  I'm not sure what words to  
use, but that should be documented somehow.  I also think that's an  
argument for the command --rev and the base rev defaulting to the same  
value.  (Is there a usecase for the command revision not being one or the  
other fileset revisions?)

One cosmetic thing below that I missed the first time.

> When I quote 3rd parameter in those tests, the output is different.   
> It's unfortunate that revsets in arg 1 or 2 requires quoting, but  
> quoting is wrong for the 3rd arg.
>
>    $ hg files 'set:status(0, "wdir()", "added()+modified()+removed()")'
>    [1]
>
>    $ hg files -S 'set:status(0, tip, "added()+modified()+removed()")'
>    abort: added()+modified()+removed() not under root  
> '$TESTTMP\cloned\sub1' (in subrepo sub1)
>    (consider using '--cwd sub1')
>    [255]
>
> If this is just because of the way the parser needs to be, mentioning it  
> in the help may be sufficient.
>
>> Same as 'revs', this predicate does not select files but switches the  
>> evaluation
>> context. This allow to match file according arbitrary status call. We  
>> can now
>> express the same query as 'hg status'.
>>
>> The API (two 'revsingle' class) have been picked instead of a single  
>> 'revs'
>> revset for multiple reasons:
>>
>>  * it is less confusing to express
>>  * it allow to express more query (eg: backward status, cross branch  
>> status)
>>
>> diff --git a/mercurial/fileset.py b/mercurial/fileset.py
>> --- a/mercurial/fileset.py
>> +++ b/mercurial/fileset.py
>> @@ -463,6 +463,25 @@ def revs(mctx, x):
>>                  result.append(f)
>>      return result
>> +@predicate('status(base, rev, pattern)')
>> +def status(mctx, x):
>> +    """``status(base, rev, revspec)``
>> +
>> +    Evaluate predicate using status change between ``base`` and
>> +    ``rev``. Examples:
>> +
>> +    - ``status(3, 7, added())`` - matches files added from "3" to "7"
>> +    """
>> +    repo = mctx.ctx.repo()
>> +    # i18n: "status" is a keyword
>> +    b, r, x = getargs(x, 3, 3, _("status takes three arguments"))
>> +    # i18n: "status" is a keyword
>> +    baserevspec = getstring(b, _("first argument to revs must be a  
>> revision"))
>> +    revspec = getstring(r, _("second argument to revs must be a  
>> revision"))

s/revs/status/g

Patch

diff --git a/tests/test-subrepo-deep-nested-change.t  
b/tests/test-subrepo-deep-nested-change.t
--- a/tests/test-subrepo-deep-nested-change.t
+++ b/tests/test-subrepo-deep-nested-change.t
@@ -455,6 +455,18 @@ 
    sub1/sub2/sub2 (glob)
    sub1/sub2/test.txt (glob)

+  $ hg files -r 2 -S 'set:status(1, 2, added()+modified()+removed())'
+  .hgsubstate
+  sub1\.hgsubstate
+  sub1\sub2\folder\test.txt
+  sub1\sub2\test.txt
+
+  $ hg status -S --rev 1 --rev 2 | sort -k2
+  M .hgsubstate
+  M sub1/.hgsubstate
+  A sub1/sub2/folder/test.txt
+  A sub1/sub2/test.txt
+
    $ hg files sub1
    sub1/.hgsub (glob)
    sub1/.hgsubstate (glob)