Re: [PATCH] extensions: formalize concept of experimental extensions

[Yuya Nishihara]

On Tue, 21 Mar 2017 19:43:56 -0700, Gregory Szorc wrote:
> On Tue, Mar 21, 2017 at 10:59 AM, Ryan McElroy  wrote:
> 
> > On 3/15/17 1:10 PM, Yuya Nishihara wrote:
> >
> >> On Tue, 14 Mar 2017 08:45:44 -0700, Jun Wu wrote:
> >>
> >>> Excerpts from Yuya Nishihara's message of 2017-03-13 19:02:06 -0700:
> >>>
>  How about :allowbeta = True ?
> 
> >>> Old clients will try to load "True" as an extension.

Crazy idea.

  [extensions]
  :allowbeta = !True

"!" may be omitted so new user can just set :allowbeta = .
Perhaps config transformer will strip leading "!".

> FWIW, I think it might be a good practice on any new config section to
> iterate keys without sub-options (strings without ":"). That way, future
> versions can introduce sub-options to tweak per-item behavior without
> having to worry about backwards compatibility.

+1
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] extensions: formalize concept of experimental extensions

[Yuya Nishihara]

On Tue, 14 Mar 2017 08:45:44 -0700, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-03-13 19:02:06 -0700:
> > How about :allowbeta = True ?
> 
> Old clients will try to load "True" as an extension.

Ah, good catch.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] extensions: formalize concept of experimental extensions

[Jun Wu]

Excerpts from Yuya Nishihara's message of 2017-03-13 19:02:06 -0700:
> How about :allowbeta = True ?

Old clients will try to load "True" as an extension.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] extensions: formalize concept of experimental extensions

[Yuya Nishihara]

On Sun, 12 Mar 2017 19:48:35 -0700, Gregory Szorc wrote:
> > On Mar 12, 2017, at 19:28, Jun Wu  wrote:
> > 
> > If I read it correctly, this means an "experimental" extension will end up
> > with:
> > 
> > | ext=| ext=!beta
> >  
> >  old client | load| do not load
> >  new client | do not load | load
> > 
> > I think that's confusing.
> > 
> > I think we can have an "[extensions:experimental]" section instead, which
> > will only be processed by new mercurial.
> 
> For extensions distributed with Mercurial, old clients won't have the 
> extension. So the old client plus ext= scenario results in no load.

Old clients can load 3rd-party extensions from PYTHONPATH and/or hgext3rd.
If evolve get into the core as an experimental extension, I would have to
adjust my .hgrc per hg versions.

Another minor concern is that some people use "!" to disable their local
extensions. "!beta" seems confusing.

How about :allowbeta = True ?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] extensions: formalize concept of experimental extensions

[Jun Wu]

Excerpts from Gregory Szorc's message of 2017-03-12 19:48:35 -0700:
> 
> > On Mar 12, 2017, at 19:28, Jun Wu  wrote:
> > 
> > If I read it correctly, this means an "experimental" extension will end up
> > with:
> > 
> > | ext=| ext=!beta
> >  
> >  old client | load| do not load
> >  new client | do not load | load
> > 
> > I think that's confusing.
> > 
> > I think we can have an "[extensions:experimental]" section instead, which
> > will only be processed by new mercurial.
> 
> For extensions distributed with Mercurial, old clients won't have the
> extension. So the old client plus ext= scenario results in no load.

I think it's better to make the "experimental" thing usable for 3rd party
extensions too.

Another concern is for hg developers / testers running different versions of
hg - they may share hgrc.

> Of course, you probably didn't have ext= in the first place because it
> would emit an annoying warning on every command. So I doubt ext= for
> experimental extensions will occur much in the wild.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] extensions: formalize concept of experimental extensions

[Gregory Szorc]

> On Mar 12, 2017, at 19:28, Jun Wu  wrote:
> 
> If I read it correctly, this means an "experimental" extension will end up
> with:
> 
> | ext=| ext=!beta
>  
>  old client | load| do not load
>  new client | do not load | load
> 
> I think that's confusing.
> 
> I think we can have an "[extensions:experimental]" section instead, which
> will only be processed by new mercurial.

For extensions distributed with Mercurial, old clients won't have the 
extension. So the old client plus ext= scenario results in no load.

Of course, you probably didn't have ext= in the first place because it would 
emit an annoying warning on every command. So I doubt ext= for experimental 
extensions will occur much in the wild.

> 
> Excerpts from Gregory Szorc's message of 2017-03-12 17:41:49 -0700:
>> # HG changeset patch
>> # User Gregory Szorc 
>> # Date 1489364702 25200
>> #  Sun Mar 12 17:25:02 2017 -0700
>> # Node ID 6a85d5031daf1ab3a5cb3c6705a46367d5e0de29
>> # Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
>> extensions: formalize concept of experimental extensions
>> 
>> Per discussions at the Sprint, we would like to be more
>> welcoming to shipping experimental extensions with the official
>> Mercurial distribution so they are more easily available to
>> end-users.
>> 
>> A concern with "experimental" extensions is where to put them and
>> how to differentiate them as "experimental" to end-users.
>> 
>> One idea is to put them in a special location or name them in
>> such a way that "experimental" (or some other label) is in the
>> name end-users use to load them. A problem with this is that if
>> the extension "graduates" to fully-supported status, users have to
>> update their configs to load the extension at the new name (or
>> Mercurial maintains a mapping table, which excludes 3rd party
>> extensions from having this benefit).
>> 
>> This patch formalizes the concept of experimental extensions and
>> does so in a way that is user-friendly and allows experimental
>> extensions to gracefully graduate to non-experimental status without
>> end-user intervention. It does so through 2 mechanisms:
>> 
>> 1. Extensions now set the "experimental" attribute to mark themselves
>>   as experimental
>> 2. The extension loader only loads experimental extensions if the
>>   config option value of the extension is "!beta"
>> 
>> If a user attempts to load an experimental mechanism using the
>> normal "extensions.=" syntax, a warning message is printed
>> saying the extension is "in trial mode" and tells them how to
>> activate it. If the value is "!beta" (e.g. extensions.foo=!beta),
>> the extension is loaded. This requires explicit affirmation from
>> the end-user that an extension is "beta." This should mitigate
>> any surprises from a user using an extension without realizing it
>> is experimental.
>> 
>> Because the old extension loading code interpreted a leading "!"
>> as "do not load," the "!beta" syntax results in old clients not
>> loading experimental extensions. This is a graceful failure.
>> 
>> A drawback of using "!beta" is that an explicit path to the
>> extension cannot be specified at this time. This means the
>> extension's name must correspond to a module in the "hgext" or
>> "hgext3rd" packages. I think this is acceptable.
>> 
>> I purposefully chose to use "beta" for the end-user facing value.
>> From my experience, users are scared of the "experimental" label.
>> In most cases, "experimental" features in Mercurial are more stable
>> than the other end of the spectrum. So I wanted to use a label
>> that is more reflective of reality, isn't scary, and doesn't
>> require strong English knowledge. I consulted a thesaurus for
>> suitable synonyms of "experimental" and couldn't find anything
>> "just right." So, I used "beta." This is a common technical term
>> that most people relate to (thanks, Gmail!) and it accurately
>> reflects the state of most extensions that Mercurial will
>> distribute in "experimental" form. For users who don't know
>> what it means, the translated message printed without "!beta"
>> can provide more context.
>> 
>> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
>> --- a/mercurial/extensions.py
>> +++ b/mercurial/extensions.py
>> @@ -119,9 +119,9 @@ def _reportimporterror(ui, err, failed, 
>>  % (failed, _forbytes(err), next))
>> if ui.debugflag:
>> ui.traceback()
>> 
>> -def load(ui, name, path):
>> +def load(ui, name, path, allowexperimental=False):
>> if name.startswith('hgext.') or name.startswith('hgext/'):
>> shortname = name[6:]
>> else:
>> shortname = name
>> @@ -141,8 +141,15 @@ def load(ui, name, path):
>> ui.warn(_('(third party extension %s requires version %s or newer '
>>   'of Mercurial; disabling)\n') % (shortname, minver))
>> return
>> 
>> +experimental = 

Re: [PATCH] extensions: formalize concept of experimental extensions

[Jun Wu]

If I read it correctly, this means an "experimental" extension will end up
with:

 | ext=| ext=!beta
  
  old client | load| do not load
  new client | do not load | load

I think that's confusing.

I think we can have an "[extensions:experimental]" section instead, which
will only be processed by new mercurial.

Excerpts from Gregory Szorc's message of 2017-03-12 17:41:49 -0700:
> # HG changeset patch
> # User Gregory Szorc 
> # Date 1489364702 25200
> #  Sun Mar 12 17:25:02 2017 -0700
> # Node ID 6a85d5031daf1ab3a5cb3c6705a46367d5e0de29
> # Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
> extensions: formalize concept of experimental extensions
> 
> Per discussions at the Sprint, we would like to be more
> welcoming to shipping experimental extensions with the official
> Mercurial distribution so they are more easily available to
> end-users.
> 
> A concern with "experimental" extensions is where to put them and
> how to differentiate them as "experimental" to end-users.
> 
> One idea is to put them in a special location or name them in
> such a way that "experimental" (or some other label) is in the
> name end-users use to load them. A problem with this is that if
> the extension "graduates" to fully-supported status, users have to
> update their configs to load the extension at the new name (or
> Mercurial maintains a mapping table, which excludes 3rd party
> extensions from having this benefit).
> 
> This patch formalizes the concept of experimental extensions and
> does so in a way that is user-friendly and allows experimental
> extensions to gracefully graduate to non-experimental status without
> end-user intervention. It does so through 2 mechanisms:
> 
> 1. Extensions now set the "experimental" attribute to mark themselves
>as experimental
> 2. The extension loader only loads experimental extensions if the
>config option value of the extension is "!beta"
> 
> If a user attempts to load an experimental mechanism using the
> normal "extensions.=" syntax, a warning message is printed
> saying the extension is "in trial mode" and tells them how to
> activate it. If the value is "!beta" (e.g. extensions.foo=!beta),
> the extension is loaded. This requires explicit affirmation from
> the end-user that an extension is "beta." This should mitigate
> any surprises from a user using an extension without realizing it
> is experimental.
> 
> Because the old extension loading code interpreted a leading "!"
> as "do not load," the "!beta" syntax results in old clients not
> loading experimental extensions. This is a graceful failure.
> 
> A drawback of using "!beta" is that an explicit path to the
> extension cannot be specified at this time. This means the
> extension's name must correspond to a module in the "hgext" or
> "hgext3rd" packages. I think this is acceptable.
> 
> I purposefully chose to use "beta" for the end-user facing value.
> From my experience, users are scared of the "experimental" label.
> In most cases, "experimental" features in Mercurial are more stable
> than the other end of the spectrum. So I wanted to use a label
> that is more reflective of reality, isn't scary, and doesn't
> require strong English knowledge. I consulted a thesaurus for
> suitable synonyms of "experimental" and couldn't find anything
> "just right." So, I used "beta." This is a common technical term
> that most people relate to (thanks, Gmail!) and it accurately
> reflects the state of most extensions that Mercurial will
> distribute in "experimental" form. For users who don't know
> what it means, the translated message printed without "!beta"
> can provide more context.
> 
> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
> --- a/mercurial/extensions.py
> +++ b/mercurial/extensions.py
> @@ -119,9 +119,9 @@ def _reportimporterror(ui, err, failed, 
>   % (failed, _forbytes(err), next))
>  if ui.debugflag:
>  ui.traceback()
>  
> -def load(ui, name, path):
> +def load(ui, name, path, allowexperimental=False):
>  if name.startswith('hgext.') or name.startswith('hgext/'):
>  shortname = name[6:]
>  else:
>  shortname = name
> @@ -141,8 +141,15 @@ def load(ui, name, path):
>  ui.warn(_('(third party extension %s requires version %s or newer '
>'of Mercurial; disabling)\n') % (shortname, minver))
>  return
>  
> +experimental = getattr(mod, 'experimental', None)
> +if experimental and not allowexperimental:
> +ui.warn(_('(extension %s is in trial mode and requires explicit '
> +  'opt-in by setting extensions.%s=!beta; disabling)\n') %
> +(name, name))
> +return
> +
>  _extensions[shortname] = mod
>  _order.append(shortname)
>  for fn in _aftercallbacks.get(shortname, []):
>  fn(loaded=True)
> @@ -166,14 +173,18 @@ def _runextsetup(name, ui):

[PATCH] extensions: formalize concept of experimental extensions

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1489364702 25200
#  Sun Mar 12 17:25:02 2017 -0700
# Node ID 6a85d5031daf1ab3a5cb3c6705a46367d5e0de29
# Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
extensions: formalize concept of experimental extensions

Per discussions at the Sprint, we would like to be more
welcoming to shipping experimental extensions with the official
Mercurial distribution so they are more easily available to
end-users.

A concern with "experimental" extensions is where to put them and
how to differentiate them as "experimental" to end-users.

One idea is to put them in a special location or name them in
such a way that "experimental" (or some other label) is in the
name end-users use to load them. A problem with this is that if
the extension "graduates" to fully-supported status, users have to
update their configs to load the extension at the new name (or
Mercurial maintains a mapping table, which excludes 3rd party
extensions from having this benefit).

This patch formalizes the concept of experimental extensions and
does so in a way that is user-friendly and allows experimental
extensions to gracefully graduate to non-experimental status without
end-user intervention. It does so through 2 mechanisms:

1. Extensions now set the "experimental" attribute to mark themselves
   as experimental
2. The extension loader only loads experimental extensions if the
   config option value of the extension is "!beta"

If a user attempts to load an experimental mechanism using the
normal "extensions.=" syntax, a warning message is printed
saying the extension is "in trial mode" and tells them how to
activate it. If the value is "!beta" (e.g. extensions.foo=!beta),
the extension is loaded. This requires explicit affirmation from
the end-user that an extension is "beta." This should mitigate
any surprises from a user using an extension without realizing it
is experimental.

Because the old extension loading code interpreted a leading "!"
as "do not load," the "!beta" syntax results in old clients not
loading experimental extensions. This is a graceful failure.

A drawback of using "!beta" is that an explicit path to the
extension cannot be specified at this time. This means the
extension's name must correspond to a module in the "hgext" or
"hgext3rd" packages. I think this is acceptable.

I purposefully chose to use "beta" for the end-user facing value.
From my experience, users are scared of the "experimental" label.
In most cases, "experimental" features in Mercurial are more stable
than the other end of the spectrum. So I wanted to use a label
that is more reflective of reality, isn't scary, and doesn't
require strong English knowledge. I consulted a thesaurus for
suitable synonyms of "experimental" and couldn't find anything
"just right." So, I used "beta." This is a common technical term
that most people relate to (thanks, Gmail!) and it accurately
reflects the state of most extensions that Mercurial will
distribute in "experimental" form. For users who don't know
what it means, the translated message printed without "!beta"
can provide more context.

diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -119,9 +119,9 @@ def _reportimporterror(ui, err, failed, 
  % (failed, _forbytes(err), next))
 if ui.debugflag:
 ui.traceback()
 
-def load(ui, name, path):
+def load(ui, name, path, allowexperimental=False):
 if name.startswith('hgext.') or name.startswith('hgext/'):
 shortname = name[6:]
 else:
 shortname = name
@@ -141,8 +141,15 @@ def load(ui, name, path):
 ui.warn(_('(third party extension %s requires version %s or newer '
   'of Mercurial; disabling)\n') % (shortname, minver))
 return
 
+experimental = getattr(mod, 'experimental', None)
+if experimental and not allowexperimental:
+ui.warn(_('(extension %s is in trial mode and requires explicit '
+  'opt-in by setting extensions.%s=!beta; disabling)\n') %
+(name, name))
+return
+
 _extensions[shortname] = mod
 _order.append(shortname)
 for fn in _aftercallbacks.get(shortname, []):
 fn(loaded=True)
@@ -166,14 +173,18 @@ def _runextsetup(name, ui):
 def loadall(ui):
 result = ui.configitems("extensions")
 newindex = len(_order)
 for (name, path) in result:
+allowexperimental = False
 if path:
-if path[0:1] == '!':
+if path.startswith('!beta'):
+allowexperimental = True
+path = ''
+elif path[0:1] == '!':
 _disabledextensions[name] = path[1:]
 continue
 try:
-load(ui, name, path)
+load(ui, name, path, allowexperimental=allowexperimental)
 except KeyboardInterrupt:
 raise
 except Exception as inst: