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

[Pierre-Yves David]

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 at 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,

-- 
Pierre-Yves David