Patchwork D3185: context: convert binary changeid to hex also for filtered ones

login
register
mail settings
Submitter phabricator
Date April 7, 2018, 6:15 a.m.
Message ID <differential-rev-PHID-DREV-hcw24pnmvnxddxshq4wg-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30532/
State Superseded
Headers show

Comments

phabricator - April 7, 2018, 6:15 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Funnily enough, within about a day of writing https://phab.mercurial-scm.org/D3144, I ran into the
  exception in practice. I spent some time trying figure out why I got
  the "unknown revision" message when the node was clearly in the repo,
  and then I finally remembered that I had just fixed that bug.  Anyway,
  even with the fix applied, the message wasn't ideal: now it would
  instead be something like "abort: hidden revision 'K*}s!", because the
  error message didn't go through the conversion to hex that happens at
  the end of the try-block. Let's fix that by converting to hex before
  re-raising.
  
  (I included example output in https://phab.mercurial-scm.org/D3144, which is apparently incorrect. I
  suppose that must have been produced when the patch was later in my
  stack of unsent patches, before I rebased it and sent it. Sorry about
  that.)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3185

AFFECTED FILES
  mercurial/context.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - April 7, 2018, 7:59 a.m.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  IIUC, `Filtered*Error`s are re-raised up to scmutil.revsymbol(), so fixing
  up `changeid` here wouldn't work.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3185

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - April 8, 2018, 3:28 p.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3185#51146, @yuja wrote:
  
  > IIUC, `Filtered*Error`s are re-raised up to scmutil.revsymbol(), so fixing
  >  up `changeid` here wouldn't work.
  
  
  Sigh, you're right, of course. Thanks for checking. So this is already handled correctly. I suppose that's why I sent https://phab.mercurial-scm.org/D3144 before https://phab.mercurial-scm.org/D3143. I'll just drop this patch then.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3185

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -411,6 +411,7 @@ 
                     self._rev = repo.changelog.rev(changeid)
                     return
                 except error.FilteredLookupError:
+                    changeid = hex(changeid) # for the error message
                     raise
                 except LookupError:
                     pass