From 7769ddd52c338ad7e369ef1c404e1bd63a3b63f5 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 22 Jun 2018 08:16:41 +0530 Subject: [PATCH 1/9] Rename deprecation warnings to not be versioned --- src/pip/_internal/utils/deprecation.py | 51 +++++++++++--------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index 89b66e9ad..a1644c778 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -16,50 +16,43 @@ class PipDeprecationWarning(Warning): pass -class Pending(object): +class PipPendingDeprecationWarning(PipDeprecationWarning): pass -class RemovedInPip12Warning(PipDeprecationWarning, Pending): - pass - - -# Warnings <-> Logging Integration - - _warnings_showwarning = None # type: Any +# Warnings <-> Logging Integration def _showwarning(message, category, filename, lineno, file=None, line=None): if file is not None: if _warnings_showwarning is not None: _warnings_showwarning( message, category, filename, lineno, file, line, ) - else: - if issubclass(category, PipDeprecationWarning): - # We use a specially named logger which will handle all of the - # deprecation messages for pip. - logger = logging.getLogger("pip._internal.deprecations") + elif issubclass(category, PipDeprecationWarning): + # We use a specially named logger which will handle all of the + # deprecation messages for pip. + logger = logging.getLogger("pip._internal.deprecations") - # This is purposely using the % formatter here instead of letting - # the logging module handle the interpolation. This is because we - # want it to appear as if someone typed this entire message out. - log_message = "DEPRECATION: %s" % message + # This is purposely using the % formatter here instead of letting + # the logging module handle the interpolation. This is because we + # want it to appear as if someone typed this entire message out. + log_message = "DEPRECATION: %s" % message - # PipDeprecationWarnings that are Pending still have at least 2 - # versions to go until they are removed so they can just be - # warnings. Otherwise, they will be removed in the very next - # version of pip. We want these to be more obvious so we use the - # ERROR logging level. - if issubclass(category, Pending): - logger.warning(log_message) - else: - logger.error(log_message) + # PipPendingDeprecationWarnings still have at least 2 + # versions to go until they are removed so they can just be + # warnings. Otherwise, they will be removed in the very next + # version of pip. We want these to be more obvious so we use the + # ERROR logging level. + if issubclass(category, PipPendingDeprecationWarning): + logger.warning(log_message) else: - _warnings_showwarning( - message, category, filename, lineno, file, line, - ) + logger.error(log_message) + else: + _warnings_showwarning( + message, category, filename, lineno, file, line, + ) def install_warning_logger(): From a8eaf1e0ff013b9e61e4420b28477f05470ff0ae Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 22 Jun 2018 09:30:17 +0530 Subject: [PATCH 2/9] Add a helper function for deprecation (and use it) --- src/pip/_internal/index.py | 8 ++++---- src/pip/_internal/operations/freeze.py | 8 ++++---- src/pip/_internal/req/req_install.py | 9 +++++---- src/pip/_internal/utils/deprecation.py | 20 +++++++++++++++++++- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index 857486cc1..fae26eeff 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -9,7 +9,6 @@ import os import posixpath import re import sys -import warnings from collections import namedtuple from pip._vendor import html5lib, requests, six @@ -29,7 +28,7 @@ from pip._internal.exceptions import ( ) from pip._internal.models.index import PyPI from pip._internal.pep425tags import get_supported -from pip._internal.utils.deprecation import RemovedInPip12Warning +from pip._internal.utils.deprecation import deprecated from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import ( ARCHIVE_EXTENSIONS, SUPPORTED_EXTENSIONS, cached_property, normalize_path, @@ -212,10 +211,11 @@ class PackageFinder(object): # # dependency_links value # # FIXME: also, we should track comes_from (i.e., use Link) if self.process_dependency_links: - warnings.warn( + deprecated( "Dependency Links processing has been deprecated and will be " "removed in a future release.", - RemovedInPip12Warning, + replacement=None, + issue=4187, ) self.dependency_links.extend(links) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 82d0a8c4b..80fdd1044 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -4,7 +4,6 @@ import collections import logging import os import re -import warnings from pip._vendor import pkg_resources, six from pip._vendor.packaging.utils import canonicalize_name @@ -13,7 +12,7 @@ from pip._vendor.pkg_resources import RequirementParseError from pip._internal.exceptions import InstallationError from pip._internal.req import InstallRequirement from pip._internal.req.req_file import COMMENT_RE -from pip._internal.utils.deprecation import RemovedInPip12Warning +from pip._internal.utils.deprecation import deprecated from pip._internal.utils.misc import ( dist_is_editable, get_installed_distributions, ) @@ -216,10 +215,11 @@ class FrozenRequirement(object): 'for this package:' ) else: - warnings.warn( + deprecated( "SVN editable detection based on dependency links " "will be dropped in the future.", - RemovedInPip12Warning, + replacement=None, + issue=4187, ) comments.append( '# Installing as editable to satisfy requirement %s:' % diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 8838dd2eb..6a33e4e42 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -8,7 +8,6 @@ import shutil import sys import sysconfig import traceback -import warnings import zipfile from distutils.util import change_root from email.parser import FeedParser # type: ignore @@ -33,7 +32,7 @@ from pip._internal.locations import ( PIP_DELETE_MARKER_FILENAME, running_under_virtualenv, ) from pip._internal.req.req_uninstall import UninstallPathSet -from pip._internal.utils.deprecation import RemovedInPip12Warning +from pip._internal.utils.deprecation import deprecated from pip._internal.utils.hashes import Hashes from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import ( @@ -582,11 +581,13 @@ class InstallRequirement(object): if requires is None: logging.warn(template, self, "it is missing.") - warnings.warn( + deprecated( "Future versions of pip may reject packages with " "pyproject.toml files that do not contain the [build-system]" "table and the requires key, as specified in PEP 518.", - RemovedInPip12Warning, + replacement=None, + issue=5416, + imminent=True, ) # Currently, we're isolating the build based on the presence of the diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index a1644c778..271e6d9fc 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -9,7 +9,7 @@ import warnings from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Any # noqa: F401 + from typing import Any, Optional # noqa: F401 class PipDeprecationWarning(Warning): @@ -64,3 +64,21 @@ def install_warning_logger(): if _warnings_showwarning is None: _warnings_showwarning = warnings.showwarning warnings.showwarning = _showwarning + + +def deprecated(reason, replacement, issue=None, imminent=False): + # type: (str, Optional[str], Optional[int], bool) -> None + if imminent: + category = PipDeprecationWarning + else: + category = PipPendingDeprecationWarning + + # Construct a nice message. + message = reason + if replacement is not None: + message += " An alternative is to {}.".format(replacement) + if issue is not None: + url = "https://github.com/pypa/pip/issues/" + str(issue) + message += " You can find discussion regarding this at {}.".format(url) + + warnings.warn(message, category=category, stacklevel=2) From a1912454b8eb3bbbc3347089da59543b700165a7 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 22 Jun 2018 09:31:34 +0530 Subject: [PATCH 3/9] Rename variable for clarity It doesn't represent warnings.showwarning's current state so, the name shouldn't suggest that. --- src/pip/_internal/utils/deprecation.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index 271e6d9fc..94b3caa0a 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -20,14 +20,14 @@ class PipPendingDeprecationWarning(PipDeprecationWarning): pass -_warnings_showwarning = None # type: Any +_original_showwarning = None # type: Any # Warnings <-> Logging Integration def _showwarning(message, category, filename, lineno, file=None, line=None): if file is not None: - if _warnings_showwarning is not None: - _warnings_showwarning( + if _original_showwarning is not None: + _original_showwarning( message, category, filename, lineno, file, line, ) elif issubclass(category, PipDeprecationWarning): @@ -50,7 +50,7 @@ def _showwarning(message, category, filename, lineno, file=None, line=None): else: logger.error(log_message) else: - _warnings_showwarning( + _original_showwarning( message, category, filename, lineno, file, line, ) @@ -59,10 +59,10 @@ def install_warning_logger(): # Enable our Deprecation Warnings warnings.simplefilter("default", PipDeprecationWarning, append=True) - global _warnings_showwarning + global _original_showwarning - if _warnings_showwarning is None: - _warnings_showwarning = warnings.showwarning + if _original_showwarning is None: + _original_showwarning = warnings.showwarning warnings.showwarning = _showwarning From 4c2b268d52e84452e785468dfbfabac80874ee47 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 6 Jul 2018 04:41:19 +0530 Subject: [PATCH 4/9] Add "DEPRECATION" to start of message when composing it --- src/pip/_internal/utils/deprecation.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index 94b3caa0a..e8f59705c 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -35,11 +35,6 @@ def _showwarning(message, category, filename, lineno, file=None, line=None): # deprecation messages for pip. logger = logging.getLogger("pip._internal.deprecations") - # This is purposely using the % formatter here instead of letting - # the logging module handle the interpolation. This is because we - # want it to appear as if someone typed this entire message out. - log_message = "DEPRECATION: %s" % message - # PipPendingDeprecationWarnings still have at least 2 # versions to go until they are removed so they can just be # warnings. Otherwise, they will be removed in the very next @@ -74,7 +69,9 @@ def deprecated(reason, replacement, issue=None, imminent=False): category = PipPendingDeprecationWarning # Construct a nice message. - message = reason + # This is purposely eagerly formatted as we want it to appear as if someone + # typed this entire message out. + message = "DEPRECATION: " + reason if replacement is not None: message += " An alternative is to {}.".format(replacement) if issue is not None: From e027cd84494b27da6e404dd03cba1a61bb4a0b60 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 6 Jul 2018 04:47:33 +0530 Subject: [PATCH 5/9] Update deprecation utilities to specify versions Also: - Remove conditional warning/error level logging - Remove now-obsolete class - Update call sites as per new signature --- src/pip/_internal/index.py | 1 + src/pip/_internal/operations/freeze.py | 1 + src/pip/_internal/req/req_install.py | 2 +- src/pip/_internal/utils/deprecation.py | 34 ++++++++++---------------- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py index fae26eeff..8c0ec82c0 100644 --- a/src/pip/_internal/index.py +++ b/src/pip/_internal/index.py @@ -215,6 +215,7 @@ class PackageFinder(object): "Dependency Links processing has been deprecated and will be " "removed in a future release.", replacement=None, + gone_in="18.2", issue=4187, ) self.dependency_links.extend(links) diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py index 80fdd1044..4bbc27b0a 100644 --- a/src/pip/_internal/operations/freeze.py +++ b/src/pip/_internal/operations/freeze.py @@ -219,6 +219,7 @@ class FrozenRequirement(object): "SVN editable detection based on dependency links " "will be dropped in the future.", replacement=None, + gone_in="18.2", issue=4187, ) comments.append( diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 6a33e4e42..efe96a774 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -586,8 +586,8 @@ class InstallRequirement(object): "pyproject.toml files that do not contain the [build-system]" "table and the requires key, as specified in PEP 518.", replacement=None, + gone_in="18.2", issue=5416, - imminent=True, ) # Currently, we're isolating the build based on the presence of the diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index e8f59705c..73bf69cd8 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -6,6 +6,9 @@ from __future__ import absolute_import import logging import warnings +from pip._vendor.packaging.version import parse + +from pip import __version__ as current_version from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: @@ -16,10 +19,6 @@ class PipDeprecationWarning(Warning): pass -class PipPendingDeprecationWarning(PipDeprecationWarning): - pass - - _original_showwarning = None # type: Any @@ -34,16 +33,7 @@ def _showwarning(message, category, filename, lineno, file=None, line=None): # We use a specially named logger which will handle all of the # deprecation messages for pip. logger = logging.getLogger("pip._internal.deprecations") - - # PipPendingDeprecationWarnings still have at least 2 - # versions to go until they are removed so they can just be - # warnings. Otherwise, they will be removed in the very next - # version of pip. We want these to be more obvious so we use the - # ERROR logging level. - if issubclass(category, PipPendingDeprecationWarning): - logger.warning(log_message) - else: - logger.error(log_message) + logger.warning(message) else: _original_showwarning( message, category, filename, lineno, file, line, @@ -61,12 +51,11 @@ def install_warning_logger(): warnings.showwarning = _showwarning -def deprecated(reason, replacement, issue=None, imminent=False): - # type: (str, Optional[str], Optional[int], bool) -> None - if imminent: - category = PipDeprecationWarning - else: - category = PipPendingDeprecationWarning +def deprecated(reason, replacement, gone_in, issue=None): + # type: (str, Optional[str], Optional[str], Optional[int]) -> None + """Helper to deprecate existing functionality. + """ + # NOTE: treat replacement, gone_in, issue as keyword only arguments. # Construct a nice message. # This is purposely eagerly formatted as we want it to appear as if someone @@ -78,4 +67,7 @@ def deprecated(reason, replacement, issue=None, imminent=False): url = "https://github.com/pypa/pip/issues/" + str(issue) message += " You can find discussion regarding this at {}.".format(url) - warnings.warn(message, category=category, stacklevel=2) + # Raise as an error if it has to be removed. + if gone_in is not None and parse(current_version) >= parse(gone_in): + raise PipDeprecationWarning(message) + warnings.warn(message, category=PipDeprecationWarning, stacklevel=2) From 3e0e45ba61955d84dd373585116e948ef86b5dc3 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 6 Jul 2018 04:42:23 +0530 Subject: [PATCH 6/9] Nicer language in line with parameter name --- src/pip/_internal/utils/deprecation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index 73bf69cd8..d6a20fd57 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -62,7 +62,7 @@ def deprecated(reason, replacement, gone_in, issue=None): # typed this entire message out. message = "DEPRECATION: " + reason if replacement is not None: - message += " An alternative is to {}.".format(replacement) + message += " A possible replacement is {}.".format(replacement) if issue is not None: url = "https://github.com/pypa/pip/issues/" + str(issue) message += " You can find discussion regarding this at {}.".format(url) From a84dde598297495fe6f0f8b233b3a3761b0df7d4 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Fri, 6 Jul 2018 14:17:56 +0530 Subject: [PATCH 7/9] Update test to check newer logic --- tests/functional/test_warning.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/functional/test_warning.py b/tests/functional/test_warning.py index 20f246795..6c097c75b 100644 --- a/tests/functional/test_warning.py +++ b/tests/functional/test_warning.py @@ -1,21 +1,22 @@ +import textwrap + def test_environ(script, tmpdir): """$PYTHONWARNINGS was added in python2.7""" demo = tmpdir.join('warnings_demo.py') - demo.write(''' -from pip._internal.utils import deprecation -deprecation.install_warning_logger() + demo.write(textwrap.dedent(''' + from logging import basicConfig + from pip._internal.utils import deprecation -from logging import basicConfig -basicConfig() + deprecation.install_warning_logger() + basicConfig() -from warnings import warn -warn("deprecated!", deprecation.PipDeprecationWarning) -''') + deprecation.deprecated("deprecated!", replacement=None, gone_in=None) + ''')) result = script.run('python', demo, expect_stderr=True) - assert result.stderr == \ - 'ERROR:pip._internal.deprecations:DEPRECATION: deprecated!\n' + expected = 'WARNING:pip._internal.deprecations:DEPRECATION: deprecated!\n' + assert result.stderr == expected script.environ['PYTHONWARNINGS'] = 'ignore' result = script.run('python', demo) From 28f183e1a26f5f9091b68b32bca0d10e3564b8a5 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 15 Jul 2018 15:47:10 +0530 Subject: [PATCH 8/9] Document parameters of the deprecated helper --- src/pip/_internal/utils/deprecation.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index d6a20fd57..bd744cf2c 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -54,8 +54,24 @@ def install_warning_logger(): def deprecated(reason, replacement, gone_in, issue=None): # type: (str, Optional[str], Optional[str], Optional[int]) -> None """Helper to deprecate existing functionality. + + reason: + Textual reason shown to the user about why this functionality has + been deprecated. + replacement: + Textual suggestion shown to the user about what alternative + functionality they can use. + gone_in: + The version of pip does this functionality should get removed in. + Raises errors if pip's current version is greater than or equal to + this. + issue: + Issue number on the tracker that would serve as a useful place for + users to find related discussion and provide feedback. + + Always pass replacement, gone_in and issue as keyword arguments for clarity + at the call site. """ - # NOTE: treat replacement, gone_in, issue as keyword only arguments. # Construct a nice message. # This is purposely eagerly formatted as we want it to appear as if someone From b6dae7a5949ec67c16678f0a367cc8452f10d3b0 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 15 Jul 2018 15:52:34 +0530 Subject: [PATCH 9/9] Mention the helper along with pip's deprecation policy --- docs/development.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/development.rst b/docs/development.rst index bc1d36777..5b1ad1370 100644 --- a/docs/development.rst +++ b/docs/development.rst @@ -211,6 +211,12 @@ document existing behavior with the intention of covering that behavior with the above deprecation process are always acceptable, and will be considered on their merits. +.. note:: + + pip has a helper function for making deprecation easier for pip maintainers. + The supporting documentation can be found in the source code of + ``pip._internal.utils.deprecation.deprecated``. The function is not a part of + pip's public API. Release Process ===============