Patchwork [1,of,2,V2] util: add a way to issue deprecation warning without a UI object

login
register
mail settings
Submitter Pierre-Yves David
Date April 4, 2017, 9:58 a.m.
Message ID <3992b843a7832bf23f24.1491299929@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/19949/
State Deferred
Headers show

Comments

Pierre-Yves David - April 4, 2017, 9:58 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1491296609 -7200
#      Tue Apr 04 11:03:29 2017 +0200
# Node ID 3992b843a7832bf23f2430dc2308907d1f5ba7f9
# Parent  2632df096fc0ac7582382b1f94ea4b9ad0bce8f2
# EXP-Topic vfs.cleanup
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3992b843a783
util: add a way to issue deprecation warning without a UI object

Our current deprecation warning mechanism rely on ui object. They are case where
we cannot have access to the UI object. On a general basis we avoid using the
python mechanism for deprecation warning because up to Python 2.6 it is exposing
warning to unsuspecting user who cannot do anything to deal with them.

So we build a "safe" strategy to hide this warnings behind a flag in an
environment variable. The test runner set this flag so that tests show these
warning.  This will help us marker API as deprecated for extensions to update
their code.
Yuya Nishihara - April 6, 2017, 1:58 p.m.
On Tue, 04 Apr 2017 11:58:49 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1491296609 -7200
> #      Tue Apr 04 11:03:29 2017 +0200
> # Node ID 3992b843a7832bf23f2430dc2308907d1f5ba7f9
> # Parent  2632df096fc0ac7582382b1f94ea4b9ad0bce8f2
> # EXP-Topic vfs.cleanup
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3992b843a783
> util: add a way to issue deprecation warning without a UI object
> 
> Our current deprecation warning mechanism rely on ui object. They are case where
> we cannot have access to the UI object. On a general basis we avoid using the
> python mechanism for deprecation warning because up to Python 2.6 it is exposing
> warning to unsuspecting user who cannot do anything to deal with them.
> 
> So we build a "safe" strategy to hide this warnings behind a flag in an
> environment variable. The test runner set this flag so that tests show these
> warning.  This will help us marker API as deprecated for extensions to update
> their code.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -38,6 +38,7 @@ import tempfile
>  import textwrap
>  import time
>  import traceback
> +import warnings
>  import zlib
>  
>  from . import (
> @@ -155,6 +156,31 @@ def bitsfrom(container):
>          bits |= bit
>      return bits
>  
> +# python 2.6 still have deprecation warning enabled by default. We do not want
> +# to display anything to standard user so detect if we are running test and
> +# only use python deprecation warning in this case.
> +_dowarn = bool(encoding.environ.get('HGEMITWARNINGS'))
> +if _dowarn:
> +    # explicitly unfilter our warning for python 2.7
> +    #
> +    # The option of setting PYTHONWARNINGS in the test runner was investigated.
> +    # However, module name set through PYTHONWARNINGS was exactly matched, so
> +    # we cannot set 'mercurial' and have it match eg: 'mercurial.scmutil'. This
> +    # makes the whole PYTHONWARNINGS thing useless for our usecase.
> +    warnings.filterwarnings('default', '', DeprecationWarning, 'mercurial')
> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext')
> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext3rd')
> +
> +def nouideprecwarn(msg, version, stacklevel=1):
> +    """Issue an python native deprecation warning
> +
> +    This is a noop outside of tests, use 'ui.deprecwarn' when possible.
> +    """
> +    if _dowarn:
> +        msg += ("\n(compatibility will be dropped after Mercurial-%s,"
> +                " update your code.)") % version
> +        warnings.warn(msg, DeprecationWarning, stacklevel + 1)

Sorry for late reply, but can this DeprecationWarning really reach extension
authors who need the warning?

If dirty hack allowed, I would do something like the following:

  # util.py
  def _deprecwarn(msg, version):
      pass

  # somewhere ui is available, maybe in dispatch.py
  util._deprecwarn = ui.deprecwarn
Pierre-Yves David - April 6, 2017, 2:09 p.m.
On 04/06/2017 03:58 PM, Yuya Nishihara wrote:
> On Tue, 04 Apr 2017 11:58:49 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1491296609 -7200
>> #      Tue Apr 04 11:03:29 2017 +0200
>> # Node ID 3992b843a7832bf23f2430dc2308907d1f5ba7f9
>> # Parent  2632df096fc0ac7582382b1f94ea4b9ad0bce8f2
>> # EXP-Topic vfs.cleanup
>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3992b843a783
>> util: add a way to issue deprecation warning without a UI object
>>
>> Our current deprecation warning mechanism rely on ui object. They are case where
>> we cannot have access to the UI object. On a general basis we avoid using the
>> python mechanism for deprecation warning because up to Python 2.6 it is exposing
>> warning to unsuspecting user who cannot do anything to deal with them.
>>
>> So we build a "safe" strategy to hide this warnings behind a flag in an
>> environment variable. The test runner set this flag so that tests show these
>> warning.  This will help us marker API as deprecated for extensions to update
>> their code.
>>
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -38,6 +38,7 @@ import tempfile
>>  import textwrap
>>  import time
>>  import traceback
>> +import warnings
>>  import zlib
>>
>>  from . import (
>> @@ -155,6 +156,31 @@ def bitsfrom(container):
>>          bits |= bit
>>      return bits
>>
>> +# python 2.6 still have deprecation warning enabled by default. We do not want
>> +# to display anything to standard user so detect if we are running test and
>> +# only use python deprecation warning in this case.
>> +_dowarn = bool(encoding.environ.get('HGEMITWARNINGS'))
>> +if _dowarn:
>> +    # explicitly unfilter our warning for python 2.7
>> +    #
>> +    # The option of setting PYTHONWARNINGS in the test runner was investigated.
>> +    # However, module name set through PYTHONWARNINGS was exactly matched, so
>> +    # we cannot set 'mercurial' and have it match eg: 'mercurial.scmutil'. This
>> +    # makes the whole PYTHONWARNINGS thing useless for our usecase.
>> +    warnings.filterwarnings('default', '', DeprecationWarning, 'mercurial')
>> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext')
>> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext3rd')
>> +
>> +def nouideprecwarn(msg, version, stacklevel=1):
>> +    """Issue an python native deprecation warning
>> +
>> +    This is a noop outside of tests, use 'ui.deprecwarn' when possible.
>> +    """
>> +    if _dowarn:
>> +        msg += ("\n(compatibility will be dropped after Mercurial-%s,"
>> +                " update your code.)") % version
>> +        warnings.warn(msg, DeprecationWarning, stacklevel + 1)
>
> Sorry for late reply, but can this DeprecationWarning really reach extension
> authors who need the warning?

The test runner have been updated to make sure they appears and official 
recommendation is to use the test runner from the core repository.

So extensions author following the good practice will get the warning 
(I've caught a couple already in client extensions). The other one will 
get bitten and have one extra incentive to follow the good practice :-)

>
> If dirty hack allowed, I would do something like the following:
>
>   # util.py
>   def _deprecwarn(msg, version):
>       pass
>
>   # somewhere ui is available, maybe in dispatch.py
>   util._deprecwarn = ui.deprecwarn

That is a diry hack. I would prefer we did not used it.
Jun Wu - April 6, 2017, 3:03 p.m.
Excerpts from Yuya Nishihara's message of 2017-04-06 22:58:24 +0900:
> If dirty hack allowed, I would do something like the following:
> 
>   # util.py
>   def _deprecwarn(msg, version):
>       pass
> 
>   # somewhere ui is available, maybe in dispatch.py
>   util._deprecwarn = ui.deprecwarn

I think this is actually better since it works better with chg -
util._deprecwarn could be replaced without restarting.
Yuya Nishihara - April 6, 2017, 3:44 p.m.
On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
> 
> 
> On 04/06/2017 03:58 PM, Yuya Nishihara wrote:
> > On Tue, 04 Apr 2017 11:58:49 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> >> # Date 1491296609 -7200
> >> #      Tue Apr 04 11:03:29 2017 +0200
> >> # Node ID 3992b843a7832bf23f2430dc2308907d1f5ba7f9
> >> # Parent  2632df096fc0ac7582382b1f94ea4b9ad0bce8f2
> >> # EXP-Topic vfs.cleanup
> >> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> >> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3992b843a783
> >> util: add a way to issue deprecation warning without a UI object
> >>
> >> Our current deprecation warning mechanism rely on ui object. They are case where
> >> we cannot have access to the UI object. On a general basis we avoid using the
> >> python mechanism for deprecation warning because up to Python 2.6 it is exposing
> >> warning to unsuspecting user who cannot do anything to deal with them.
> >>
> >> So we build a "safe" strategy to hide this warnings behind a flag in an
> >> environment variable. The test runner set this flag so that tests show these
> >> warning.  This will help us marker API as deprecated for extensions to update
> >> their code.
> >>
> >> diff --git a/mercurial/util.py b/mercurial/util.py
> >> --- a/mercurial/util.py
> >> +++ b/mercurial/util.py
> >> @@ -38,6 +38,7 @@ import tempfile
> >>  import textwrap
> >>  import time
> >>  import traceback
> >> +import warnings
> >>  import zlib
> >>
> >>  from . import (
> >> @@ -155,6 +156,31 @@ def bitsfrom(container):
> >>          bits |= bit
> >>      return bits
> >>
> >> +# python 2.6 still have deprecation warning enabled by default. We do not want
> >> +# to display anything to standard user so detect if we are running test and
> >> +# only use python deprecation warning in this case.
> >> +_dowarn = bool(encoding.environ.get('HGEMITWARNINGS'))
> >> +if _dowarn:
> >> +    # explicitly unfilter our warning for python 2.7
> >> +    #
> >> +    # The option of setting PYTHONWARNINGS in the test runner was investigated.
> >> +    # However, module name set through PYTHONWARNINGS was exactly matched, so
> >> +    # we cannot set 'mercurial' and have it match eg: 'mercurial.scmutil'. This
> >> +    # makes the whole PYTHONWARNINGS thing useless for our usecase.
> >> +    warnings.filterwarnings('default', '', DeprecationWarning, 'mercurial')
> >> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext')
> >> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext3rd')
> >> +
> >> +def nouideprecwarn(msg, version, stacklevel=1):
> >> +    """Issue an python native deprecation warning
> >> +
> >> +    This is a noop outside of tests, use 'ui.deprecwarn' when possible.
> >> +    """
> >> +    if _dowarn:
> >> +        msg += ("\n(compatibility will be dropped after Mercurial-%s,"
> >> +                " update your code.)") % version
> >> +        warnings.warn(msg, DeprecationWarning, stacklevel + 1)
> >
> > Sorry for late reply, but can this DeprecationWarning really reach extension
> > authors who need the warning?
> 
> The test runner have been updated to make sure they appears and official 
> recommendation is to use the test runner from the core repository.
> 
> So extensions author following the good practice will get the warning 
> (I've caught a couple already in client extensions). The other one will 
> get bitten and have one extra incentive to follow the good practice :-)

You are the core developer. I believe many extensions wouldn't have tests,
and if they do, the authors wouldn't run tests unless they change their
code. But a develwarn will hopefully be noticed while running. For example,
you've fixed several opener uses in TortoiseHg probably because you'd see
warnings.

> > If dirty hack allowed, I would do something like the following:
> >
> >   # util.py
> >   def _deprecwarn(msg, version):
> >       pass
> >
> >   # somewhere ui is available, maybe in dispatch.py
> >   util._deprecwarn = ui.deprecwarn
> 
> That is a diry hack. I would prefer we did not used it.

Yeah, that is dirty and I don't like it. But I'm okay with it as long as
it is a temporary hack.
Pierre-Yves David - April 7, 2017, 5:03 p.m.
On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
>>
>> On 04/06/2017 03:58 PM, Yuya Nishihara wrote:
>>> On Tue, 04 Apr 2017 11:58:49 +0200, Pierre-Yves David wrote:
>>>> # HG changeset patch
>>>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>>>> # Date 1491296609 -7200
>>>> #      Tue Apr 04 11:03:29 2017 +0200
>>>> # Node ID 3992b843a7832bf23f2430dc2308907d1f5ba7f9
>>>> # Parent  2632df096fc0ac7582382b1f94ea4b9ad0bce8f2
>>>> # EXP-Topic vfs.cleanup
>>>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3992b843a783
>>>> util: add a way to issue deprecation warning without a UI object
>>>>
>>>> Our current deprecation warning mechanism rely on ui object. They are case where
>>>> we cannot have access to the UI object. On a general basis we avoid using the
>>>> python mechanism for deprecation warning because up to Python 2.6 it is exposing
>>>> warning to unsuspecting user who cannot do anything to deal with them.
>>>>
>>>> So we build a "safe" strategy to hide this warnings behind a flag in an
>>>> environment variable. The test runner set this flag so that tests show these
>>>> warning.  This will help us marker API as deprecated for extensions to update
>>>> their code.
>>>>
>>>> diff --git a/mercurial/util.py b/mercurial/util.py
>>>> --- a/mercurial/util.py
>>>> +++ b/mercurial/util.py
>>>> @@ -38,6 +38,7 @@ import tempfile
>>>>  import textwrap
>>>>  import time
>>>>  import traceback
>>>> +import warnings
>>>>  import zlib
>>>>
>>>>  from . import (
>>>> @@ -155,6 +156,31 @@ def bitsfrom(container):
>>>>          bits |= bit
>>>>      return bits
>>>>
>>>> +# python 2.6 still have deprecation warning enabled by default. We do not want
>>>> +# to display anything to standard user so detect if we are running test and
>>>> +# only use python deprecation warning in this case.
>>>> +_dowarn = bool(encoding.environ.get('HGEMITWARNINGS'))
>>>> +if _dowarn:
>>>> +    # explicitly unfilter our warning for python 2.7
>>>> +    #
>>>> +    # The option of setting PYTHONWARNINGS in the test runner was investigated.
>>>> +    # However, module name set through PYTHONWARNINGS was exactly matched, so
>>>> +    # we cannot set 'mercurial' and have it match eg: 'mercurial.scmutil'. This
>>>> +    # makes the whole PYTHONWARNINGS thing useless for our usecase.
>>>> +    warnings.filterwarnings('default', '', DeprecationWarning, 'mercurial')
>>>> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext')
>>>> +    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext3rd')
>>>> +
>>>> +def nouideprecwarn(msg, version, stacklevel=1):
>>>> +    """Issue an python native deprecation warning
>>>> +
>>>> +    This is a noop outside of tests, use 'ui.deprecwarn' when possible.
>>>> +    """
>>>> +    if _dowarn:
>>>> +        msg += ("\n(compatibility will be dropped after Mercurial-%s,"
>>>> +                " update your code.)") % version
>>>> +        warnings.warn(msg, DeprecationWarning, stacklevel + 1)
>>>
>>> Sorry for late reply, but can this DeprecationWarning really reach extension
>>> authors who need the warning?
>>
>> The test runner have been updated to make sure they appears and official
>> recommendation is to use the test runner from the core repository.
>>
>> So extensions author following the good practice will get the warning
>> (I've caught a couple already in client extensions). The other one will
>> get bitten and have one extra incentive to follow the good practice :-)
>
> You are the core developer. I believe many extensions wouldn't have tests,
> and if they do, the authors wouldn't run tests unless they change their
> code.

Having tests and running them for each new Mercurial release is also 
something I would put into good pratice o:-)

> But a develwarn will hopefully be noticed while running. For example,
> you've fixed several opener uses in TortoiseHg probably because you'd see
> warnings.

I've more hope of developer running tests than in developer knowing 
about the 'devel.all-warnings=1' option. I've have it set because I'm 
the one who introduced it. I wonder who else does?

But I see your point, using the same function as for the rest increase 
chance of people seeing them.

>>> If dirty hack allowed, I would do something like the following:
>>>
>>>   # util.py
>>>   def _deprecwarn(msg, version):
>>>       pass
>>>
>>>   # somewhere ui is available, maybe in dispatch.py
>>>   util._deprecwarn = ui.deprecwarn
>>
>> That is a diry hack. I would prefer we did not used it.
>
> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
> it is a temporary hack.

If you think the dirty hack is worth the potential extra exposure, I'm 
fine with it.

However, I'm confused about your usage of "temporary hack" here. Why are 
you using temporary?

Cheers,
Yuya Nishihara - April 8, 2017, 8:16 a.m.
On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
> > On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
> >>> If dirty hack allowed, I would do something like the following:
> >>>
> >>>   # util.py
> >>>   def _deprecwarn(msg, version):
> >>>       pass
> >>>
> >>>   # somewhere ui is available, maybe in dispatch.py
> >>>   util._deprecwarn = ui.deprecwarn
> >>
> >> That is a diry hack. I would prefer we did not used it.
> >
> > Yeah, that is dirty and I don't like it. But I'm okay with it as long as
> > it is a temporary hack.
> 
> If you think the dirty hack is worth the potential extra exposure, I'm 
> fine with it.
> 
> However, I'm confused about your usage of "temporary hack" here. Why are 
> you using temporary?

I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
sure if a permanent one is necessary.
Pierre-Yves David - April 8, 2017, 9:37 a.m.
On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
> On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
>> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
>>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
>>>>> If dirty hack allowed, I would do something like the following:
>>>>>
>>>>>   # util.py
>>>>>   def _deprecwarn(msg, version):
>>>>>       pass
>>>>>
>>>>>   # somewhere ui is available, maybe in dispatch.py
>>>>>   util._deprecwarn = ui.deprecwarn
>>>>
>>>> That is a diry hack. I would prefer we did not used it.
>>>
>>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
>>> it is a temporary hack.
>>
>> If you think the dirty hack is worth the potential extra exposure, I'm
>> fine with it.
>>
>> However, I'm confused about your usage of "temporary hack" here. Why are
>> you using temporary?
>
> I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
> sure if a permanent one is necessary.

I think this kind of needs will appears again in the future. So I would 
not flag the solution as temporary. I would have made the function local 
to the scmutil module.

So I would rather have a long terms solution. What do you think?

Cheers,
Yuya Nishihara - April 9, 2017, 1:08 p.m.
On Sat, 8 Apr 2017 11:37:20 +0200, Pierre-Yves David wrote:
> On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
> > On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
> >> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
> >>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
> >>>>> If dirty hack allowed, I would do something like the following:
> >>>>>
> >>>>>   # util.py
> >>>>>   def _deprecwarn(msg, version):
> >>>>>       pass
> >>>>>
> >>>>>   # somewhere ui is available, maybe in dispatch.py
> >>>>>   util._deprecwarn = ui.deprecwarn
> >>>>
> >>>> That is a diry hack. I would prefer we did not used it.
> >>>
> >>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
> >>> it is a temporary hack.
> >>
> >> If you think the dirty hack is worth the potential extra exposure, I'm
> >> fine with it.
> >>
> >> However, I'm confused about your usage of "temporary hack" here. Why are
> >> you using temporary?
> >
> > I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
> > sure if a permanent one is necessary.
> 
> I think this kind of needs will appears again in the future. So I would 
> not flag the solution as temporary. I would have made the function local 
> to the scmutil module.
> 
> So I would rather have a long terms solution. What do you think?

If we need less ad-hoc one, I would add a global flag to enable deprecwarn
and set it only by global configuration (i.e. ui, not lui nor repo.ui.)
And perhaps will remove ui.deprecwarn() in favor of util.deprecwarn().
Pierre-Yves David - April 10, 2017, 1:41 p.m.
On 04/09/2017 03:08 PM, Yuya Nishihara wrote:
> On Sat, 8 Apr 2017 11:37:20 +0200, Pierre-Yves David wrote:
>> On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
>>> On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
>>>> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
>>>>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
>>>>>>> If dirty hack allowed, I would do something like the following:
>>>>>>>
>>>>>>>   # util.py
>>>>>>>   def _deprecwarn(msg, version):
>>>>>>>       pass
>>>>>>>
>>>>>>>   # somewhere ui is available, maybe in dispatch.py
>>>>>>>   util._deprecwarn = ui.deprecwarn
>>>>>>
>>>>>> That is a diry hack. I would prefer we did not used it.
>>>>>
>>>>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
>>>>> it is a temporary hack.
>>>>
>>>> If you think the dirty hack is worth the potential extra exposure, I'm
>>>> fine with it.
>>>>
>>>> However, I'm confused about your usage of "temporary hack" here. Why are
>>>> you using temporary?
>>>
>>> I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
>>> sure if a permanent one is necessary.
>>
>> I think this kind of needs will appears again in the future. So I would
>> not flag the solution as temporary. I would have made the function local
>> to the scmutil module.
>>
>> So I would rather have a long terms solution. What do you think?
>
> If we need less ad-hoc one, I would add a global flag to enable deprecwarn
> and set it only by global configuration (i.e. ui, not lui nor repo.ui.)

This seems a bit hacky (I also have some concerns about possible of 
deprecated function being created before the ui is. but I'm not sure 
they are funded).

> And perhaps will remove ui.deprecwarn() in favor of util.deprecwarn().

ui.deprecwarn is much more configurable and manageable that the global 
one (based on Python deprec warning). I do not think we should drop 
`ui.deprecwarc` in favor of the other one.

Cheers,
Yuya Nishihara - April 10, 2017, 2:35 p.m.
On Mon, 10 Apr 2017 15:41:08 +0200, Pierre-Yves David wrote:
> On 04/09/2017 03:08 PM, Yuya Nishihara wrote:
> > On Sat, 8 Apr 2017 11:37:20 +0200, Pierre-Yves David wrote:
> >> On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
> >>> On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
> >>>> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
> >>>>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
> >>>>>>> If dirty hack allowed, I would do something like the following:
> >>>>>>>
> >>>>>>>   # util.py
> >>>>>>>   def _deprecwarn(msg, version):
> >>>>>>>       pass
> >>>>>>>
> >>>>>>>   # somewhere ui is available, maybe in dispatch.py
> >>>>>>>   util._deprecwarn = ui.deprecwarn
> >>>>>>
> >>>>>> That is a diry hack. I would prefer we did not used it.
> >>>>>
> >>>>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
> >>>>> it is a temporary hack.
> >>>>
> >>>> If you think the dirty hack is worth the potential extra exposure, I'm
> >>>> fine with it.
> >>>>
> >>>> However, I'm confused about your usage of "temporary hack" here. Why are
> >>>> you using temporary?
> >>>
> >>> I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
> >>> sure if a permanent one is necessary.
> >>
> >> I think this kind of needs will appears again in the future. So I would
> >> not flag the solution as temporary. I would have made the function local
> >> to the scmutil module.
> >>
> >> So I would rather have a long terms solution. What do you think?
> >
> > If we need less ad-hoc one, I would add a global flag to enable deprecwarn
> > and set it only by global configuration (i.e. ui, not lui nor repo.ui.)
> 
> This seems a bit hacky (I also have some concerns about possible of 
> deprecated function being created before the ui is. but I'm not sure 
> they are funded).

IMHO it's as bad as using environment variable. Both are a kind of globals.

> > And perhaps will remove ui.deprecwarn() in favor of util.deprecwarn().
> 
> ui.deprecwarn is much more configurable and manageable that the global
> one (based on Python deprec warning). I do not think we should drop
> `ui.deprecwarc` in favor of the other one.

It's more configurable, but less easy to use because ui is required.

Anyway, I don't think it's worth spending time for designing the deprecwarn
feature. I'm okay for any of the ideas in this thread.
Pierre-Yves David - April 12, 2017, 11:14 p.m.
On 04/10/2017 04:35 PM, Yuya Nishihara wrote:
> On Mon, 10 Apr 2017 15:41:08 +0200, Pierre-Yves David wrote:
>> On 04/09/2017 03:08 PM, Yuya Nishihara wrote:
>>> On Sat, 8 Apr 2017 11:37:20 +0200, Pierre-Yves David wrote:
>>>> On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
>>>>> On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
>>>>>> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
>>>>>>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
>>>>>>>>> If dirty hack allowed, I would do something like the following:
>>>>>>>>>
>>>>>>>>>   # util.py
>>>>>>>>>   def _deprecwarn(msg, version):
>>>>>>>>>       pass
>>>>>>>>>
>>>>>>>>>   # somewhere ui is available, maybe in dispatch.py
>>>>>>>>>   util._deprecwarn = ui.deprecwarn
>>>>>>>>
>>>>>>>> That is a diry hack. I would prefer we did not used it.
>>>>>>>
>>>>>>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
>>>>>>> it is a temporary hack.
>>>>>>
>>>>>> If you think the dirty hack is worth the potential extra exposure, I'm
>>>>>> fine with it.
>>>>>>
>>>>>> However, I'm confused about your usage of "temporary hack" here. Why are
>>>>>> you using temporary?
>>>>>
>>>>> I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
>>>>> sure if a permanent one is necessary.
>>>>
>>>> I think this kind of needs will appears again in the future. So I would
>>>> not flag the solution as temporary. I would have made the function local
>>>> to the scmutil module.
>>>>
>>>> So I would rather have a long terms solution. What do you think?
>>>
>>> If we need less ad-hoc one, I would add a global flag to enable deprecwarn
>>> and set it only by global configuration (i.e. ui, not lui nor repo.ui.)
>>
>> This seems a bit hacky (I also have some concerns about possible of
>> deprecated function being created before the ui is. but I'm not sure
>> they are funded).
>
> IMHO it's as bad as using environment variable. Both are a kind of globals.

The global assignment possibly keeps an object alive longer than its due 
time. So I find it a bit hackier.

>>> And perhaps will remove ui.deprecwarn() in favor of util.deprecwarn().
>>
>> ui.deprecwarn is much more configurable and manageable that the global
>> one (based on Python deprec warning). I do not think we should drop
>> `ui.deprecwarc` in favor of the other one.
>
> It's more configurable, but less easy to use because ui is required.
>
> Anyway, I don't think it's worth spending time for designing the deprecwarn
> feature. I'm okay for any of the ideas in this thread.

This the current series is right here and implemented. Are you okay with 
moving forward with it?
Yuya Nishihara - April 13, 2017, 12:07 p.m.
On Thu, 13 Apr 2017 01:14:48 +0200, Pierre-Yves David wrote:
> 
> 
> On 04/10/2017 04:35 PM, Yuya Nishihara wrote:
> > On Mon, 10 Apr 2017 15:41:08 +0200, Pierre-Yves David wrote:
> >> On 04/09/2017 03:08 PM, Yuya Nishihara wrote:
> >>> On Sat, 8 Apr 2017 11:37:20 +0200, Pierre-Yves David wrote:
> >>>> On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
> >>>>> On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
> >>>>>> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
> >>>>>>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
> >>>>>>>>> If dirty hack allowed, I would do something like the following:
> >>>>>>>>>
> >>>>>>>>>   # util.py
> >>>>>>>>>   def _deprecwarn(msg, version):
> >>>>>>>>>       pass
> >>>>>>>>>
> >>>>>>>>>   # somewhere ui is available, maybe in dispatch.py
> >>>>>>>>>   util._deprecwarn = ui.deprecwarn
> >>>>>>>>
> >>>>>>>> That is a diry hack. I would prefer we did not used it.
> >>>>>>>
> >>>>>>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
> >>>>>>> it is a temporary hack.
> >>>>>>
> >>>>>> If you think the dirty hack is worth the potential extra exposure, I'm
> >>>>>> fine with it.
> >>>>>>
> >>>>>> However, I'm confused about your usage of "temporary hack" here. Why are
> >>>>>> you using temporary?
> >>>>>
> >>>>> I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
> >>>>> sure if a permanent one is necessary.
> >>>>
> >>>> I think this kind of needs will appears again in the future. So I would
> >>>> not flag the solution as temporary. I would have made the function local
> >>>> to the scmutil module.
> >>>>
> >>>> So I would rather have a long terms solution. What do you think?
> >>>
> >>> If we need less ad-hoc one, I would add a global flag to enable deprecwarn
> >>> and set it only by global configuration (i.e. ui, not lui nor repo.ui.)
> >>
> >> This seems a bit hacky (I also have some concerns about possible of
> >> deprecated function being created before the ui is. but I'm not sure
> >> they are funded).
> >
> > IMHO it's as bad as using environment variable. Both are a kind of globals.
> 
> The global assignment possibly keeps an object alive longer than its due 
> time. So I find it a bit hackier.

That's why I proposed a global boolean flag as an alternative.

> >>> And perhaps will remove ui.deprecwarn() in favor of util.deprecwarn().
> >>
> >> ui.deprecwarn is much more configurable and manageable that the global
> >> one (based on Python deprec warning). I do not think we should drop
> >> `ui.deprecwarc` in favor of the other one.
> >
> > It's more configurable, but less easy to use because ui is required.
> >
> > Anyway, I don't think it's worth spending time for designing the deprecwarn
> > feature. I'm okay for any of the ideas in this thread.
> 
> This the current series is right here and implemented. Are you okay with 
> moving forward with it?

Sure. Can you rebase these on tip? The first patch couldn't apply.
Pierre-Yves David - April 13, 2017, 2:45 p.m.
On 04/13/2017 02:07 PM, Yuya Nishihara wrote:
> On Thu, 13 Apr 2017 01:14:48 +0200, Pierre-Yves David wrote:
>>
>>
>> On 04/10/2017 04:35 PM, Yuya Nishihara wrote:
>>> On Mon, 10 Apr 2017 15:41:08 +0200, Pierre-Yves David wrote:
>>>> On 04/09/2017 03:08 PM, Yuya Nishihara wrote:
>>>>> On Sat, 8 Apr 2017 11:37:20 +0200, Pierre-Yves David wrote:
>>>>>> On 04/08/2017 10:16 AM, Yuya Nishihara wrote:
>>>>>>> On Fri, 7 Apr 2017 19:03:55 +0200, Pierre-Yves David wrote:
>>>>>>>> On 04/06/2017 05:44 PM, Yuya Nishihara wrote:
>>>>>>>>> On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote:
>>>>>>>>>>> If dirty hack allowed, I would do something like the following:
>>>>>>>>>>>
>>>>>>>>>>>   # util.py
>>>>>>>>>>>   def _deprecwarn(msg, version):
>>>>>>>>>>>       pass
>>>>>>>>>>>
>>>>>>>>>>>   # somewhere ui is available, maybe in dispatch.py
>>>>>>>>>>>   util._deprecwarn = ui.deprecwarn
>>>>>>>>>>
>>>>>>>>>> That is a diry hack. I would prefer we did not used it.
>>>>>>>>>
>>>>>>>>> Yeah, that is dirty and I don't like it. But I'm okay with it as long as
>>>>>>>>> it is a temporary hack.
>>>>>>>>
>>>>>>>> If you think the dirty hack is worth the potential extra exposure, I'm
>>>>>>>> fine with it.
>>>>>>>>
>>>>>>>> However, I'm confused about your usage of "temporary hack" here. Why are
>>>>>>>> you using temporary?
>>>>>>>
>>>>>>> I suppose the hack will hopefully disappear with the vfs compat layer. I'm not
>>>>>>> sure if a permanent one is necessary.
>>>>>>
>>>>>> I think this kind of needs will appears again in the future. So I would
>>>>>> not flag the solution as temporary. I would have made the function local
>>>>>> to the scmutil module.
>>>>>>
>>>>>> So I would rather have a long terms solution. What do you think?
>>>>>
>>>>> If we need less ad-hoc one, I would add a global flag to enable deprecwarn
>>>>> and set it only by global configuration (i.e. ui, not lui nor repo.ui.)
>>>>
>>>> This seems a bit hacky (I also have some concerns about possible of
>>>> deprecated function being created before the ui is. but I'm not sure
>>>> they are funded).
>>>
>>> IMHO it's as bad as using environment variable. Both are a kind of globals.
>>
>> The global assignment possibly keeps an object alive longer than its due
>> time. So I find it a bit hackier.
>
> That's why I proposed a global boolean flag as an alternative.

ha!, I somehow missed that proposal when reading you. That seems 
interesting. If I get back to this area, I'll investigate possible 
timing issue play with that approach

>
>>>>> And perhaps will remove ui.deprecwarn() in favor of util.deprecwarn().
>>>>
>>>> ui.deprecwarn is much more configurable and manageable that the global
>>>> one (based on Python deprec warning). I do not think we should drop
>>>> `ui.deprecwarc` in favor of the other one.
>>>
>>> It's more configurable, but less easy to use because ui is required.
>>>
>>> Anyway, I don't think it's worth spending time for designing the deprecwarn
>>> feature. I'm okay for any of the ideas in this thread.
>>
>> This the current series is right here and implemented. Are you okay with
>> moving forward with it?
>
> Sure. Can you rebase these on tip? The first patch couldn't apply.

Okay, V4 incoming soon.

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -38,6 +38,7 @@  import tempfile
 import textwrap
 import time
 import traceback
+import warnings
 import zlib
 
 from . import (
@@ -155,6 +156,31 @@  def bitsfrom(container):
         bits |= bit
     return bits
 
+# python 2.6 still have deprecation warning enabled by default. We do not want
+# to display anything to standard user so detect if we are running test and
+# only use python deprecation warning in this case.
+_dowarn = bool(encoding.environ.get('HGEMITWARNINGS'))
+if _dowarn:
+    # explicitly unfilter our warning for python 2.7
+    #
+    # The option of setting PYTHONWARNINGS in the test runner was investigated.
+    # However, module name set through PYTHONWARNINGS was exactly matched, so
+    # we cannot set 'mercurial' and have it match eg: 'mercurial.scmutil'. This
+    # makes the whole PYTHONWARNINGS thing useless for our usecase.
+    warnings.filterwarnings('default', '', DeprecationWarning, 'mercurial')
+    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext')
+    warnings.filterwarnings('default', '', DeprecationWarning, 'hgext3rd')
+
+def nouideprecwarn(msg, version, stacklevel=1):
+    """Issue an python native deprecation warning
+
+    This is a noop outside of tests, use 'ui.deprecwarn' when possible.
+    """
+    if _dowarn:
+        msg += ("\n(compatibility will be dropped after Mercurial-%s,"
+                " update your code.)") % version
+        warnings.warn(msg, DeprecationWarning, stacklevel + 1)
+
 DIGESTS = {
     'md5': hashlib.md5,
     'sha1': hashlib.sha1,
diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -877,6 +877,7 @@  class Test(unittest.TestCase):
         env = os.environ.copy()
         if sysconfig is not None:
             env['PYTHONUSERBASE'] = sysconfig.get_config_var('userbase')
+        env['HGEMITWARNINGS'] = '1'
         env['TESTTMP'] = self._testtmp
         env['HOME'] = self._testtmp
         # This number should match portneeded in _getport
diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t
--- a/tests/test-devel-warnings.t
+++ b/tests/test-devel-warnings.t
@@ -3,7 +3,7 @@ 
   > """A small extension that tests our developer warnings
   > """
   > 
-  > from mercurial import cmdutil, repair, revset
+  > from mercurial import cmdutil, repair, revset, util
   > 
   > cmdtable = {}
   > command = cmdutil.command(cmdtable)
@@ -63,6 +63,10 @@ 
   >     return list(subset)
   > 
   > revset.symbols['oldstyle'] = oldstylerevset
+  > 
+  > @command('nouiwarning', [], '')
+  > def nouiwarning(ui, repo):
+  >     util.nouideprecwarn('this is a test', '13.37')
   > EOF
 
   $ cat << EOF >> $HGRCPATH
@@ -176,4 +180,15 @@  Test programming error failure:
   Traceback (most recent call last):
   mercurial.error.ProgrammingError: transaction requires locking
 
+Old style deprecation warning
+
+  $ hg nouiwarning
+  $TESTTMP/buggylocking.py:67: DeprecationWarning: this is a test
+  (compatibility will be dropped after Mercurial-13.37, update your code.)
+    util.nouideprecwarn('this is a test', '13.37')
+
+(disabled outside of test run)
+
+  $ HGEMITWARNINGS= hg nouiwarning
+
   $ cd ..