Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Make Python dependency raise error/warning in shared_library instead of shared_module #14051

Open
user202729 opened this issue Dec 29, 2024 · 7 comments

Comments

@user202729
Copy link

user202729 commented Dec 29, 2024

Currently, if shared_library instead of shared_module is used by mistake e.g. #9777 then a hard-to-understand linker error results.

Suggestion: can py.dependency() automatically detect shared_library being used instead of shared_module and suggest a better one to be used?


MWE:

cat > hello_world.c << 'EOF'
#define PY_SSIZE_T_CLEAN
#include <Python.h>
#include <numpy/arrayobject.h>

static PyObject* hello_world(PyObject* self, PyObject* args) {
    // Initialize NumPy array
    npy_intp dims[1] = {5};  // Array of size 5
    PyObject* array = PyArray_SimpleNew(1, dims, NPY_DOUBLE);
    
    if (array == NULL) {
        return NULL;  // Handle error
    }

    // Fill the array with values
    double* data = (double*) PyArray_DATA((PyArrayObject*) array);
    for (int i = 0; i < 5; i++) {
        data[i] = (double)(i + 1);  // Fill with 1.0, 2.0, ..., 5.0
    }

    // Print the array
    PyObject* repr = PyObject_Repr(array);
    const char* str = PyUnicode_AsUTF8(repr);
    printf("Hello, World! Here is a NumPy array: %s\n", str);

    // Clean up
    Py_XDECREF(repr);
    Py_DECREF(array);
    
    Py_RETURN_NONE;
}

static PyMethodDef HelloWorldMethods[] = {
    {"hello", hello_world, METH_VARARGS, "Print 'Hello, World!' and a NumPy array"},
    {NULL, NULL, 0, NULL}
};

static struct PyModuleDef helloworldmodule = {
    PyModuleDef_HEAD_INIT,
    "hello_world",
    NULL,
    -1,
    HelloWorldMethods
};

PyMODINIT_FUNC PyInit_hello_world(void) {
    import_array();  // Initialize NumPy
    return PyModule_Create(&helloworldmodule);
}
EOF

cat > meson.build << 'EOF'
project('hello_world', 'c', version: '0.1.0')

py_module = import('python')
py = py_module.find_installation(pure: false)

inc_numpy = run_command(
  py,
  [ '-c', 'import numpy; print(numpy.get_include())' ],
  check: true,
).stdout().strip()

numpy_dep = declare_dependency(include_directories: inc_numpy)

hello_world = shared_library('hello_world',
    'hello_world.c',
    dependencies: [numpy_dep, py.dependency()],
    install: true,
)
EOF

mkdir build
meson setup ./build
meson compile -C ./build --verbose

Error:

[1/2] ccache cc -Ilibhello_world.so.p -I. -I.. -I/tmp/c/env/lib/python3.12/site-packages/numpy/_core/include -I/usr/include/python3.12 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O0 -g -fPIC -MD -MQ libhello_world.so.p/hello_world.c.o -MF libhello_world.so.p/hello_world.c.o.d -o libhello_world.so.p/hello_world.c.o -c ../hello_world.c
In file included from /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/ndarraytypes.h:1913,
                 from /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/ndarrayobject.h:12,
                 from /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/arrayobject.h:5,
                 from ../hello_world.c:3:
/tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/npy_1_7_deprecated_api.h:17:2: warning: #warning "Using deprecated NumPy API, disable it with " "#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
   17 | #warning "Using deprecated NumPy API, disable it with " \
      |  ^~~~~~~
[2/2] cc  -o libhello_world.so libhello_world.so.p/hello_world.c.o -Wl,--as-needed -Wl,--no-undefined -shared -fPIC -Wl,-soname,libhello_world.so
FAILED: libhello_world.so 
cc  -o libhello_world.so libhello_world.so.p/hello_world.c.o -Wl,--as-needed -Wl,--no-undefined -shared -fPIC -Wl,-soname,libhello_world.so
/usr/bin/ld: libhello_world.so.p/hello_world.c.o: in function `Py_DECREF':
/usr/include/python3.12/object.h:705:(.text+0x94): undefined reference to `_Py_Dealloc'
/usr/bin/ld: libhello_world.so.p/hello_world.c.o: in function `_import_array':
/tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/__multiarray_api.h:1487:(.text+0xc3): undefined reference to `PyImport_ImportModule'
/usr/bin/ld: /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/__multiarray_api.h:1488:(.text+0xd5): undefined reference to `PyExc_ModuleNotFoundError'
/usr/bin/ld: /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/__multiarray_api.h:1488:(.text+0xe0): undefined reference to `PyErr_ExceptionMatches'
/usr/bin/ld: /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/__multiarray_api.h:1489:(.text+0xe9): undefined reference to `PyErr_Clear'
/usr/bin/ld: /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/__multiarray_api.h:1490:(.text+0xf8): undefined reference to `PyImport_ImportModule'
/usr/bin/ld: /tmp/c/env/lib/python3.12/site-packages/numpy/_core/include/numpy/__multiarray_api.h:1497:(.text+0x123): undefined reference to `PyObject_GetAttrString'
@eli-schwartz
Copy link
Member

Suggestion: can py.dependency() automatically detect shared_library being used instead of shared_module and suggest a better one to be used?

It's not incorrect to do so -- there are (plenty of) people that use python's C-API to embed python, rather than extend it.

Embedding means your application links to py.dependency(), then utilizes Py_Initialize() to launch its own Python interpreter. You may very well be doing that from a shared library implementing your own application. Doing so means you're passing embed: true to detect the dependency, since you are required to link to libpython itself.

If you're building a python module, please don't use shared_module(). Use py.extension_module() instead, as that handles a number of other tasks as well, such as installing to the correct site-packages directory, adding the correct file extension (.cpython-312-x86_64-linux-gnu.so on Linux instead of plain .so, or .cp312-win_amd64.pyd on Windows), or setting gnu_symbol_visibility: 'hidden' by default.

@user202729
Copy link
Author

Indeed, I figure out py.extension_module() exists afterwards.

That said, how come py.dependency() doesn't add Python at link stage by default? (Then what's the correct way to do it?)

@eli-schwartz
Copy link
Member

eli-schwartz commented Dec 29, 2024

That said, how come py.dependency() doesn't add Python at link stage by default? (Then what's the correct way to do it?)

Because the original design of the module made the judgment call that people by default use py.dependency() for extending (modules) rather than embedding (Py_Initialize()), so the default value of the embed: kwarg is tuned for that use case.

@user202729
Copy link
Author

user202729 commented Dec 29, 2024

I see. Is there any way for the dependency to detect whether it's used inside shared_library or executable with wrong embed value to raise a warning automatically then? (Looks difficult? Otherwise it would already have been done?)

@eli-schwartz
Copy link
Member

In theory we could add a special-cased loop in shared_library that checks for every single project, whether any dependency happens to be a python dependency, and if so, warns when dep.embed == False.

It's not necessarily difficult to do this, the question is more whether it is worth adding a bunch of code for what may be a documentation error and anyways doesn't seem to occur very often.

@user202729
Copy link
Author

I see. What I think about the issue:

  • it took me ≈ 1 hour to diagnose the issue because I didn't know what's the difference between shared library and shared module (and come across the bad way of using either of these before py.extension_module()), so having the warning would save me that much time. Not sure how alone I am in encountering this issue.
  • on an unrelated note, the proposed implementation (special cased loop) doesn't sound very clean software-engineer-wise. In typical programming languages I suppose the way would be for dependency interface to declare a hook overridable abstract method, which will be called with details when the dependency is used what it's used for — though I'm not sure how much of this is applicable to the meson config language.

@eli-schwartz
Copy link
Member

A hook interface would just be a formal extensibility mechanism that implements a loop over dependencies. Hook abstractions are something you add in when you expect to generically treat objects in a way that you'd want to commonly override.

The issue here is that checking the project name of a dependency in the frontend and doing special consistency checks is a rather special need, whereas usually we just pass around opaque dependency objects and unpack them in the backend, when writing out a ninja file.

We don't have another frontend use case for doing anything with dependencies other than forwarding them to the backend, so we'd have to add a special case to shared_library() and executable() which operates on the dependencies just to check whether they are python dependencies.

There are likewise no existing hook points in dependency objects themselves that accept arbitrary information about the build artifact they are being used in for the sake of performing arbitrary calculations on them e.g. in this case emitting a warning if they are used with a shared library, because dependency objects always sit lower on the data model and just need to emit their linkage requirements when asked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants