On 26/07/2018 17:24, Martin von Zweigbergk wrote:
On Thu, Jul 26, 2018 at 6:26 AM Boris FELD <lothiral...@gmail.com
<mailto:lothiral...@gmail.com>> wrote:
On 26/07/2018 15:06, Yuya Nishihara wrote:
> On Thu, 26 Jul 2018 14:21:20 +0200, Boris Feld wrote:
>> # HG changeset patch
>> # User Boris Feld <boris.f...@octobus.net
<mailto:boris.f...@octobus.net>>
>> # Date 1532595350 -7200
>> # Thu Jul 26 10:55:50 2018 +0200
>> # Branch stable
>> # Node ID 88a0bf46a3ffb78aaab203d13a7c9f53e244282b
>> # Parent a920f2620726ef26e6caed3d72b24297699b5b39
>> # EXP-Topic compat-hggit
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> # hg pull
https://bitbucket.org/octobus/mercurial-devel/ -r 88a0bf46a3ff
>> clone: process 'lookup' return as an arbitrary symbol
>>
>> In theory, checkout is expected to be a node here because it
was returned by
>> peer.lookup.
>>
>> In practice, multiple important extensions (like hg-git,
hg-subversion) use
>> peers not backed by a mercurial repository where lookup cannot
return a node.
>>
>> Allowing arbitrary symbols is necessary to make these
extensions working with
>> 4.7.
>>
>> We should probably introduce a new API in Core to have these
extensions to
>> work without abusing the lookup API. In the meantime, a small
change to
>> restore compatibility in 4.7 seems in order.
>>
>> diff --git a/mercurial/hg.py b/mercurial/hg.py
>> --- a/mercurial/hg.py
>> +++ b/mercurial/hg.py
>> @@ -730,6 +730,19 @@ def clone(ui, peeropts, source, dest=Non
>>
>> uprev = None
>> status = None
>> + if checkout is not None and not
node.isnode(checkout):
> IIUC, hggit/hgsubversion may pass in a 20-length symbolic name?
Yes, this is unfortunate. Having everything working but 20-lenght
names
is a first step, we can follow-up with more logic to check if the
symbol
is a valid name.
I think Yuya is suggesting that you simply remove the "and not
node.isnode(checkout)", and I agree with that. That way 20-character
names would also work. There are two downsides that I can see: 1) the
cost of an extra check, and 2) a 20-character binary nodeid (from a
well-behaving peer) that happens to also be a valid name will now be
interpreted as the name. (1) seems negligible given the cost of a
clone. (2) seems extremely unlikely to happen (i.e. that a binary
nodeid happens to be only readable characters that someone also
happened to use in a name). Am I missing something?
We need to detect valid node because they cannot go through revsymbol,
it breaks the tests without that check. The previous API was able to
handle any cases, it is now hard to get the same result when needed.
If a 20 chars string is a valid node (ie: exists in the repo) it should
probably take precedence over the name. It seems extremely unlikely to
not care about that case.
>
>> + # checkout was expected to be a node here
because it was
>> + # returned by peer.lookup.
>> + #
>> + # However, some extension (like hg-git)
introduce peer not
>> + # backed by a mercurial repository where
lookup cannot
>> + # return a node. Processing 'checkout' as
an arbitrary
>> + # symbols make it possible with these
extensions keep
>> + # working.
>> + if scmutil.isrevsymbol(destrepo, checkout):
>> + checkout = scmutil.revsymbol(destrepo,
checkout).node()
>> + else:
>> + checkout = None
> Do you have any plan to deprecate this hack?
Our plan is to introduce a clear separate API that could be used
by such
extensions for this specific case in 4.8.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
<mailto:Mercurial-devel@mercurial-scm.org>
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
