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

[BUG]: strdup cannot be found on Cygwin #4999

Open
2 of 3 tasks
QuLogic opened this issue Jan 4, 2024 · 5 comments
Open
2 of 3 tasks

[BUG]: strdup cannot be found on Cygwin #4999

QuLogic opened this issue Jan 4, 2024 · 5 comments
Labels
triage New bug, unverified

Comments

@QuLogic
Copy link
Contributor

QuLogic commented Jan 4, 2024

Required prerequisites

What version (or hash if on master) of pybind11 are you using?

2.11.1

Problem description

On our test builds with Cygwin, some newly added pybind11 code is no longer compiling due to strdup:

  ccache c++ -Isrc/_c_internal_utils.cpython-39-x86_64-cygwin.dll.p -Isrc -I../../src -I/usr/include/python3.9 -I/usr/local/lib/python3.9/site-packages/pybind11/include -fvisibility=hidden -fvisibility-inlines-hidden -flto=auto -fdiagnostics-color=always -DNDEBUG -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++17 -O3 -DPY_ARRAY_UNIQUE_SYMBOL=MPL__c_internal_utils_ARRAY_API -MD -MQ src/_c_internal_utils.cpython-39-x86_64-cygwin.dll.p/_c_internal_utils.cpp.o -MF src/_c_internal_utils.cpython-39-x86_64-cygwin.dll.p/_c_internal_utils.cpp.o.d -o src/_c_internal_utils.cpython-39-x86_64-cygwin.dll.p/_c_internal_utils.cpp.o -c ../../src/_c_internal_utils.cpp
  In file included from ../../src/_c_internal_utils.cpp:9:
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h: In member function 'char* pybind11::cpp_function::strdup_guard::operator()(const char*)':
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h:331:23: error: 'strdup' was not declared in this scope; did you mean 'strcmp'?
    331 |             auto *t = PYBIND11_COMPAT_STRDUP(s);
        |                       ^~~~~~
        |                       strcmp
  In file included from ../../src/_c_internal_utils.cpp:9:
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h: In member function 'void pybind11::cpp_function::initialize_generic(pybind11::cpp_function::unique_function_record&&, const char*, const std::type_info* const*, pybind11::size_t)':
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h:617:46: error: 'strdup' was not declared in this scope; did you mean 'strcmp'?
    617 |             = signatures.empty() ? nullptr : PYBIND11_COMPAT_STRDUP(signatures.c_str());
        |                                              ^~~~~~
        |                                              strcmp
  In file included from ../../src/_c_internal_utils.cpp:9:
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h: In member function 'pybind11::class_<type_, options>& pybind11::class_<type_, options>::def_property_static(const char*, const pybind11::cpp_function&, const pybind11::cpp_function&, const Extra& ...)':
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h:1789:33: error: there are no arguments to 'strdup' that depend on a template parameter, so a declaration of 'strdup' must be available [-fpermissive]
   1789 |                 rec_fget->doc = PYBIND11_COMPAT_STRDUP(rec_fget->doc);
        |                                 ^~~~~~
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h:1789:33: note: (if you use '-fpermissive', G++ will accept your code, but allowing the use of an undeclared name is deprecated)
  /usr/local/lib/python3.9/site-packages/pybind11/include/pybind11/pybind11.h:1797:33: error: there are no arguments to 'strdup' that depend on a template parameter, so a declaration of 'strdup' must be available [-fpermissive]
   1797 |                 rec_fset->doc = PYBIND11_COMPAT_STRDUP(rec_fset->doc);
        |                                 ^~~~~~

Unfortunately, the compiler backtrace only seems to point to the #include line, so I'm sure what exact code triggers this.

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

Not a regression

@ksunden
Copy link

ksunden commented Jan 16, 2024

#if defined(_MSC_VER)
# define PYBIND11_COMPAT_STRDUP _strdup
#else
# define PYBIND11_COMPAT_STRDUP strdup
#endif

My best guess (with no windows box handy to test it on) is that Cygwin also requires the _strdup version, but does not define _MSC_VER, and so that condition may be able to be expanded to e.g. #if defined(_MSC_VER) || defined(__CYGWIN__) (got the __CYGWIN__ based off of another part of the codebase).

The other option would be the inverse, that it is getting _strdup and needs strdup (and thus && !), but based on the error message not having an _, I think the former is more likely.

@DWesl
Copy link

DWesl commented Feb 19, 2024

Cygwin /usr/include/string.h:

#if __MISC_VISIBLE || __POSIX_VISIBLE >= 200809 || __XSI_VISIBLE >= 4
char 	*strdup (const char *) __malloc_like __result_use_check;
#endif

Cygwin /usr/include/python3.9/pyconfig.h:

/* Define to activate features from IEEE Stds 1003.1-2008 */
#define _POSIX_C_SOURCE 200809L

Maybe a #include <Python.h> is missing somewhere? Per the python C-API documentation:

Since Python may define some pre-processor definitions which affect the standard headers on some systems, you must include Python.h before any standard headers are included.

Given the first pybind11 example in the documentation is

#include <pybind11/pybind11.h>

int add(int i, int j) {
    return i + j;
}

PYBIND11_MODULE(example, m) {
    m.doc() = "pybind11 example plugin"; // optional module docstring

    m.def("add", &add, "A function that adds two numbers");
}

either the docs need to change to have #include <Python.h> before the pybind11 include, or include/pybind11/pybind11.h needs to have `#include <Python.h> on line 13 or so.

This was the cause of scipy/scipy#17925, which was fixed in scipy/scipy#18049

@DWesl
Copy link

DWesl commented Feb 29, 2024

Chasing includes: the first include in pybind11/pybind11.h is detail/class.h, which first includes ../attr.h, which in turn first includes detail/common.h

# if defined(_DEBUG) && !defined(Py_DEBUG)
// Workaround for a VS 2022 issue.
// NOTE: This workaround knowingly violates the Python.h include order requirement:
// https://docs.python.org/3/c-api/intro.html#include-files
// See https://github.com/pybind/pybind11/pull/3497 for full context.
# include <yvals.h>
# if _MSVC_STL_VERSION >= 143
# include <crtdefs.h>

are the two actual included files before Python.h on line 274. This does not seem to be a problem on the pybind side.

On the other hand, matplotlib's src/_c_internal_utils.cpp opens with #include <stdexcept>. matplotlib/matplotlib#27821 describes one way to fix this; alternately, moving the stdexcept include after the pybind11 include should also work.

@DWesl
Copy link

DWesl commented Mar 8, 2024

Closed by matplotlib/matplotlib#27821?

@QuLogic
Copy link
Contributor Author

QuLogic commented Mar 8, 2024

This could do with a documentation note at least? I don't think it's mentioned anywhere that pybind11.h should be first (or at least that it includes Python.h and has the same requirements as it does).

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

Successfully merging a pull request may close this issue.

3 participants