[PATCH 2 of 3] context: override workingctx.hex() to avoid a crash

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Jun 16 13:46:20 CDT 2015



On 06/16/2015 07:55 AM, Yuya Nishihara wrote:
> On Mon, 15 Jun 2015 13:09:57 -0500, Matt Mackall wrote:
>> On Tue, 2015-06-16 at 00:26 +0900, Yuya Nishihara wrote:
>>> On Mon, 15 Jun 2015 00:28:58 -0400, Matt Harbison wrote:
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>>> # Date 1434333857 14400
>>>> #      Sun Jun 14 22:04:17 2015 -0400
>>>> # Node ID f5f5e4ae488d9cae8111e9a212f647ed1430a019
>>>> # Parent  9c738d2588313bb624465495a8923b293c45b999
>>>> context: override workingctx.hex() to avoid a crash
>>>>
>>>> Since node is None for workingctx, it can't use the base class implementation of
>>>> 'hex(self.node())'.
>>>>
>>>> It doesn't appear that there are any current callers of this, but there will be
>>>> when archive supports 'wdir()'.  Since 'hg id' prints "p1.node() + '+'" for a
>>>> dirty working copy, this does too.  That of course means that it isn't possible
>>>> to call 'bin(ctx.hex())', but that seems circuitous.  Having the '+' seems like
>>>> a useful distinction when building the .hg_archival.txt file.
>>>>
>>>> diff --git a/mercurial/context.py b/mercurial/context.py
>>>> --- a/mercurial/context.py
>>>> +++ b/mercurial/context.py
>>>> @@ -1334,6 +1334,12 @@
>>>>       def __contains__(self, key):
>>>>           return self._repo.dirstate[key] not in "?r"
>>>>
>>>> +    def hex(self):
>>>> +        hex = self.p1().hex()
>>>> +        if self.dirty():
>>>> +            hex += '+'
>>>> +        return hex
>>>
>>> I won't argue, but I have to say it won't be so useful.
>>
>> I think this is pretty iffy. We might want to instead do something
>> analogous to null and assign fff... to the working copy. If I were
>> smarter way back when, I might have made the scheme look like:
>>
>> rev     hash
>> 0       00000  <- null revision
>> 1       ???    <- first revision
>> ...
>> maxint ffff   <- working copy
>
> I would agree if it were 2008 (though I would insist that rev should be
> len(repo) for ease of arithmetic operation.)
>
> IMHO, wctx.hex() should be comparable to hex(wctx.node()), but changing
> wctx.node() to '\xff...' would break many things. I doubt if we'll want to
> go such route just for wdir().

I've been thinking about this for revset, and I agree with Matt here. 
some maxint for working copy seems the way to go. Using len(repo) for 
working dir seems like an excelent way to introduce subtle error in the 
code.


-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list