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

Adrian Buehlmann adrian at cadifra.com
Wed Nov 17 02:07:01 CST 2010


On 17.11.2010 06:58, Nicolas Dumazet wrote:
> 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')".

Which is slower than what Kevin proposed.

Matt has already backed out similar things recently:

  $ hg log -r 4cdaf1adafc8 -p mercurial/revset.py
  changeset:   12401:4cdaf1adafc8
  user:        Matt Mackall <mpm at selenic.com>
  date:        Fri Sep 24 12:46:54 2010 -0500
  summary:     backout most of 4f8067c94729

  diff -r 40852b4b910c -r 4cdaf1adafc8 mercurial/revset.py
  --- a/mercurial/revset.py       Fri Sep 24 12:00:55 2010 +0200
  +++ b/mercurial/revset.py       Fri Sep 24 12:46:54 2010 -0500
  @@ -87,7 +87,7 @@
   # helpers

   def getstring(x, err):
  -    if x and x[0] in ('string', 'symbol'):
  +    if x and (x[0] == 'string' or x[0] == 'symbol'):
           return x[1]
       raise error.ParseError(err)
  ...

and explained why in thread

  http://markmail.org/thread/ubecbru4airkedur

with message

  http://markmail.org/message/e2ogri6diygu2vsi

Some quotes from that message:

On 23.09.2010 18:41, Matt Mackall wrote:
> First, I think this is basically negligible as a visual style
> improvement.
[..]
> In other words, this kind of construct is just making life hard for
> Python. Wrapping something in parens is deceptively expensive.


More information about the Mercurial-devel mailing list