On 4/16/19 12:56 PM, Gregory Szorc wrote:
On Tue, Apr 16, 2019 at 4:54 AM Gregory Szorc <gregory.sz...@gmail.com <mailto:gregory.sz...@gmail.com>> wrote:

    On Sat, Apr 13, 2019 at 5:56 PM Pierre-Yves David
    <pierre-yves.da...@ens-lyon.org
    <mailto:pierre-yves.da...@ens-lyon.org>> wrote:

        # HG changeset patch
        # User Pierre-Yves David <pierre-yves.da...@octobus.net
        <mailto:pierre-yves.da...@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@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
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to