From fb4ae152a9930f0e00cae8b2807f534058cf341a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 30 Sep 2019 01:40:17 +0200 Subject: [PATCH] bpo-38317: Fix PyConfig.warnoptions priority (GH-16478) Fix warnings options priority: PyConfig.warnoptions has the highest priority, as stated in the PEP 587. * Document options order in PyConfig.warnoptions documentation. * Make PyWideStringList_INIT macro private: replace "Py" prefix with "_Py". * test_embed: add test_init_warnoptions(). --- Doc/c-api/init_config.rst | 8 +- Include/cpython/initconfig.h | 5 +- Include/internal/pycore_initconfig.h | 2 +- Include/internal/pycore_pylifecycle.h | 2 +- Lib/test/test_embed.py | 28 +++- .../2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst | 2 + Programs/_testembed.c | 59 +++++++++ Python/initconfig.c | 124 ++++++++++++------ Python/preconfig.c | 2 +- Python/sysmodule.c | 4 +- 10 files changed, 185 insertions(+), 51 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst diff --git a/Doc/c-api/init_config.rst b/Doc/c-api/init_config.rst index 58e417441d1..4c77d5d1e0e 100644 --- a/Doc/c-api/init_config.rst +++ b/Doc/c-api/init_config.rst @@ -704,7 +704,13 @@ PyConfig .. c:member:: PyWideStringList warnoptions - Options of the :mod:`warnings` module to build warnings filters. + :data:`sys.warnoptions`: options of the :mod:`warnings` module to build + warnings filters: lowest to highest priority. + + The :mod:`warnings` module adds :data:`sys.warnoptions` in the reverse + order: the last :c:member:`PyConfig.warnoptions` item becomes the first + item of :data:`warnings.filters` which is checked first (highest + priority). .. c:member:: int write_bytecode diff --git a/Include/cpython/initconfig.h b/Include/cpython/initconfig.h index d5effb8397f..8ce5622c5c4 100644 --- a/Include/cpython/initconfig.h +++ b/Include/cpython/initconfig.h @@ -220,7 +220,10 @@ typedef struct { wchar_t *program_name; PyWideStringList xoptions; /* Command line -X options */ - PyWideStringList warnoptions; /* Warnings options */ + + /* Warnings options: lowest to highest priority. warnings.filters + is built in the reverse order (highest to lowest priority). */ + PyWideStringList warnoptions; /* If equal to zero, disable the import of the module site and the site-dependent manipulations of sys.path that it entails. Also disable diff --git a/Include/internal/pycore_initconfig.h b/Include/internal/pycore_initconfig.h index dcdb61695ca..eb6490f5a87 100644 --- a/Include/internal/pycore_initconfig.h +++ b/Include/internal/pycore_initconfig.h @@ -45,7 +45,7 @@ extern "C" { /* --- PyWideStringList ------------------------------------------------ */ -#define PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} +#define _PyWideStringList_INIT (PyWideStringList){.length = 0, .items = NULL} #ifndef NDEBUG PyAPI_FUNC(int) _PyWideStringList_CheckConsistency(const PyWideStringList *list); diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index cdf5c09a426..b0a98f5d259 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -41,7 +41,7 @@ extern PyStatus _PySys_Create( PyThreadState *tstate, PyObject **sysmod_p); extern PyStatus _PySys_SetPreliminaryStderr(PyObject *sysdict); -extern PyStatus _PySys_ReadPreinitWarnOptions(PyConfig *config); +extern PyStatus _PySys_ReadPreinitWarnOptions(PyWideStringList *options); extern PyStatus _PySys_ReadPreinitXOptions(PyConfig *config); extern int _PySys_InitMain( _PyRuntimeState *runtime, diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index ab086e2332d..b87863a372a 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -746,9 +746,9 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): 'cmdline_xoption', ], 'warnoptions': [ - 'config_warnoption', 'cmdline_warnoption', 'default::BytesWarning', + 'config_warnoption', ], 'run_command': 'pass\n', @@ -952,9 +952,9 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): 'faulthandler', ], 'warnoptions': [ - 'ignore:::config_warnoption', 'ignore:::cmdline_warnoption', 'ignore:::sysadd_warnoption', + 'ignore:::config_warnoption', ], } self.check_all_configs("test_init_sys_add", config, api=API_PYTHON) @@ -1268,6 +1268,30 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): self.assertEqual(Py_GetProgramFullPath(), config['executable']) self.assertEqual(Py_GetPythonHome(), config['home']) + def test_init_warnoptions(self): + # lowest to highest priority + warnoptions = [ + 'ignore:::PyConfig_Insert0', # PyWideStringList_Insert(0) + 'default', # PyConfig.dev_mode=1 + 'ignore:::env1', # PYTHONWARNINGS env var + 'ignore:::env2', # PYTHONWARNINGS env var + 'ignore:::cmdline1', # -W opt command line option + 'ignore:::cmdline2', # -W opt command line option + 'default::BytesWarning', # PyConfig.bytes_warnings=1 + 'ignore:::PySys_AddWarnOption1', # PySys_AddWarnOption() + 'ignore:::PySys_AddWarnOption2', # PySys_AddWarnOption() + 'ignore:::PyConfig_BeforeRead', # PyConfig.warnoptions + 'ignore:::PyConfig_AfterRead'] # PyWideStringList_Append() + preconfig = dict(allocator=PYMEM_ALLOCATOR_DEBUG) + config = { + 'dev_mode': 1, + 'faulthandler': 1, + 'bytes_warning': 1, + 'warnoptions': warnoptions, + } + self.check_all_configs("test_init_warnoptions", config, preconfig, + api=API_PYTHON) + class AuditingTests(EmbeddingTestsMixin, unittest.TestCase): def test_open_code_hook(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst b/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst new file mode 100644 index 00000000000..b6d07474cf5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst @@ -0,0 +1,2 @@ +Fix warnings options priority: ``PyConfig.warnoptions`` has the highest +priority, as stated in the :pep:`587`. diff --git a/Programs/_testembed.c b/Programs/_testembed.c index a37594524f3..c8600d58f0b 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -1603,6 +1603,64 @@ static int test_init_setpythonhome(void) } +static int test_init_warnoptions(void) +{ + PyStatus status; + putenv("PYTHONWARNINGS=ignore:::env1,ignore:::env2"); + + PySys_AddWarnOption(L"ignore:::PySys_AddWarnOption1"); + PySys_AddWarnOption(L"ignore:::PySys_AddWarnOption2"); + + PyConfig config; + config.struct_size = sizeof(PyConfig); + + status = PyConfig_InitPythonConfig(&config); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + config.dev_mode = 1; + config.bytes_warning = 1; + + config_set_program_name(&config); + + status = PyWideStringList_Append(&config.warnoptions, + L"ignore:::PyConfig_BeforeRead"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + wchar_t* argv[] = { + L"python3", + L"-Wignore:::cmdline1", + L"-Wignore:::cmdline2"}; + config_set_argv(&config, Py_ARRAY_LENGTH(argv), argv); + config.parse_argv = 1; + + status = PyConfig_Read(&config); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + status = PyWideStringList_Append(&config.warnoptions, + L"ignore:::PyConfig_AfterRead"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + status = PyWideStringList_Insert(&config.warnoptions, + 0, L"ignore:::PyConfig_Insert0"); + if (PyStatus_Exception(status)) { + Py_ExitStatusException(status); + } + + init_from_config_clear(&config); + dump_config(); + Py_Finalize(); + return 0; +} + + static void configure_init_main(PyConfig *config) { wchar_t* argv[] = { @@ -1746,6 +1804,7 @@ static struct TestCase TestCases[] = { {"test_init_setpath", test_init_setpath}, {"test_init_setpath_config", test_init_setpath_config}, {"test_init_setpythonhome", test_init_setpythonhome}, + {"test_init_warnoptions", test_init_warnoptions}, {"test_run_main", test_run_main}, {"test_open_code_hook", test_open_code_hook}, diff --git a/Python/initconfig.c b/Python/initconfig.c index 62c868d5cbc..dec4bf2e4d5 100644 --- a/Python/initconfig.c +++ b/Python/initconfig.c @@ -273,7 +273,7 @@ _PyWideStringList_Copy(PyWideStringList *list, const PyWideStringList *list2) return 0; } - PyWideStringList copy = PyWideStringList_INIT; + PyWideStringList copy = _PyWideStringList_INIT; size_t size = list2->length * sizeof(list2->items[0]); copy.items = PyMem_RawMalloc(size); @@ -2095,63 +2095,83 @@ config_init_env_warnoptions(PyConfig *config, PyWideStringList *warnoptions) static PyStatus -config_add_warnoption(PyConfig *config, const wchar_t *option) +warnoptions_append(PyConfig *config, PyWideStringList *options, + const wchar_t *option) { + /* config_init_warnoptions() add existing config warnoptions at the end: + ensure that the new option is not already present in this list to + prevent change the options order whne config_init_warnoptions() is + called twice. */ if (_PyWideStringList_Find(&config->warnoptions, option)) { /* Already present: do nothing */ return _PyStatus_OK(); } - return PyWideStringList_Append(&config->warnoptions, option); + if (_PyWideStringList_Find(options, option)) { + /* Already present: do nothing */ + return _PyStatus_OK(); + } + return PyWideStringList_Append(options, option); +} + + +static PyStatus +warnoptions_extend(PyConfig *config, PyWideStringList *options, + const PyWideStringList *options2) +{ + const Py_ssize_t len = options2->length; + wchar_t *const *items = options2->items; + + for (Py_ssize_t i = 0; i < len; i++) { + PyStatus status = warnoptions_append(config, options, items[i]); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + } + return _PyStatus_OK(); } static PyStatus config_init_warnoptions(PyConfig *config, const PyWideStringList *cmdline_warnoptions, - const PyWideStringList *env_warnoptions) + const PyWideStringList *env_warnoptions, + const PyWideStringList *sys_warnoptions) { PyStatus status; + PyWideStringList options = _PyWideStringList_INIT; - /* The priority order for warnings configuration is (highest precedence - * first): + /* Priority of warnings options, lowest to highest: * - * - early PySys_AddWarnOption() calls - * - 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 + * - PyConfig.dev_mode: "default" filter + * - PYTHONWARNINGS environment variable + * - '-W' command line options + * - PyConfig.bytes_warning ('-b' and '-bb' command line options): + * "default::BytesWarning" or "error::BytesWarning" filter + * - early PySys_AddWarnOption() calls + * - PyConfig.warnoptions * - * 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. + * PyConfig.warnoptions is copied to sys.warnoptions. 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 (config->dev_mode) { - status = config_add_warnoption(config, L"default"); + status = warnoptions_append(config, &options, L"default"); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } } - Py_ssize_t i; - const PyWideStringList *options; - - options = env_warnoptions; - for (i = 0; i < options->length; i++) { - status = config_add_warnoption(config, options->items[i]); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + status = warnoptions_extend(config, &options, env_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } - options = cmdline_warnoptions; - for (i = 0; i < options->length; i++) { - status = config_add_warnoption(config, options->items[i]); - if (_PyStatus_EXCEPTION(status)) { - return status; - } + status = warnoptions_extend(config, &options, cmdline_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; } /* If the bytes_warning_flag isn't set, bytesobject.c and bytearrayobject.c @@ -2166,19 +2186,30 @@ config_init_warnoptions(PyConfig *config, else { filter = L"default::BytesWarning"; } - status = config_add_warnoption(config, filter); + status = warnoptions_append(config, &options, filter); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } } - /* Handle early PySys_AddWarnOption() calls */ - status = _PySys_ReadPreinitWarnOptions(config); + status = warnoptions_extend(config, &options, sys_warnoptions); if (_PyStatus_EXCEPTION(status)) { - return status; + goto error; } + /* Always add all PyConfig.warnoptions options */ + status = _PyWideStringList_Extend(&options, &config->warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto error; + } + + _PyWideStringList_Clear(&config->warnoptions); + config->warnoptions = options; return _PyStatus_OK(); + +error: + _PyWideStringList_Clear(&options); + return status; } @@ -2186,7 +2217,7 @@ static PyStatus config_update_argv(PyConfig *config, Py_ssize_t opt_index) { const PyWideStringList *cmdline_argv = &config->argv; - PyWideStringList config_argv = PyWideStringList_INIT; + PyWideStringList config_argv = _PyWideStringList_INIT; /* Copy argv to be able to modify it (to force -c/-m) */ if (cmdline_argv->length <= opt_index) { @@ -2306,8 +2337,9 @@ static PyStatus config_read_cmdline(PyConfig *config) { PyStatus status; - PyWideStringList cmdline_warnoptions = PyWideStringList_INIT; - PyWideStringList env_warnoptions = PyWideStringList_INIT; + PyWideStringList cmdline_warnoptions = _PyWideStringList_INIT; + PyWideStringList env_warnoptions = _PyWideStringList_INIT; + PyWideStringList sys_warnoptions = _PyWideStringList_INIT; if (config->parse_argv < 0) { config->parse_argv = 1; @@ -2351,9 +2383,16 @@ config_read_cmdline(PyConfig *config) } } + /* Handle early PySys_AddWarnOption() calls */ + status = _PySys_ReadPreinitWarnOptions(&sys_warnoptions); + if (_PyStatus_EXCEPTION(status)) { + goto done; + } + status = config_init_warnoptions(config, &cmdline_warnoptions, - &env_warnoptions); + &env_warnoptions, + &sys_warnoptions); if (_PyStatus_EXCEPTION(status)) { goto done; } @@ -2363,6 +2402,7 @@ config_read_cmdline(PyConfig *config) done: _PyWideStringList_Clear(&cmdline_warnoptions); _PyWideStringList_Clear(&env_warnoptions); + _PyWideStringList_Clear(&sys_warnoptions); return status; } @@ -2433,7 +2473,7 @@ PyStatus PyConfig_Read(PyConfig *config) { PyStatus status; - PyWideStringList orig_argv = PyWideStringList_INIT; + PyWideStringList orig_argv = _PyWideStringList_INIT; status = config_check_struct_size(config); if (_PyStatus_EXCEPTION(status)) { diff --git a/Python/preconfig.c b/Python/preconfig.c index d18b01dc458..01c72f5d6ba 100644 --- a/Python/preconfig.c +++ b/Python/preconfig.c @@ -75,7 +75,7 @@ _Py_SetFileSystemEncoding(const char *encoding, const char *errors) PyStatus _PyArgv_AsWstrList(const _PyArgv *args, PyWideStringList *list) { - PyWideStringList wargv = PyWideStringList_INIT; + PyWideStringList wargv = _PyWideStringList_INIT; if (args->use_bytes_argv) { size_t size = sizeof(wchar_t*) * args->argc; wargv.items = (wchar_t **)PyMem_RawMalloc(size); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 9cc7b4b27c1..92935408a58 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2037,13 +2037,13 @@ _clear_preinit_entries(_Py_PreInitEntry *optionlist) PyStatus -_PySys_ReadPreinitWarnOptions(PyConfig *config) +_PySys_ReadPreinitWarnOptions(PyWideStringList *options) { PyStatus status; _Py_PreInitEntry entry; for (entry = _preinit_warnoptions; entry != NULL; entry = entry->next) { - status = PyWideStringList_Append(&config->warnoptions, entry->value); + status = PyWideStringList_Append(options, entry->value); if (_PyStatus_EXCEPTION(status)) { return status; }