[PATCH 1 of 4] subrepo: do not try to get hidden revisions

Pierre-Yves David pierre-yves.david at ens-lyon.org
Thu Jan 16 15:06:45 CST 2014


On 01/15/2014 11:04 PM, Angel Ezquerra wrote:
> On Wed, Jan 15, 2014 at 4:00 PM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>> On 11/25/2013 12:46 PM, Angel Ezquerra wrote:
>>> # HG changeset patch
>>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>>> # Date 1385255414 -3600
>>> #      Sun Nov 24 02:10:14 2013 +0100
>>> # Node ID adb3d2a8dfd8bb1c6531225ef84be55f082bac0f
>>> # Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
>>> subrepo: do not try to get hidden revisions
>>>
>>> If a subrepo revision is hidden (because it was amended, for example) it
>>> does
>>> not make sense to try to "get" it from the remote subrepository.
>>>
>>> Note that in order to avoid making the change look bigger than it is, this
>>> adds
>>> an unnecessary else clause. This will be removed on a follow up patch.
>>>
>>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>>> --- a/mercurial/subrepo.py
>>> +++ b/mercurial/subrepo.py
>>> @@ -648,7 +648,10 @@
>>>          def _get(self, state):
>>>            source, revision, kind = state
>>> -        if revision not in self._repo:
>>> +        urepo = self._repo.unfiltered()
>>> +        if revision in urepo:
>>> +            return
>>
>> The right way to do it is:
>>
>> 1) having a new exception to "FilteredLookupError" inheriting from
>> "LookupError" (may need to add a Repo here somewhere),
>>
>> 2) Catch that specific exception when you need to distinct between "revision
>> is missing" and "revision is filtered",
>>
>> I see only one reason to not request for this approach here:
>>
>> Why are you not using `if revision in self._repo.unfiltered(): return` In
>> the first place ?
> I think in this case it makes more sense to just do as you suggest and
> do `if revision in self._repo.unfiltered(): return`, since the urepo
> variable that I introduced is not necessary.
>
> Would you be OK with a new patch that did just that (rather than
> introducing the new exception type that you suggest)?
>
Yes


More information about the Mercurial-devel mailing list