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

login
register
mail settings
Submitter Gregory Szorc
Date Nov. 24, 2015, 11:19 p.m.
Message ID <7493ec7396d172cdbed8.1448407150@gps-mbp.local>
Download mbox | patch
Permalink /patch/11629/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Gregory Szorc - Nov. 24, 2015, 11:19 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@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.
Yuya Nishihara - Nov. 27, 2015, 1:47 p.m.
On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@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.
timeless - Nov. 27, 2015, 4:42 p.m.
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.


On Fri, Nov 27, 2015 at 8:47 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@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@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Nov. 30, 2015, 10:15 p.m.
On Fri, Nov 27, 2015 at 10:47:41PM +0900, Yuya Nishihara wrote:
> On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@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.

I really like this.

>
> > 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@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Nov. 30, 2015, 10:16 p.m.
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@tcha.org> wrote:
> > On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
> >> # HG changeset patch
> >> # User Gregory Szorc <gregory.szorc@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@selenic.com
> > https://selenic.com/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
timeless - Nov. 30, 2015, 10:31 p.m.
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@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@tcha.org> wrote:
>> > On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
>> >> # HG changeset patch
>> >> # User Gregory Szorc <gregory.szorc@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@selenic.com
>> > https://selenic.com/mailman/listinfo/mercurial-devel
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> https://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Nov. 30, 2015, 10:33 p.m.
> On Nov 30, 2015, at 17:31, timeless <timeless@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@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@tcha.org> wrote:
>>>> On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
>>>>> # HG changeset patch
>>>>> # User Gregory Szorc <gregory.szorc@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@selenic.com
>>>> https://selenic.com/mailman/listinfo/mercurial-devel
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@selenic.com
>>> https://selenic.com/mailman/listinfo/mercurial-devel
timeless - Nov. 30, 2015, 10:36 p.m.
On Mon, Nov 30, 2015 at 5:33 PM, Augie Fackler <raf@durin42.com> wrote:
>
>> On Nov 30, 2015, at 17:31, timeless <timeless@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.

ok.
Yuya Nishihara - Dec. 1, 2015, 1:28 p.m.
On Tue, 24 Nov 2015 15:19:10 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@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

Pushed this to the clowncopter. Thanks for additional reviewing.

Patch

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
+
     _extensions[shortname] = mod
     _order.append(shortname)
     for fn in _aftercallbacks.get(shortname, []):
         fn(loaded=True)
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -1008,8 +1008,52 @@  Test version number support in 'hg versi
   Enabled extensions:
   
     throw  1.twentythree
 
+Refuse to load extensions with minimum version requirements
+
+  $ cat > minversion1.py << EOF
+  > from mercurial import util
+  > util.version = lambda: '3.5.2'
+  > minimumhgversion = '3.6'
+  > EOF
+  $ hg --config extensions.minversion=minversion1.py version
+  (third party extension minversion requires version 3.6 or newer of Mercurial; disabling)
+  Mercurial Distributed SCM (version 3.5.2)
+  (see https://mercurial-scm.org for more information)
+  
+  Copyright (C) 2005-* Matt Mackall and others (glob)
+  This is free software; see the source for copying conditions. There is NO
+  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+
+  $ cat > minversion2.py << EOF
+  > from mercurial import util
+  > util.version = lambda: '3.6'
+  > minimumhgversion = '3.7'
+  > EOF
+  $ hg --config extensions.minversion=minversion2.py version 2>&1 | egrep '\(third'
+  (third party extension minversion requires version 3.7 or newer of Mercurial; disabling)
+
+Can load version that is only off by point release
+
+  $ cat > minversion2.py << EOF
+  > from mercurial import util
+  > util.version = lambda: '3.6.1'
+  > minimumhgversion = '3.6'
+  > EOF
+  $ hg --config extensions.minversion=minversion3.py version 2>&1 | egrep '\(third'
+  [1]
+
+Can load minimum version identical to current
+
+  $ cat > minversion3.py << EOF
+  > from mercurial import util
+  > util.version = lambda: '3.5'
+  > minimumhgversion = '3.5'
+  > EOF
+  $ hg --config extensions.minversion=minversion3.py version 2>&1 | egrep '\(third'
+  [1]
+
 Restore HGRCPATH
 
   $ HGRCPATH=$ORGHGRCPATH
   $ export HGRCPATH