Patchwork fileset: evaluate status sets in working directory (BC)

login
register
mail settings
Submitter Martin von Zweigbergk
Date March 20, 2015, 10:33 p.m.
Message ID <bad329a772daa6daf111.1426890814@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/8208/
State Deferred
Headers show

Comments

Martin von Zweigbergk - March 20, 2015, 10:33 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1426873345 25200
#      Fri Mar 20 10:42:25 2015 -0700
# Node ID bad329a772daa6daf111b3b5bd30e15a44394eb1
# Parent  dc7588ce06b30a6ef347f7554e9646ac00e4456a
fileset: evaluate status sets in working directory (BC)

When no revision argument is given, it's probably a safe bet that
users expect filesets to be evaluated in the working directory, so
e.g. "hg revert 'set:modified()'" will revert the files modified in
the working directory, not files modified in revision '.', for
example. This also matches what "hg help filesets" says, which is
"...according to status" without mentioning any revisions.

Now consider what context the filesets are evaluated in for the
following commands:

 1. hg status -r $rev 'set:modified()'
 2. hg revert -r $rev 'set:modified()'
 3. hg files -r $rev 'set:modified()'

It seems likely that for 1. and 2., the user meant "for all the files
modified in the working directory, {show status for,revert them to}
revision $rev". It seems unlikely that a user would want to say "for
all the files modified between $rev^ and $rev, show the status between
$rev and the working directory". The latter is the current behavior.

The third case seems a little tricker. My first guess was that it
would tell list files that were modified in the working directory that
also exist in $rev. Another possible interpretation, and how it
currently behaves, is that it would list files modified in $rev
(compared to its parent) that still exist in $rev. (all, since they're
modified). Note that "hg files -r $rev 'set:removed()'" is always
empty, since files removed in $rev will not be in $rev (meta-note:
test-fileset.t calls debugfileset, obscuring this fact).

Since it seems like the user should mostly want to have status
filesets evaluated against the working directory, and since that's
also what the documentation seems to suggests, let's make it so. We
can later add a revision argument to these filesets ("hg files -r $rev
'set:modified($rev')") if we see a need.

As a bonus, this fixes some broken revert tests using status filesets
(even the ones I had not marked "BROKEN" were in fact broken).
Yuya Nishihara - March 21, 2015, 3:09 a.m.
On Fri, 20 Mar 2015 15:33:34 -0700, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1426873345 25200
> #      Fri Mar 20 10:42:25 2015 -0700
> # Node ID bad329a772daa6daf111b3b5bd30e15a44394eb1
> # Parent  dc7588ce06b30a6ef347f7554e9646ac00e4456a
> fileset: evaluate status sets in working directory (BC)
>
> When no revision argument is given, it's probably a safe bet that
> users expect filesets to be evaluated in the working directory, so
> e.g. "hg revert 'set:modified()'" will revert the files modified in
> the working directory, not files modified in revision '.', for
> example. This also matches what "hg help filesets" says, which is
> "...according to status" without mentioning any revisions.
> 
> Now consider what context the filesets are evaluated in for the
> following commands:
> 
>  1. hg status -r $rev 'set:modified()'
>  2. hg revert -r $rev 'set:modified()'
>  3. hg files -r $rev 'set:modified()'

What about "hg status --change $rev 'set:modified()'" ?
I expect it is the same as "hg status --change $rev --modified".

Another use of the current behavior is to archive changed files at that
revision.

  $ hg archive -r $rev -I 'set:added() or set:modified()'

> Since it seems like the user should mostly want to have status
> filesets evaluated against the working directory, and since that's
> also what the documentation seems to suggests, let's make it so.

IMHO, "revert" is the only exception that I expect fileset is evaluated
against workingctx.

> we can later add a revision argument to these filesets ("hg files -r $rev
> 'set:modified($rev')") if we see a need.

I don't think 'set:modified($rev)' would work well because a fileset function
narrows down given subset, which might not include all files at $rev.

Regards,
Martin von Zweigbergk - March 21, 2015, 3:39 a.m.
On Fri, Mar 20, 2015 at 8:10 PM Yuya Nishihara <yuya@tcha.org> wrote:

> On Fri, 20 Mar 2015 15:33:34 -0700, Martin von Zweigbergk wrote:
> > # HG changeset patch
> > # User Martin von Zweigbergk <martinvonz@google.com>
> > # Date 1426873345 25200
> > #      Fri Mar 20 10:42:25 2015 -0700
> > # Node ID bad329a772daa6daf111b3b5bd30e15a44394eb1
> > # Parent  dc7588ce06b30a6ef347f7554e9646ac00e4456a
> > fileset: evaluate status sets in working directory (BC)
> >
> > When no revision argument is given, it's probably a safe bet that
> > users expect filesets to be evaluated in the working directory, so
> > e.g. "hg revert 'set:modified()'" will revert the files modified in
> > the working directory, not files modified in revision '.', for
> > example. This also matches what "hg help filesets" says, which is
> > "...according to status" without mentioning any revisions.
> >
> > Now consider what context the filesets are evaluated in for the
> > following commands:
> >
> >  1. hg status -r $rev 'set:modified()'
> >  2. hg revert -r $rev 'set:modified()'
> >  3. hg files -r $rev 'set:modified()'
>
> What about "hg status --change $rev 'set:modified()'" ?
> I expect it is the same as "hg status --change $rev --modified".
>

I'm not sure what I would expect, but I'm glad to know about --modified,
which seems to provide the functionality that this patch is taking away :-)

>
> Another use of the current behavior is to archive changed files at that
> revision.
>
>   $ hg archive -r $rev -I 'set:added() or set:modified()'
>

That's a fair example that I don't have an answer for (before we make it
possible to pass a different context to the fileset). What do you use it
for?


>
> > Since it seems like the user should mostly want to have status
> > filesets evaluated against the working directory, and since that's
> > also what the documentation seems to suggests, let's make it so.
>
> IMHO, "revert" is the only exception that I expect fileset is evaluated
> against workingctx.
>
> > we can later add a revision argument to these filesets ("hg files -r $rev
> > 'set:modified($rev')") if we see a need.
>
> I don't think 'set:modified($rev)' would work well because a fileset
> function
> narrows down given subset, which might not include all files at $rev.
>

I guess I don't know enough about filesets to follow. What's the "given
subset" here? Could you give an example of how to produce such a subset
that doesn't include the files?
Martin von Zweigbergk - March 21, 2015, 4:21 a.m.
On Fri, Mar 20, 2015 at 8:10 PM Yuya Nishihara <yuya@tcha.org> wrote:

> On Fri, 20 Mar 2015 15:33:34 -0700, Martin von Zweigbergk wrote:
> > # HG changeset patch
> > # User Martin von Zweigbergk <martinvonz@google.com>
> > # Date 1426873345 25200
> > #      Fri Mar 20 10:42:25 2015 -0700
> > # Node ID bad329a772daa6daf111b3b5bd30e15a44394eb1
> > # Parent  dc7588ce06b30a6ef347f7554e9646ac00e4456a
> > fileset: evaluate status sets in working directory (BC)
> >
> > When no revision argument is given, it's probably a safe bet that
> > users expect filesets to be evaluated in the working directory, so
> > e.g. "hg revert 'set:modified()'" will revert the files modified in
> > the working directory, not files modified in revision '.', for
> > example. This also matches what "hg help filesets" says, which is
> > "...according to status" without mentioning any revisions.
> >
> > Now consider what context the filesets are evaluated in for the
> > following commands:
> >
> >  1. hg status -r $rev 'set:modified()'
> >  2. hg revert -r $rev 'set:modified()'
> >  3. hg files -r $rev 'set:modified()'
>
> What about "hg status --change $rev 'set:modified()'" ?
> I expect it is the same as "hg status --change $rev --modified".
>
> Another use of the current behavior is to archive changed files at that
> revision.
>
>   $ hg archive -r $rev -I 'set:added() or set:modified()'
>
> > Since it seems like the user should mostly want to have status
> > filesets evaluated against the working directory, and since that's
> > also what the documentation seems to suggests, let's make it so.
>
> IMHO, "revert" is the only exception that I expect fileset is evaluated
> against workingctx.
>

Actually, revert is why I sent this patch. While I do think the other cases
I listed are also not what I expect, I think the bug in revert is a deeper
one. I tried fixing it in a better way (which I only now realize was in
fact better), but largefiles is a mess and...


>
> > we can later add a revision argument to these filesets ("hg files -r $rev
> > 'set:modified($rev')") if we see a need.
>
> I don't think 'set:modified($rev)' would work well because a fileset
> function
> narrows down given subset, which might not include all files at $rev.
>
> Regards,
>
Yuya Nishihara - March 21, 2015, 4:36 a.m.
On Sat, 21 Mar 2015 03:39:28 +0000, Martin von Zweigbergk wrote:
> On Fri, Mar 20, 2015 at 8:10 PM Yuya Nishihara <yuya@tcha.org> wrote:
> > Another use of the current behavior is to archive changed files at that
> > revision.
> >
> >   $ hg archive -r $rev -I 'set:added() or set:modified()'
> >
> 
> That's a fair example that I don't have an answer for (before we make it
> possible to pass a different context to the fileset). What do you use it
> for?

TortoiseHg has a checkbox to archive "only files modified/created in this
revision". I know this checkbox is actually used because there were several
bug reports about it.

> > > Since it seems like the user should mostly want to have status
> > > filesets evaluated against the working directory, and since that's
> > > also what the documentation seems to suggests, let's make it so.
> >
> > IMHO, "revert" is the only exception that I expect fileset is evaluated
> > against workingctx.
> >
> > > we can later add a revision argument to these filesets ("hg files -r $rev
> > > 'set:modified($rev')") if we see a need.
> >
> > I don't think 'set:modified($rev)' would work well because a fileset
> > function
> > narrows down given subset, which might not include all files at $rev.
>
> I guess I don't know enough about filesets to follow. What's the "given
> subset" here? Could you give an example of how to produce such a subset
> that doesn't include the files?

I meant mctx.subset. The initial mctx.subset should be calculated from the
provided ctx, but repo[$rev] can have files that does not exist in the initial
mctx.subset.

I think I can explain the problem better if I send patches that implements
"set:wdir(modified())" function.

Regards,
Martin von Zweigbergk - March 21, 2015, 4:40 a.m.
On Fri, Mar 20, 2015 at 9:38 PM Yuya Nishihara <yuya@tcha.org> wrote:

> On Sat, 21 Mar 2015 03:39:28 +0000, Martin von Zweigbergk wrote:
> > On Fri, Mar 20, 2015 at 8:10 PM Yuya Nishihara <yuya@tcha.org> wrote:
> > > Another use of the current behavior is to archive changed files at that
> > > revision.
> > >
> > >   $ hg archive -r $rev -I 'set:added() or set:modified()'
> > >
> >
> > That's a fair example that I don't have an answer for (before we make it
> > possible to pass a different context to the fileset). What do you use it
> > for?
>
> TortoiseHg has a checkbox to archive "only files modified/created in this
> revision". I know this checkbox is actually used because there were several
> bug reports about it.
>
> > > > Since it seems like the user should mostly want to have status
> > > > filesets evaluated against the working directory, and since that's
> > > > also what the documentation seems to suggests, let's make it so.
> > >
> > > IMHO, "revert" is the only exception that I expect fileset is evaluated
> > > against workingctx.
> > >
> > > > we can later add a revision argument to these filesets ("hg files -r
> $rev
> > > > 'set:modified($rev')") if we see a need.
> > >
> > > I don't think 'set:modified($rev)' would work well because a fileset
> > > function
> > > narrows down given subset, which might not include all files at $rev.
> >
> > I guess I don't know enough about filesets to follow. What's the "given
> > subset" here? Could you give an example of how to produce such a subset
> > that doesn't include the files?
>
> I meant mctx.subset. The initial mctx.subset should be calculated from the
> provided ctx, but repo[$rev] can have files that does not exist in the
> initial
> mctx.subset.
>
> I think I can explain the problem better if I send patches that implements
> "set:wdir(modified())" function.
>
> Regards,
>

Sounds good. Now that I realize that I have to bite the bullet and touch
largefiles anyway, I don't depend on this patch getting in. Thanks.

Patch

diff -r dc7588ce06b3 -r bad329a772da mercurial/fileset.py
--- a/mercurial/fileset.py	Thu Mar 19 15:21:08 2015 -0500
+++ b/mercurial/fileset.py	Fri Mar 20 10:42:25 2015 -0700
@@ -494,8 +494,7 @@ 
         ignored = _intree(['ignored'], tree)
 
         r = ctx.repo()
-        status = r.status(ctx.p1(), ctx,
-                          unknown=unknown, ignored=ignored, clean=True)
+        status = r.status(unknown=unknown, ignored=ignored, clean=True)
         subset = []
         for c in status:
             subset.extend(c)
diff -r dc7588ce06b3 -r bad329a772da tests/test-fileset-generated.t
--- a/tests/test-fileset-generated.t	Thu Mar 19 15:21:08 2015 -0500
+++ b/tests/test-fileset-generated.t	Fri Mar 20 10:42:25 2015 -0700
@@ -141,39 +141,34 @@ 
   
 Test revert
 
-BROKEN: the files that get undeleted were not modified, they were removed,
-and content1_content2_missing-tracked was also not modified, it was deleted
-
   $ hg revert 'set:modified()'
   reverting content1_content1_content3-tracked
   reverting content1_content2_content1-tracked
-  undeleting content1_content2_content1-untracked
-  undeleting content1_content2_content2-untracked
   reverting content1_content2_content3-tracked
-  undeleting content1_content2_content3-untracked
-  reverting content1_content2_missing-tracked
-  undeleting content1_content2_missing-untracked
   reverting missing_content2_content3-tracked
 
-BROKEN: only the files that get forgotten are correct
-
   $ hg revert 'set:added()'
   forgetting content1_missing_content1-tracked
   forgetting content1_missing_content3-tracked
-  undeleting missing_content2_content2-untracked
-  undeleting missing_content2_content3-untracked
-  reverting missing_content2_missing-tracked
-  undeleting missing_content2_missing-untracked
   forgetting missing_missing_content3-tracked
 
   $ hg revert 'set:removed()'
   undeleting content1_content1_content1-untracked
   undeleting content1_content1_content3-untracked
   undeleting content1_content1_missing-untracked
+  undeleting content1_content2_content1-untracked
+  undeleting content1_content2_content2-untracked
+  undeleting content1_content2_content3-untracked
+  undeleting content1_content2_missing-untracked
+  undeleting missing_content2_content2-untracked
+  undeleting missing_content2_content3-untracked
+  undeleting missing_content2_missing-untracked
 
   $ hg revert 'set:deleted()'
   reverting content1_content1_missing-tracked
+  reverting content1_content2_missing-tracked
   forgetting content1_missing_missing-tracked
+  reverting missing_content2_missing-tracked
   forgetting missing_missing_missing-tracked
 
   $ hg revert 'set:unknown()'
diff -r dc7588ce06b3 -r bad329a772da tests/test-fileset.t
--- a/tests/test-fileset.t	Thu Mar 19 15:21:08 2015 -0500
+++ b/tests/test-fileset.t	Fri Mar 20 10:42:25 2015 -0700
@@ -191,15 +191,20 @@ 
   o  0 addfiles
   
   $ echo unknown > unknown
+TODO: allow passing revision to modified()
   $ fileset -r1 'modified()'
-  b2
+TODO: allow passing revision to added()
   $ fileset -r1 'added() and c1'
-  c1
+TODO: allow passing revision to removed()
   $ fileset -r1 'removed()'
-  a2
   $ fileset -r1 'deleted()'
   $ fileset -r1 'unknown()'
+  b2.orig
+  c3
+  unknown
   $ fileset -r1 'ignored()'
+  .hgignore
+  c2
   $ fileset -r1 'hgignore()'
   b2
   bin
diff -r dc7588ce06b3 -r bad329a772da tests/test-revset.t
--- a/tests/test-revset.t	Thu Mar 19 15:21:08 2015 -0500
+++ b/tests/test-revset.t	Fri Mar 20 10:42:25 2015 -0700
@@ -407,8 +407,10 @@ 
   $ log 'modifies("*")'
   4
   6
+  $ echo bbb >> b
   $ log 'modifies("set:modified()")'
   4
+  $ hg revert b
   $ log 'id(5)'
   2
   $ log 'only(9)'