From 747f48e2e92390c44c72f52a1239959601cde157 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 12 Dec 2017 22:59:48 +0100 Subject: bpo-32230: Set sys.warnoptions with -X dev (#4820) Rather than supporting dev mode directly in the warnings module, this instead adjusts the initialisation code to add an extra 'default' entry to sys.warnoptions when dev mode is enabled. This ensures that dev mode behaves *exactly* as if `-Wdefault` had been passed on the command line, including in the way it interacts with `sys.warnoptions`, and with other command line flags like `-bb`. Fix also bpo-20361: have -b & -bb options take precedence over any other warnings options. Patch written by Nick Coghlan, with minor modifications of Victor Stinner. --- Doc/tools/susp-ignored.csv | 3 + Doc/using/cmdline.rst | 6 +- Doc/whatsnew/3.7.rst | 42 ++++++++--- Lib/subprocess.py | 18 +++-- Lib/test/test_cmd_line.py | 81 ++++++++++++++++------ Lib/test/test_warnings/__init__.py | 15 ++-- Lib/unittest/test/test_runner.py | 2 +- Lib/warnings.py | 35 ++-------- .../2017-12-06-20-18-34.bpo-32230.PgGQaB.rst | 3 + .../2017-12-07-17-22-30.bpo-20361.zQUmbi.rst | 4 ++ Modules/main.c | 66 ++++++++++++++++++ Python/_warnings.c | 73 ++++++------------- 12 files changed, 224 insertions(+), 124 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-12-06-20-18-34.bpo-32230.PgGQaB.rst create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-12-07-17-22-30.bpo-20361.zQUmbi.rst diff --git a/Doc/tools/susp-ignored.csv b/Doc/tools/susp-ignored.csv index d52f81b..48dd53f 100644 --- a/Doc/tools/susp-ignored.csv +++ b/Doc/tools/susp-ignored.csv @@ -327,3 +327,6 @@ whatsnew/changelog,,:end,str[start:end] library/binascii,,`,'`' library/uu,,`,'`' whatsnew/3.7,,`,'`' +whatsnew/3.7,,::,error::BytesWarning +whatsnew/changelog,,::,error::BytesWarning +whatsnew/changelog,,::,default::BytesWarning diff --git a/Doc/using/cmdline.rst b/Doc/using/cmdline.rst index 716bc82..e32f77e 100644 --- a/Doc/using/cmdline.rst +++ b/Doc/using/cmdline.rst @@ -430,11 +430,7 @@ Miscellaneous options not be more verbose than the default if the code is correct: new warnings are only emitted when an issue is detected. Effect of the developer mode: - * Warning filters: add a filter to display all warnings (``"default"`` - action), except of :exc:`BytesWarning` which still depends on the - :option:`-b` option, and use ``"always"`` action for - :exc:`ResourceWarning` warnings. For example, display - :exc:`DeprecationWarning` warnings. + * Add ``default`` warning filter, as :option:`-W` ``default``. * Install debug hooks on memory allocators: see the :c:func:`PyMem_SetupDebugHooks` C function. * Enable the :mod:`faulthandler` module to dump the Python traceback diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst index 3487662..58bfaef 100644 --- a/Doc/whatsnew/3.7.rst +++ b/Doc/whatsnew/3.7.rst @@ -188,14 +188,11 @@ resolution on Linux and Windows. New Development Mode: -X dev ---------------------------- -Add a new "development mode": ``-X dev`` command line option to enable debug -checks at runtime. - -In short, ``python3 -X dev ...`` behaves as ``PYTHONMALLOC=debug python3 -W -default -X faulthandler ...``, except that the PYTHONMALLOC environment -variable is not set in practice. - -See :option:`-X` ``dev`` for the details. +Add a new "development mode": :option:`-X` ``dev`` command line option and +:envvar:`PYTHONDEVMODE` environment variable to enable CPython's "development +mode", introducing additional runtime checks which are too expensive to be +enabled by default. See :option:`-X` ``dev`` documentation for the effects of +the development mode. Hash-based pycs --------------- @@ -481,6 +478,29 @@ Function :func:`~uu.encode` now accepts an optional *backtick* keyword argument. When it's true, zeros are represented by ``'`'`` instead of spaces. (Contributed by Xiang Zhang in :issue:`30103`.) +warnings +-------- + +The initialization of the default warnings filters has changed as follows: + +* warnings enabled via command line options (including those for :option:`-b` + and the new CPython-specific ``-X dev`` option) are always passed to the + warnings machinery via the ``sys.warnoptions`` attribute. +* warnings filters enabled via the command line or the environment now have the + following precedence order: + + * the ``BytesWarning`` filter for :option:`-b` (or ``-bb``) + * any filters specified with :option:`-W` + * any filters specified with :envvar:`PYTHONWARNINGS` + * any other CPython specific filters (e.g. the ``default`` filter added + for the new ``-X dev`` mode) + * any implicit filters defined directly by the warnings machinery +* in CPython debug builds, all warnings are now displayed by default (the + implicit filter list is empty) + +(Contributed by Nick Coghlan and Victor Stinner in :issue:`20361`, +:issue:`32043`, and :issue:`32230`) + xml.etree --------- @@ -854,6 +874,12 @@ Other CPython implementation changes either in embedding applications, or in CPython itself. (Contributed by Nick Coghlan and Eric Snow as part of :issue:`22257`.) +* Due to changes in the way the default warnings filters are configured, + setting ``Py_BytesWarningFlag`` to a value greater than one is no longer + sufficient to both emit ``BytesWarning`` messages and have them converted + to exceptions. Instead, the flag must be set (to cause the warnings to be + emitted in the first place), and an explicit ``error::BytesWarning`` + warnings filter added to convert them to exceptions. Documentation ============= diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 35bfddd..301433c 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -241,7 +241,7 @@ def _optim_args_from_interpreter_flags(): def _args_from_interpreter_flags(): """Return a list of command-line arguments reproducing the current - settings in sys.flags and sys.warnoptions.""" + settings in sys.flags, sys.warnoptions and sys._xoptions.""" flag_opt_map = { 'debug': 'd', # 'inspect': 'i', @@ -262,12 +262,22 @@ def _args_from_interpreter_flags(): args.append('-' + opt * v) # -W options - for opt in sys.warnoptions: + warnopts = sys.warnoptions[:] + bytes_warning = sys.flags.bytes_warning + xoptions = getattr(sys, '_xoptions', {}) + dev_mode = ('dev' in xoptions) + + if bytes_warning > 1: + warnopts.remove("error::BytesWarning") + elif bytes_warning: + warnopts.remove("default::BytesWarning") + if dev_mode: + warnopts.remove('default') + for opt in warnopts: args.append('-W' + opt) # -X options - xoptions = getattr(sys, '_xoptions', {}) - if 'dev' in xoptions: + if dev_mode: args.extend(('-X', 'dev')) for opt in ('faulthandler', 'tracemalloc', 'importtime', 'showalloccount', 'showrefcount'): diff --git a/Lib/test/test_cmd_line.py b/Lib/test/test_cmd_line.py index 383302b..2aff51b 100644 --- a/Lib/test/test_cmd_line.py +++ b/Lib/test/test_cmd_line.py @@ -14,6 +14,11 @@ from test.support.script_helper import ( interpreter_requires_environment ) + +# Debug build? +Py_DEBUG = hasattr(sys, "gettotalrefcount") + + # XXX (ncoghlan): Move to script_helper and make consistent with run_python def _kill_python_and_exit_code(p): data = kill_python(p) @@ -97,7 +102,7 @@ class CmdLineTest(unittest.TestCase): # "-X showrefcount" shows the refcount, but only in debug builds rc, out, err = run_python('-X', 'showrefcount', '-c', code) self.assertEqual(out.rstrip(), b"{'showrefcount': True}") - if hasattr(sys, 'gettotalrefcount'): # debug build + if Py_DEBUG: self.assertRegex(err, br'^\[\d+ refs, \d+ blocks\]') else: self.assertEqual(err, b'') @@ -541,31 +546,26 @@ class CmdLineTest(unittest.TestCase): code = ("import sys, warnings; " "print(' '.join('%s::%s' % (f[0], f[2].__name__) " "for f in warnings.filters))") + if Py_DEBUG: + expected_filters = "default::Warning" + else: + expected_filters = ("default::Warning " + "ignore::DeprecationWarning " + "ignore::PendingDeprecationWarning " + "ignore::ImportWarning " + "ignore::ResourceWarning") out = self.run_xdev("-c", code) - self.assertEqual(out, - "ignore::BytesWarning " - "default::ResourceWarning " - "default::Warning") + self.assertEqual(out, expected_filters) out = self.run_xdev("-b", "-c", code) - self.assertEqual(out, - "default::BytesWarning " - "default::ResourceWarning " - "default::Warning") + self.assertEqual(out, f"default::BytesWarning {expected_filters}") out = self.run_xdev("-bb", "-c", code) - self.assertEqual(out, - "error::BytesWarning " - "default::ResourceWarning " - "default::Warning") + self.assertEqual(out, f"error::BytesWarning {expected_filters}") out = self.run_xdev("-Werror", "-c", code) - self.assertEqual(out, - "error::Warning " - "ignore::BytesWarning " - "default::ResourceWarning " - "default::Warning") + self.assertEqual(out, f"error::Warning {expected_filters}") # Memory allocator debug hooks try: @@ -592,6 +592,46 @@ class CmdLineTest(unittest.TestCase): out = self.run_xdev("-c", code) self.assertEqual(out, "True") + def check_warnings_filters(self, cmdline_option, envvar, use_pywarning=False): + if use_pywarning: + code = ("import sys; from test.support import import_fresh_module; " + "warnings = import_fresh_module('warnings', blocked=['_warnings']); ") + else: + code = "import sys, warnings; " + code += ("print(' '.join('%s::%s' % (f[0], f[2].__name__) " + "for f in warnings.filters))") + args = (sys.executable, '-W', cmdline_option, '-bb', '-c', code) + env = dict(os.environ) + env.pop('PYTHONDEVMODE', None) + env["PYTHONWARNINGS"] = envvar + proc = subprocess.run(args, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True, + env=env) + self.assertEqual(proc.returncode, 0, proc) + return proc.stdout.rstrip() + + def test_warnings_filter_precedence(self): + expected_filters = ("error::BytesWarning " + "once::UserWarning " + "always::UserWarning") + if not Py_DEBUG: + expected_filters += (" " + "ignore::DeprecationWarning " + "ignore::PendingDeprecationWarning " + "ignore::ImportWarning " + "ignore::ResourceWarning") + + out = self.check_warnings_filters("once::UserWarning", + "always::UserWarning") + self.assertEqual(out, expected_filters) + + out = self.check_warnings_filters("once::UserWarning", + "always::UserWarning", + use_pywarning=True) + self.assertEqual(out, expected_filters) + def check_pythonmalloc(self, env_var, name): code = 'import _testcapi; print(_testcapi.pymem_getallocatorsname())' env = dict(os.environ) @@ -611,13 +651,12 @@ class CmdLineTest(unittest.TestCase): def test_pythonmalloc(self): # Test the PYTHONMALLOC environment variable - pydebug = hasattr(sys, "gettotalrefcount") pymalloc = support.with_pymalloc() if pymalloc: - default_name = 'pymalloc_debug' if pydebug else 'pymalloc' + default_name = 'pymalloc_debug' if Py_DEBUG else 'pymalloc' default_name_debug = 'pymalloc_debug' else: - default_name = 'malloc_debug' if pydebug else 'malloc' + default_name = 'malloc_debug' if Py_DEBUG else 'malloc' default_name_debug = 'malloc_debug' tests = [ diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index e60bc4d..039c96e 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -1110,20 +1110,23 @@ class EnvironmentVariableTests(BaseTest): def test_single_warning(self): rc, stdout, stderr = assert_python_ok("-c", "import sys; sys.stdout.write(str(sys.warnoptions))", - PYTHONWARNINGS="ignore::DeprecationWarning") + PYTHONWARNINGS="ignore::DeprecationWarning", + PYTHONDEVMODE="") self.assertEqual(stdout, b"['ignore::DeprecationWarning']") def test_comma_separated_warnings(self): rc, stdout, stderr = assert_python_ok("-c", "import sys; sys.stdout.write(str(sys.warnoptions))", - PYTHONWARNINGS="ignore::DeprecationWarning,ignore::UnicodeWarning") + PYTHONWARNINGS="ignore::DeprecationWarning,ignore::UnicodeWarning", + PYTHONDEVMODE="") self.assertEqual(stdout, b"['ignore::DeprecationWarning', 'ignore::UnicodeWarning']") def test_envvar_and_command_line(self): rc, stdout, stderr = assert_python_ok("-Wignore::UnicodeWarning", "-c", "import sys; sys.stdout.write(str(sys.warnoptions))", - PYTHONWARNINGS="ignore::DeprecationWarning") + PYTHONWARNINGS="ignore::DeprecationWarning", + PYTHONDEVMODE="") self.assertEqual(stdout, b"['ignore::DeprecationWarning', 'ignore::UnicodeWarning']") @@ -1131,7 +1134,8 @@ class EnvironmentVariableTests(BaseTest): rc, stdout, stderr = assert_python_failure("-Werror::DeprecationWarning", "-c", "import sys, warnings; sys.stdout.write(str(sys.warnoptions)); " "warnings.warn('Message', DeprecationWarning)", - PYTHONWARNINGS="default::DeprecationWarning") + PYTHONWARNINGS="default::DeprecationWarning", + PYTHONDEVMODE="") self.assertEqual(stdout, b"['default::DeprecationWarning', 'error::DeprecationWarning']") self.assertEqual(stderr.splitlines(), @@ -1145,7 +1149,8 @@ class EnvironmentVariableTests(BaseTest): rc, stdout, stderr = assert_python_ok("-c", "import sys; sys.stdout.write(str(sys.warnoptions))", PYTHONIOENCODING="utf-8", - PYTHONWARNINGS="ignore:DeprecaciónWarning") + PYTHONWARNINGS="ignore:DeprecaciónWarning", + PYTHONDEVMODE="") self.assertEqual(stdout, "['ignore:DeprecaciónWarning']".encode('utf-8')) diff --git a/Lib/unittest/test/test_runner.py b/Lib/unittest/test/test_runner.py index ddc498c..3c40056 100644 --- a/Lib/unittest/test/test_runner.py +++ b/Lib/unittest/test/test_runner.py @@ -289,7 +289,7 @@ class Test_TextTestRunner(unittest.TestCase): at_msg = b'Please use assertTrue instead.' # no args -> all the warnings are printed, unittest warnings only once - p = subprocess.Popen([sys.executable, '_test_warnings.py'], **opts) + p = subprocess.Popen([sys.executable, '-E', '_test_warnings.py'], **opts) with p: out, err = get_parse_out_err(p) self.assertIn(b'OK', err) diff --git a/Lib/warnings.py b/Lib/warnings.py index 4e7241f..f4331c8 100644 --- a/Lib/warnings.py +++ b/Lib/warnings.py @@ -519,34 +519,11 @@ except ImportError: # Module initialization _processoptions(sys.warnoptions) if not _warnings_defaults: - dev_mode = ('dev' in getattr(sys, '_xoptions', {})) - py_debug = hasattr(sys, 'gettotalrefcount') - - if not(dev_mode or py_debug): - silence = [ImportWarning, PendingDeprecationWarning] - silence.append(DeprecationWarning) - for cls in silence: - simplefilter("ignore", category=cls) - - bytes_warning = sys.flags.bytes_warning - if bytes_warning > 1: - bytes_action = "error" - elif bytes_warning: - bytes_action = "default" - else: - bytes_action = "ignore" - simplefilter(bytes_action, category=BytesWarning, append=1) - - # resource usage warnings are enabled by default in pydebug mode - if dev_mode or py_debug: - resource_action = "default" - else: - resource_action = "ignore" - simplefilter(resource_action, category=ResourceWarning, append=1) - - if dev_mode: - simplefilter("default", category=Warning, append=1) - - del py_debug, dev_mode + # Several warning categories are ignored by default in Py_DEBUG builds + if not hasattr(sys, 'gettotalrefcount'): + simplefilter("ignore", category=DeprecationWarning, append=1) + simplefilter("ignore", category=PendingDeprecationWarning, append=1) + simplefilter("ignore", category=ImportWarning, append=1) + simplefilter("ignore", category=ResourceWarning, append=1) del _warnings_defaults diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-12-06-20-18-34.bpo-32230.PgGQaB.rst b/Misc/NEWS.d/next/Core and Builtins/2017-12-06-20-18-34.bpo-32230.PgGQaB.rst new file mode 100644 index 0000000..b8656e4 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-12-06-20-18-34.bpo-32230.PgGQaB.rst @@ -0,0 +1,3 @@ +`-X dev` now injects a ``'default'`` entry into sys.warnoptions, ensuring +that it behaves identically to actually passing ``-Wdefault`` at the command +line. diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-12-07-17-22-30.bpo-20361.zQUmbi.rst b/Misc/NEWS.d/next/Core and Builtins/2017-12-07-17-22-30.bpo-20361.zQUmbi.rst new file mode 100644 index 0000000..df7aed7 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-12-07-17-22-30.bpo-20361.zQUmbi.rst @@ -0,0 +1,4 @@ +``-b`` and ``-bb`` now inject ``'default::BytesWarning'`` and +``error::BytesWarning`` entries into ``sys.warnoptions``, ensuring that they +take precedence over any other warning filters configured via the ``-W`` +option or the ``PYTHONWARNINGS`` environment variable. diff --git a/Modules/main.c b/Modules/main.c index 68ee616..ac8a38c 100644 --- a/Modules/main.c +++ b/Modules/main.c @@ -774,11 +774,73 @@ pymain_add_warnings_optlist(_Py_OptList *warnings) return 0; } + +static int +pymain_add_warning_dev_mode(_PyCoreConfig *core_config) +{ + if (core_config->dev_mode) { + PyObject *option = PyUnicode_FromString("default"); + if (option == NULL) { + return -1; + } + if (_PySys_AddWarnOptionWithError(option)) { + Py_DECREF(option); + return -1; + } + Py_DECREF(option); + } + return 0; +} + + +static int +pymain_add_warning_bytes_flag(int bytes_warning_flag) +{ + /* If the bytes_warning_flag isn't set, bytesobject.c and bytearrayobject.c + * don't even try to emit a warning, so we skip setting the filter in that + * case. + */ + if (bytes_warning_flag) { + const char *filter = (bytes_warning_flag > 1) ? "error::BytesWarning": + "default::BytesWarning"; + PyObject *option = PyUnicode_FromString(filter); + if (option == NULL) { + return -1; + } + if (_PySys_AddWarnOptionWithError(option)) { + Py_DECREF(option); + return -1; + } + Py_DECREF(option); + } + return 0; +} + + static int pymain_add_warnings_options(_PyMain *pymain) { PySys_ResetWarnOptions(); + /* The priority order for warnings configuration is (highest precedence + * first): + * + * - the BytesWarning filter, if needed ('-b', '-bb') + * - any '-W' command line options; then + * - the 'PYTHONWARNINGS' environment variable; then + * - the dev mode filter ('-X dev', 'PYTHONDEVMODE'); then + * - any implicit filters added by _warnings.c/warnings.py + * + * All settings except the last are passed to the warnings module via + * the `sys.warnoptions` list. Since the warnings module works on the basis + * of "the most recently added filter will be checked first", we add + * the lowest precedence entries first so that later entries override them. + */ + + if (pymain_add_warning_dev_mode(&pymain->core_config) < 0) { + pymain->err = _Py_INIT_NO_MEMORY(); + return -1; + } if (pymain_add_warnings_optlist(&pymain->env_warning_options) < 0) { pymain->err = _Py_INIT_NO_MEMORY(); return -1; @@ -787,6 +849,10 @@ pymain_add_warnings_options(_PyMain *pymain) pymain->err = _Py_INIT_NO_MEMORY(); return -1; } + if (pymain_add_warning_bytes_flag(pymain->cmdline.bytes_warning) < 0) { + pymain->err = _Py_INIT_NO_MEMORY(); + return -1; + } return 0; } diff --git a/Python/_warnings.c b/Python/_warnings.c index 2937036..7b2bdd5 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -11,9 +11,9 @@ MODULE_NAME " provides basic warning filtering support.\n" _Py_IDENTIFIER(argv); _Py_IDENTIFIER(stderr); +#ifndef Py_DEBUG _Py_IDENTIFIER(ignore); -_Py_IDENTIFIER(error); -_Py_static_string(PyId_default, "default"); +#endif static int check_matched(PyObject *obj, PyObject *arg) @@ -1156,6 +1156,7 @@ static PyMethodDef warnings_functions[] = { }; +#ifndef Py_DEBUG static PyObject * create_filter(PyObject *category, _Py_Identifier *id) { @@ -1168,70 +1169,40 @@ create_filter(PyObject *category, _Py_Identifier *id) return PyTuple_Pack(5, action_str, Py_None, category, Py_None, _PyLong_Zero); } +#endif + static PyObject * init_filters(const _PyCoreConfig *config) { - int dev_mode = config->dev_mode; - - Py_ssize_t count = 2; - if (dev_mode) { - count++; - } -#ifndef Py_DEBUG - if (!dev_mode) { - count += 3; - } -#endif - PyObject *filters = PyList_New(count); - if (filters == NULL) - return NULL; - - size_t pos = 0; /* Post-incremented in each use. */ -#ifndef Py_DEBUG - if (!dev_mode) { - PyList_SET_ITEM(filters, pos++, - create_filter(PyExc_DeprecationWarning, &PyId_ignore)); - PyList_SET_ITEM(filters, pos++, - create_filter(PyExc_PendingDeprecationWarning, &PyId_ignore)); - PyList_SET_ITEM(filters, pos++, - create_filter(PyExc_ImportWarning, &PyId_ignore)); - } -#endif - - _Py_Identifier *bytes_action; - if (Py_BytesWarningFlag > 1) - bytes_action = &PyId_error; - else if (Py_BytesWarningFlag) - bytes_action = &PyId_default; - else - bytes_action = &PyId_ignore; - PyList_SET_ITEM(filters, pos++, create_filter(PyExc_BytesWarning, - bytes_action)); - - _Py_Identifier *resource_action; - /* resource usage warnings are enabled by default in pydebug mode */ #ifdef Py_DEBUG - resource_action = &PyId_default; + /* Py_DEBUG builds show all warnings by default */ + return PyList_New(0); #else - resource_action = (dev_mode ? &PyId_default: &PyId_ignore); -#endif - PyList_SET_ITEM(filters, pos++, create_filter(PyExc_ResourceWarning, - resource_action)); - - if (dev_mode) { - PyList_SET_ITEM(filters, pos++, - create_filter(PyExc_Warning, &PyId_default)); + /* Other builds ignore a number of warning categories by default */ + PyObject *filters = PyList_New(4); + if (filters == NULL) { + return NULL; } + size_t pos = 0; /* Post-incremented in each use. */ + PyList_SET_ITEM(filters, pos++, + create_filter(PyExc_DeprecationWarning, &PyId_ignore)); + PyList_SET_ITEM(filters, pos++, + create_filter(PyExc_PendingDeprecationWarning, &PyId_ignore)); + PyList_SET_ITEM(filters, pos++, + create_filter(PyExc_ImportWarning, &PyId_ignore)); + PyList_SET_ITEM(filters, pos++, + create_filter(PyExc_ResourceWarning, &PyId_ignore)); + for (size_t x = 0; x < pos; x++) { if (PyList_GET_ITEM(filters, x) == NULL) { Py_DECREF(filters); return NULL; } } - return filters; +#endif } static struct PyModuleDef warningsmodule = { -- cgit v0.12