pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

[BUG]: strdup cannot be found on Cygwin

Open QuLogic opened this issue 1 year ago • 5 comments

Required prerequisites

  • [X] Make sure you've read the documentation. Your issue may be addressed there.
  • [X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
  • [ ] Consider asking first in the Gitter chat room or in a Discussion.

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

QuLogic avatar Jan 04 '24 01:01 QuLogic

https://github.com/pybind/pybind11/blob/39e65e10d05bfdc80413a6290bdae3391f0f93d7/include/pybind11/pybind11.h#L116-L120

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.

ksunden avatar Jan 16 '24 19:01 ksunden

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 avatar Feb 19 '24 20:02 DWesl

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 https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/detail/common.h#L226-L233 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 avatar Feb 29 '24 12:02 DWesl

Closed by matplotlib/matplotlib#27821?

DWesl avatar Mar 08 '24 17:03 DWesl

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).

QuLogic avatar Mar 08 '24 23:03 QuLogic

doc/limitations.rst or doc/basics.rst look like decent places for such a warning. Do the maintainers have a preference on which place the warning goes?

DWesl avatar Aug 15 '24 14:08 DWesl