gh-76595: PyCapsule_Import() now imports submodules if needed (GH-6898)

A submodule not imported by its package is now imported if needed.
Errors raised during importing the module or looking up an attribute are now propagated instead of being replaced with generic ImportError or AttributeError.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: serhiy-storchaka

Doc/c-api/capsule.rst

@@ -108,25 +108,20 @@ Refer to :ref:`using-capsules` for more information on using these objects.
 
    Import a pointer to a C object from a capsule attribute in a module.  The
    *name* parameter should specify the full name to the attribute, as in
-   ``module.attribute``.  The *name* stored in the capsule must match this
-   string exactly.
-
-   This function splits *name* on the ``.`` character, and imports the first
-   element. It then processes further elements using attribute lookups.
+   ``package.module.attribute``.
+   Modules are imported if needed,
+   other components are looked up as attributes.
+   The *name* stored in the capsule must match this string exactly.
 
    Return the capsule's internal *pointer* on success.  On failure, set an
    exception and return ``NULL``.
 
-   .. note::
-
-      If *name* points to an attribute of some submodule or subpackage, this
-      submodule or subpackage must be previously imported using other means
-      (for example, by using :c:func:`PyImport_ImportModule`) for the
-      attribute lookups to succeed.
-
    .. versionchanged:: 3.3
       *no_block* has no effect anymore.
 
+   .. versionchanged:: next
+      Submodules are now imported if needed.
+
 
 .. c:function:: int PyCapsule_IsValid(PyObject *capsule, const char *name)
 

Lib/test/test_capi/test_capsule.py

@@ -93,49 +93,46 @@ def test_non_ascii_module_name(self):
         self.check_import(f'{name}.capsule')
 
     def test_submodule(self):
-        # Only the first component is imported; a submodule not imported
-        # by its package is not found.
-        self.assertRaises(AttributeError,
-                          _testlimitedcapi.PyCapsule_Import, 'capsule_pkg.sub.capsule')
-        # It is found after explicit import.
-        importlib.import_module('capsule_pkg.sub')
+        # A submodule not imported by its package is imported if needed.
+        self.assertNotIn('capsule_pkg.sub', sys.modules)
+        self.check_import('capsule_pkg.sub.capsule')
+        self.assertIn('capsule_pkg.sub', sys.modules)
+        # It is also found if already imported.
         self.check_import('capsule_pkg.sub.capsule')
         # A submodule imported by its package is found.
         self.check_import('capsule_autopkg.sub.capsule')
 
     def test_invalid_name(self):
         pycapsule_import = _testlimitedcapi.PyCapsule_Import
         # Non-existing module.
-        self.assertRaisesRegex(ImportError,
-            'PyCapsule_Import could not import module "capsule_nonexistent"',
+        self.assertRaisesRegex(ModuleNotFoundError,
+            "No module named 'capsule_nonexistent'",
             pycapsule_import, 'capsule_nonexistent.capsule')
         # Non-UTF-8 module name.
-        self.assertRaisesRegex(ImportError,
-            'PyCapsule_Import could not import module',
-            pycapsule_import, b'\xff\xfe.capsule')
+        self.assertRaises(UnicodeDecodeError,
+                          pycapsule_import, b'\xff\xfe.capsule')
         # Empty module name.
-        self.assertRaisesRegex(ImportError,
-            'PyCapsule_Import could not import module ""',
-            pycapsule_import, '.capsule_mod.capsule')
+        self.assertRaisesRegex(ValueError, 'Empty module name',
+                               pycapsule_import, '.capsule_mod.capsule')
         # Empty name.
-        self.assertRaisesRegex(ImportError,
-            'PyCapsule_Import could not import module ""',
-            pycapsule_import, '')
+        self.assertRaisesRegex(AttributeError, 'is not valid',
+                               pycapsule_import, '')
         # Only a dot.
-        self.assertRaisesRegex(ImportError,
-            'PyCapsule_Import could not import module ""',
-            pycapsule_import, '.')
+        self.assertRaisesRegex(ValueError, 'Empty module name',
+                               pycapsule_import, '.')
         # Non-existing attribute.
-        self.assertRaises(AttributeError,
-                          pycapsule_import, 'capsule_mod.nonexistent')
+        self.assertRaisesRegex(AttributeError, 'is not valid',
+                               pycapsule_import, 'capsule_mod.nonexistent')
         # Empty attribute name.
-        self.assertRaises(AttributeError, pycapsule_import, 'capsule_mod.')
+        self.assertRaisesRegex(AttributeError, 'is not valid',
+                               pycapsule_import, 'capsule_mod.')
         # Consecutive dots.
-        self.assertRaises(AttributeError,
-                          pycapsule_import, 'capsule_mod..capsule')
+        self.assertRaisesRegex(ModuleNotFoundError,
+            "No module named 'capsule_mod.'",
+            pycapsule_import, 'capsule_mod..capsule')
         # Attribute of an object which is not a module.
-        self.assertRaises(AttributeError,
-                          pycapsule_import, 'capsule_mod.not_capsule.capsule')
+        self.assertRaisesRegex(AttributeError, 'is not valid',
+                               pycapsule_import, 'capsule_mod.not_capsule.capsule')
         # No attribute name.
         self.assertRaisesRegex(AttributeError, 'is not valid',
                                pycapsule_import, 'capsule_mod')
@@ -162,13 +159,9 @@ def test_invalid_capsule(self):
                                pycapsule_import, 'capsule_mod.nullname')
 
     def test_error_from_import(self):
-        # The exception raised during importing the module is replaced
-        # with generic ImportError.
-        with self.assertRaises(ImportError) as cm:
-            _testlimitedcapi.PyCapsule_Import('capsule_broken.capsule')
-        self.assertEqual(str(cm.exception),
-                         'PyCapsule_Import could not import '
-                         'module "capsule_broken"')
+        # The exception raised during importing the module is propagated.
+        self.assertRaises(ZeroDivisionError,
+                          _testlimitedcapi.PyCapsule_Import, 'capsule_broken.capsule')
 
     def test_error_from_attribute_lookup(self):
         self.assertRaises(FloatingPointError,

Misc/NEWS.d/next/C_API/2018-05-16-13-47-18.bpo-32414.NODPbj.rst

@@ -0,0 +1,3 @@
+:c:func:`PyCapsule_Import` now imports submodules if needed.  Previously
+names like ``package.module.attribute`` worked only if ``package.module``
+was already imported.

Objects/capsule.c

@@ -4,6 +4,7 @@
 #include "pycore_capsule.h"       // export _PyCapsule_SetTraverse()
 #include "pycore_gc.h"            // _PyObject_GC_IS_TRACKED()
 #include "pycore_object.h"        // _PyObject_GC_TRACK()
+#include "pycore_pymem.h"         // _PyMem_Strdup()
 
 
 /* Internal structure of PyCapsule */
@@ -227,58 +228,58 @@ _PyCapsule_SetTraverse(PyObject *op, traverseproc traverse_func, inquiry clear_f
 
 
 void *
-PyCapsule_Import(const char *name, int no_block)
+PyCapsule_Import(const char *name, int Py_UNUSED(no_block))
 {
     PyObject *object = NULL;
     void *return_value = NULL;
-    char *trace;
-    size_t name_length = (strlen(name) + 1) * sizeof(char);
-    char *name_dup = (char *)PyMem_Malloc(name_length);
+    char *name_dup = _PyMem_Strdup(name);
 
     if (!name_dup) {
         return PyErr_NoMemory();
     }
 
-    memcpy(name_dup, name, name_length);
-
-    trace = name_dup;
-    while (trace) {
+    char *trace = name_dup;
+    while (1) {
         char *dot = strchr(trace, '.');
         if (dot) {
-            *dot++ = '\0';
+            *dot = '\0';
         }
-
-        if (object == NULL) {
-            object = PyImport_ImportModule(trace);
-            if (!object) {
-                PyErr_Format(PyExc_ImportError, "PyCapsule_Import could not import module \"%s\"", trace);
+        if (object) {
+            PyObject *attr;
+            if (PyObject_GetOptionalAttrString(object, trace, &attr) < 0) {
+                Py_CLEAR(object);
+                break;
             }
-        } else {
-            PyObject *object2 = PyObject_GetAttrString(object, trace);
-            Py_SETREF(object, object2);
+            Py_SETREF(object, attr);
         }
-        if (!object) {
-            goto EXIT;
+        if (!dot) {
+            // We are done
+            break;
         }
 
-        trace = dot;
+        if (!object) {
+            object = PyImport_ImportModule(name_dup);
+            if (!object) {
+                break;
+            }
+        }
+        *dot = '.';
+        trace = dot + 1;
     }
 
     /* compare attribute name to module.name by hand */
     if (PyCapsule_IsValid(object, name)) {
         PyCapsule *capsule = (PyCapsule *)object;
         return_value = capsule->pointer;
-    } else {
+    }
+    else if (!PyErr_Occurred()) {
         PyErr_Format(PyExc_AttributeError,
             "PyCapsule_Import \"%s\" is not valid",
             name);
     }
 
-EXIT:
     Py_XDECREF(object);
-    if (name_dup) {
-        PyMem_Free(name_dup);
-    }
+    PyMem_Free(name_dup);
     return return_value;
 }