]> granicus.if.org Git - python/commitdiff
bpo-38317: Fix PyConfig.warnoptions priority (GH-16478)
authorVictor Stinner <vstinner@redhat.com>
Sun, 29 Sep 2019 23:40:17 +0000 (01:40 +0200)
committerGitHub <noreply@github.com>
Sun, 29 Sep 2019 23:40:17 +0000 (01:40 +0200)
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
Include/cpython/initconfig.h
Include/internal/pycore_initconfig.h
Include/internal/pycore_pylifecycle.h
Lib/test/test_embed.py
Misc/NEWS.d/next/Core and Builtins/2019-09-30-00-56-21.bpo-38317.pmqlIQ.rst [new file with mode: 0644]
Programs/_testembed.c
Python/initconfig.c
Python/preconfig.c
Python/sysmodule.c

index 58e417441d1393871c3382ba40b68ad45c0109f4..4c77d5d1e0e82733b6bdea15e6073947b11ea8cb 100644 (file)
@@ -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
 
index d5effb8397fc5be39d9849066c11c33695de3548..8ce5622c5c403917d7873700fefacd86fb13e6d8 100644 (file)
@@ -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
index dcdb61695cab5f2bed6f43e9e5da19d93f4e7135..eb6490f5a87f067034ab5f6c0f6ee0d09ef61ccb 100644 (file)
@@ -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);
index cdf5c09a4262ba78eae7f19a416bba8c003d0c57..b0a98f5d25971f81699f272148a6ee38775a1881 100644 (file)
@@ -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,
index ab086e2332dadceb20d76d7f6d37044c3cdff81f..b87863a372a6c41eafc63db5ae75a7c2d93853b7 100644 (file)
@@ -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 (file)
index 0000000..b6d0747
--- /dev/null
@@ -0,0 +1,2 @@
+Fix warnings options priority: ``PyConfig.warnoptions`` has the highest
+priority, as stated in the :pep:`587`.
index a37594524f3d440ed9000eaba4e2d444effc3236..c8600d58f0b4e0aacb0defe9e6eae78a8d509b35 100644 (file)
@@ -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},
index 62c868d5cbc9d6c5629f7ef57dd64ebc73337b7b..dec4bf2e4d57789b676a8b3b5525dcc1cbb5435d 100644 (file)
@@ -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)) {
index d18b01dc4586b66c653a41a06828bfe09d937a6b..01c72f5d6bad3b14d352e181fe5c82cdecce63e1 100644 (file)
@@ -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);
index 9cc7b4b27c1565e9f28328f6fa6d7f8a500d8829..92935408a58eb8b4f87863a87ea738551e437041 100644 (file)
@@ -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;
         }