[PATCH 2 of 2] mq: update subrepos when applying / unapplying patches that change .hgsubstate

[Angel Ezquerra]

On Sat, Aug 17, 2013 at 12:11 PM, Martin Geisler <martin at geisler.net> wrote:
> Angel Ezquerra <angel.ezquerra at gmail.com> writes:
>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1376350710 -7200
>> #      Tue Aug 13 01:38:30 2013 +0200
>> # Node ID 8921ebf2a815106702b0cb4da07177bfe61133ed
>> # Parent  8d70ca4d1804ca496ecf5f19fc7987b54f2de1ba
>> mq: update subrepos when applying / unapplying patches that change .hgsubstate
>>
>> Up until now applying or unapplying a patch that modified .hgsubstate
>> would not work as expected because it would not update the subrepos
>> according to the .hgsubstate change. This made it very easy to lose
>> subrepo changes when using mq.
>>
>> This revision also changes the test-mq-subrepo test so that on the
>> qpop / qpush tests. We no longer use the debugsub command to check the
>> state of the subrepos after the qpop and qpush operations. Instead we
>> directly run the id command on the subrepos that we want to check. The
>> reason is that using the debugsub command is misleading because it
>> does not really check the state of the subrepos on the working
>> directory (it just returns what the change that is specified on a
>> given revision). Because of this the tests did not detect the problem
>> that this revision fixes (i.e. that applying a patch did not update
>> the subrepos to the corresponding revisions).
>>
>> diff --git a/hgext/mq.py b/hgext/mq.py
>> --- a/hgext/mq.py
>> +++ b/hgext/mq.py
>> @@ -800,6 +800,13 @@
>>                  p1, p2 = repo.dirstate.parents()
>>                  repo.setparents(p1, merge)
>>
>> +            if all_files and '.hgsubstate' in all_files:
>> +                for line in open(repo.wjoin('.hgsubstate'), 'r').readlines():
>> +                    rev, s = line.split()
>> +                    files.append(s)
>> +                    state = (s, rev, 'any')
>> +                    repo[None].sub(s).get(state, overwrite=True)

Martin, thanks for the review. Sorry it took me a while to get back to you...

> This functionality must be present somewhere else -- since the normal
> update code also parses and updates subrepos as needed. I found it in
> the subrepo.state function.
>
> Maybe you could even use the entire submerge command? That should
> automatically handle things like changed subrepo sources too.

I'll look into it. My first impression is that you may be right.

, although in this case there would be no ancestor, or perhaps I could
check if the original parent of the patch is still on the repository,
and use that as ancestor? I have not really looked into the mq code
before this patch so I don't really know if that is something that mq
usually does... Or perhaps I could just use use the nullstate as the
ancestor?

My other concern is that usually subrepos are merged by doing an
actual merge on the subrepo which is something that I definitely think
should _not_ be done when you apply a patch! I don't know if submerge
is the one who does that, so I'll have to look into that as well.

In fact, this subrepo behavior (doing actual merges on subrepos when
merging the parent) is one of the most annoying things when using
subrepos (IMHO) and one I have been asked how to avoid tons of times
since I started supporting mercurial on my company... maybe I should
look into that as well :-)

Anyway, I'll see if I can use subrepo.state and subrepo.merge as you
suggest. There is already quite a lot of code that mq seems to almost
duplicate from regular mercurial commands. Let's see if I can avoid
adding even more :-)

Cheers,

Angel