[PATCH 2 of 2 RFC] mq: ignore subrepos (issue2499)

[Nicolas Dumazet]

On Tue, 16 Nov 2010 13:13:31 -0600
Kevin Bullock <kbullock+mercurial at ringworld.org> wrote:

> # HG changeset patch
> # User Kevin Bullock <kbullock at ringworld.org>
> # Date 1289934367 21600
> # Node ID 98ef79b835aab117950f320da3abb06c2d013a58
> # Parent  742ab7cea58e8ca3aaea5d5d4b4fa0e252f9389e
> mq: ignore subrepos (issue2499)
> 
> If MQ allows modifying .hgsub or .hgsubstate in a patch, it can easily
> lead to an inconsistent subrepo state. This patch prevents qrefresh from
> adding any modifications to .hgsub or .hgsubstate to a patch. The user
> is warned that these files are not included in the patch.

Good. Thanks for working on this, I like the idea.

> 
> The tests test both the slightly irrational and the pathological cases.
> 
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -1298,12 +1298,18 @@
>              # local dirstate. in this case, we want them to only
>              # show up in the added section
>              for x in m:
> +                if x == '.hgsub' or x == '.hgsubstate':

But I would rather see everywhere "x in ('.hgsub', '.hgsubstate')".

Or do we want to use the exclude parameter of match objects? we always give
such objects to status() calls, we could use them to exclude .hgsub and 
.hgsubstate instead of matching manually filenames?

By the way, would we like to see a "mqignore" kind of feature, or at least,
a module-wide variable? say, a list of patterns that can be used as exclude
parameters to match objects?
That would allow extensions to extend the list, to add files to be ignored?


> diff --git a/tests/test-mq-qrefresh.t b/tests/test-mq-qrefresh.t
> --- a/tests/test-mq-qrefresh.t
> +++ b/tests/test-mq-qrefresh.t
> @@ -487,3 +487,76 @@

[...]

>  
> +test when deleting
> +  $ rm .hgsub .hgsubstate
> +  $ hg qrefresh
> +  warning: not removing .hgsub
> +  warning: not removing .hgsubstate
> +  refresh interrupted while patch was popped! (revert --all, qpush to recover)
> +  abort: No such file or directory: $TESTTMP/repo-2499/.hgsub
> +  [255]
> +  $ hg status
> +  ! .hgsub
> +  ! .hgsubstate
> +  $ hg cat -r1 .hgsub > .hgsub
> +  $ hg revert --no-backup .hgsubstate

Urgh, the abort does not look good. 

I mean, if you do something like:

 $ touch foo
 $ hg qnew some-patch
 $ hg add foo; hg qref foo
 $ rm foo
 $ hg qref

It does not fail, so I would expect the same behaviour here.


Questions:
1) what happens if .hgsub(state) has pending changes, and if we try a qpop/qpush?
Usually, those commands abort when we have local changes; if we decide to ignore .hgsub(state) we should not abort, and make sure that the files are untouched while qpopping or qpushing.

2) Similarly, what happens if you qimport a patch that has .hgsub(state) changes? I agree, maybe this usecase is rare. We could decide to ignore it as a particularly silly example. But I still think that when qpushing a patch, we may want to check that we never touch .hgsub(state)?

Regards,
-- 
Nicolas Dumazet — NicDumZ