[PATCH 3 of 3] extensions: refuse to load extensions if minimum hg version not met

Augie Fackler raf at durin42.com
Mon Nov 30 16:33:43 CST 2015 

> On Nov 30, 2015, at 17:31, timeless <timeless at gmail.com> wrote:
> 
> Let me put this differently.
> 

I appreciate all the forward-thinking here (normally it's a good thing!), but this is Python code, not an on-disk format, so we can be a little bit lazier here and if we ever want to be more flexible we can do something like rename the variable and then check for both.

> obsolete as shipped in 3.0 was so broken, that if it ever encountered
> a repo w/ obsolete v1 markers, it would break. It didn't look forward
> enough to ensure that it wouldn't crash. Specifically, it loaded the
> markers w/o checking to see if the feature was enabled, and if it
> didn't understand the marker format, it crashed.
> 
> Similarly, the merge state stuff in v2 was so broken that we can't
> actually extend it (if we added a new marker, it'd cause a v2 reader
> to crash, just as obsolete was broken), so we're making a v3.
> 
> What I'm mostly asking for is something such that:
> 
> If we add support for multiple versions, doesn't break. and behaves sanely.
> Or perhaps, we just spend the extra few minutes to design it to
> support extra tupples now.
> 
> I don't want an extension that says 3.8.2, 3.9.1 to actually crash a
> 3.7 because it was looking at something and couldn't handle it.
> 
> If we can make the minversion at least just treat "I don't understand"
> as "I don't support", I'd be slightly happier than "we didn't test
> that case".
> 
> I'd be even happier if we had it support "checking the first field,
> and ignore everything after a comma", so that "3.7.1, 3.8.0" could
> work in a 3.7.1 even if it couldn't parse the stuff after the comma --
> but still protected me from running that extension in "3.7.0".
> 
> 
> 
> On Mon, Nov 30, 2015 at 5:16 PM, Augie Fackler <raf at durin42.com> wrote:
>> On Fri, Nov 27, 2015 at 11:42:33AM -0500, timeless wrote:
>>> 1. A complaint I have is about extensions (hi evolve) which interfere
>>> with my ability to push.
>>> 
>>> I'm using systems I don't control. I have a local hg which is new and
>>> supports evolve, and a global version of hg which is old, and isn't
>>> supported by evolve.
>>> 
>>> I can't push to these systems because of an error from evolve:
>>> 
>>> $ hg out gcc112
>>> comparing with ssh://gcc112/hg/crew
>>> remote: *** failed to import extension evolve from
>>> ~/hg/evolve-main/hgext/evolve.py: evolve needs version 3.4.3 or above
>>> remote: abort: parsing obsolete marker: unknown version 1
>>> abort: unexpected response: empty string
>>> 
>>> I'd really really like for this to be solved in a way that results in
>>> extensions are silently not loaded in such cases. A remote user can't
>>> do anything about this and shouldn't be punished for it.
>>> 
>>> 2. I can imagine an extension that would work on 1.2.3, 1.4.2, 1.6.1, 1.8.
>>> It might be nice to support a list of versions instead of a single version.
>>> 
>> 
>> I feel your pain (honest!), but this is enough of a weird edge case
>> that I think any kind of minversion() business is enough of an
>> improvement that we should run with it for now, and worry about this later.
>> 
>>> 
>>> 
>>> On Fri, Nov 27, 2015 at 8:47 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>>>> On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
>>>>> # HG changeset patch
>>>>> # User Gregory Szorc <gregory.szorc at gmail.com>
>>>>> # Date 1448406985 28800
>>>>> #      Tue Nov 24 15:16:25 2015 -0800
>>>>> # Node ID 7493ec7396d172cdbed8bc3b1aa300b2d2adc4df
>>>>> # Parent  ed2e8713d5f11c64bf9d5c9b6a0d802b5d534fbe
>>>>> extensions: refuse to load extensions if minimum hg version not met
>>>>> 
>>>>> As the author of several 3rd party extensions, I frequently see bug
>>>>> reports from users attempting to run my extension with an old version
>>>>> of Mercurial that I no longer support in my extension. Oftentimes, the
>>>>> extension will import just fine. But as soon as we run extsetup(),
>>>>> reposetup(), or get into the guts of a wrapped function, we encounter
>>>>> an exception and abort. Today, Mercurial will print a message about
>>>>> extensions that don't have a "testedwith" declaring explicit
>>>>> compatibility with the current version.
>>>>> 
>>>>> The existing mechanism is a good start. But it isn't as robust as I
>>>>> would like. Specifically, Mercurial assumes compatibility by default.
>>>>> This means extension authors must perform compatibility checking in
>>>>> their extsetup() or we wait and see if we encounter an abort at
>>>>> runtime. And, compatibility checking can involve a lot of code and
>>>>> lots of error checking. It's a lot of effort for extension authors.
>>>>> 
>>>>> Oftentimes, extension authors know which versions of Mercurial there
>>>>> extension works on and more importantly where it is broken.
>>>>> 
>>>>> This patch introduces a magic "minimumhgversion" attribute in
>>>>> extensions. When found, the extension loading mechanism will compare
>>>>> the declared version against the current Mercurial version. If the
>>>>> extension explicitly states we require a newer Mercurial version, a
>>>>> warning is printed and the extension isn't loaded beyond importing
>>>>> the Python module. This causes a graceful failure while alerting
>>>>> the user of the compatibility issue.
>>>>> 
>>>>> I would be receptive to the idea of making the failure more fatal.
>>>>> However, care would need to be taken to not criple every hg command.
>>>>> e.g. the user may use `hg config` to fix the hgrc and if we aborted
>>>>> trying to run that, the user would effectively be locked out of `hg`!
>>>>> 
>>>>> A potential future improvement to this functionality would be to catch
>>>>> ImportError for the extension/module and parse the source code for
>>>>> "minimumhgversion = 'XXX'" and do similar checking. This way we could
>>>>> give more information about why the extension failed to load.
>>>> 
>>>> I think this patch is good start, but I want to see others' opinion.
>>>> 
>>>>> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
>>>>> --- a/mercurial/extensions.py
>>>>> +++ b/mercurial/extensions.py
>>>>> @@ -99,8 +99,19 @@ def load(ui, name, path):
>>>>>                      % (name, err, name))
>>>>>             if ui.debugflag:
>>>>>                 ui.traceback()
>>>>>             mod = importh(name)
>>>>> +
>>>>> +    # Before we do anything with the extension, check against minimum stated
>>>>> +    # compatibility. This gives extension authors a mechanism to have their
>>>>> +    # extensions short circuit when loaded with a known incompatible version
>>>>> +    # of Mercurial.
>>>>> +    minver = getattr(mod, 'minimumhgversion', None)
>>>>> +    if minver and util.versiontuple(minver, 2) > util.versiontuple(n=2):
>>>>> +        ui.warn(_('(third party extension %s requires version %s or newer '
>>>>> +                  'of Mercurial; disabling)\n') % (shortname, minver))
>>>>> +        return
>>>> 
>>>> Nitpick: extension authors might want to specify the 3rd digit because we
>>>> sometimes change the internal API in stable releases.
>>>> _______________________________________________
>>>> Mercurial-devel mailing list
>>>> Mercurial-devel at selenic.com
>>>> https://selenic.com/mailman/listinfo/mercurial-devel
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at selenic.com
>>> https://selenic.com/mailman/listinfo/mercurial-devel

More information about the Mercurial-devel mailing list