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

timeless timeless at gmail.com
Mon Nov 30 16:31:23 CST 2015 

Let me put this differently.

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