Patchwork [V2] context: override workingctx.hex() to avoid a crash

login
register
mail settings
Submitter Matt Harbison
Date June 16, 2015, 2:33 a.m.
Message ID <e8aad4e14a74b6034d48.1434421990@Envy>
Download mbox | patch
Permalink /patch/9657/
State Accepted
Delegated to: Matt Mackall
Headers show

Comments

Matt Harbison - June 16, 2015, 2:33 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1434333857 14400
#      Sun Jun 14 22:04:17 2015 -0400
# Node ID e8aad4e14a74b6034d48183d737d52e78ba4c6b4
# Parent  a69983942fb4dbfdf5ca8c7a123120ed09ead5db
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()'.  My first thought was to use "{p1node}+", but
that would cause headaches elsewhere [1].

We should probably fix up localrepository.__getitem__ to accept this hash for
consistency, as a followup.  This works, if the full hash is specified:

  @@ -480,7 +480,7 @@
           return dirstate.dirstate(self.vfs, self.ui, self.root, validate)

       def __getitem__(self, changeid):
  -        if changeid is None:
  +        if changeid is None or changeid == 'ff' * 20:
               return context.workingctx(self)
           if isinstance(changeid, slice):
               return [context.changectx(self, i)

That differs from null, where it will accept any number of 0s, as long as it
isn't ambiguous.


[1] https://www.selenic.com/pipermail/mercurial-devel/2015-June/071166.html
Matt Mackall - June 16, 2015, 9:22 p.m.
On Mon, 2015-06-15 at 22:33 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1434333857 14400
> #      Sun Jun 14 22:04:17 2015 -0400
> # Node ID e8aad4e14a74b6034d48183d737d52e78ba4c6b4
> # Parent  a69983942fb4dbfdf5ca8c7a123120ed09ead5db
> context: override workingctx.hex() to avoid a crash

Queued for default, thanks. I'm still a bit anxious about this, though,
so let's not add too many dependencies on this idea right away.
Matt Harbison - June 17, 2015, 12:48 a.m.
On Tue, 16 Jun 2015 17:22:01 -0400, Matt Mackall <mpm@selenic.com> wrote:

> On Mon, 2015-06-15 at 22:33 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1434333857 14400
>> #      Sun Jun 14 22:04:17 2015 -0400
>> # Node ID e8aad4e14a74b6034d48183d737d52e78ba4c6b4
>> # Parent  a69983942fb4dbfdf5ca8c7a123120ed09ead5db
>> context: override workingctx.hex() to avoid a crash
>
> Queued for default, thanks. I'm still a bit anxious about this, though,
> so let's not add too many dependencies on this idea right away.

My plan after getting the wdir() archiving landed is to change how the  
metadata file is built, so that it will display {p1node}+ if necessary.   
That's at least a bit more informative than all 'f's.

This just seemed like a pothole that needed filling.
Pierre-Yves David - June 17, 2015, 6:39 p.m.
On 06/15/2015 07:33 PM, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1434333857 14400
> #      Sun Jun 14 22:04:17 2015 -0400
> # Node ID e8aad4e14a74b6034d48183d737d52e78ba4c6b4
> # Parent  a69983942fb4dbfdf5ca8c7a123120ed09ead5db
> 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()'.  My first thought was to use "{p1node}+", but
> that would cause headaches elsewhere [1].
>
> We should probably fix up localrepository.__getitem__ to accept this hash for
> consistency, as a followup.  This works, if the full hash is specified:
>
>    @@ -480,7 +480,7 @@
>             return dirstate.dirstate(self.vfs, self.ui, self.root, validate)
>
>         def __getitem__(self, changeid):
>    -        if changeid is None:
>    +        if changeid is None or changeid == 'ff' * 20:

Look's like we need a mercurial.node.wdirid
(and mercurial.node.wdirrev)
Yuya Nishihara - June 17, 2015, 11:01 p.m.
On Wed, 17 Jun 2015 11:39:45 -0700, Pierre-Yves David wrote:
> On 06/15/2015 07:33 PM, Matt Harbison wrote:
> > # HG changeset patch
> > # User Matt Harbison <matt_harbison@yahoo.com>
> > # Date 1434333857 14400
> > #      Sun Jun 14 22:04:17 2015 -0400
> > # Node ID e8aad4e14a74b6034d48183d737d52e78ba4c6b4
> > # Parent  a69983942fb4dbfdf5ca8c7a123120ed09ead5db
> > 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()'.  My first thought was to use "{p1node}+", but
> > that would cause headaches elsewhere [1].
> >
> > We should probably fix up localrepository.__getitem__ to accept this hash for
> > consistency, as a followup.  This works, if the full hash is specified:
> >
> >    @@ -480,7 +480,7 @@
> >             return dirstate.dirstate(self.vfs, self.ui, self.root, validate)
> >
> >         def __getitem__(self, changeid):
> >    -        if changeid is None:
> >    +        if changeid is None or changeid == 'ff' * 20:
> 
> Look's like we need a mercurial.node.wdirid
> (and mercurial.node.wdirrev)

I'm thinking of it. I'll try them in revset experiment.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1334,6 +1334,9 @@ 
     def __contains__(self, key):
         return self._repo.dirstate[key] not in "?r"
 
+    def hex(self):
+        return "ff" * 20
+
     @propertycache
     def _parents(self):
         p = self._repo.dirstate.parents()