[PATCH 04 of 14 "] pull: deal with locally filtered changeset passed into --rev

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Apr 16 08:26:39 EDT 2019



On 4/16/19 12:56 PM, Gregory Szorc wrote:
> On Tue, Apr 16, 2019 at 4:54 AM Gregory Szorc <gregory.szorc at gmail.com 
> <mailto:gregory.szorc at gmail.com>> wrote:
> 
>     On Sat, Apr 13, 2019 at 5:56 PM Pierre-Yves David
>     <pierre-yves.david at ens-lyon.org
>     <mailto:pierre-yves.david at ens-lyon.org>> wrote:
> 
>         # HG changeset patch
>         # User Pierre-Yves David <pierre-yves.david at octobus.net
>         <mailto:pierre-yves.david at octobus.net>>
>         # Date 1554472565 -7200
>         #      Fri Apr 05 15:56:05 2019 +0200
>         # Node ID 0adcfded9b03fff84190594ef29e37110967419f
>         # Parent  d5f42ea7b06825ee86620cdc18aaa3a53504bff5
>         # EXP-Topic hgweb-obsolete
>         # Available At https://bitbucket.org/octobus/mercurial-devel/
>         #              hg pull
>         https://bitbucket.org/octobus/mercurial-devel/ -r 0adcfded9b03
>         pull: deal with locally filtered changeset passed into --rev
> 
>         Nowadays, it is possible to explicitly pull a remote revision
>         that end up being
>         hidden locally (eg: obsoleted locally). However before this
>         patch, some
>         internal processing where crashing trying to resolve a filtered
>         revision.
> 
>         Without this patches, the pull output result a confusing output:
> 
>            $ hg pull ../repo-Bob --rev 956063ac4557
>            pulling from ../repo-Bob
>            searching for changes
>            adding changesets
>            adding manifests
>            adding file changes
>            added 2 changesets with 0 changes to 2 files (+1 heads)
>            (2 other changesets obsolete on arrival)
>            abort:
>         00changelog.i at 956063ac4557828781733b2d5677a351ce856f59: filtered
>         node!
> 
> 
>     The existing abort message is bad and should be improved because
>     typical users won't have a clue what it means.
> 
>     But I have reservations about this patch because it isn't clear what
>     will happen with `pull -u -r <hidden>`. If the working directory
>     will be updated to a hidden revision without --hidden specified,
>     this feels wrong to me.
> 
> 
> Oh - maybe part 5 (and later?) address my concerns?

It does. Regarding the abort message. I know we have a better message 
available for this kind of error. I am not sure why that better message 
is not used here and I intend to dig into that next cycle (there are a 
couple of usual suspect: wrong exception types, bits still in evolve 
extension, etc…)

>     Could we get test coverage showing what happens in this case? Please
>     also check for behavior with `hg clone -r <hidden>` and `hg clone -u
>     <rev>` as well.

(as you already noticed, this is addressed in the next patch).

>     Also, I'm a little confused about "checkout" and "brev" both doing
>     similar things. It seems that "checkout" is used internally and
>     "brev" is used for user-facing output. I wish this code were better
>     documented. But that is scope bloat...

+1

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list