Skip to content

Commit

Permalink
fix: Mingw64 corrected and add a CI job to test it (#3132)
Browse files Browse the repository at this point in the history
* mingw64 platform string is like mingw_xxx not "mingw"

See https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-python/0099-Change-the-get_platform-method-in-sysconfig-and-dist.patch

* Mingw: Do not dllexport exceptions

This is a fix for errors like:

D:/a/pybind11/pybind11/include/pybind11/detail/common.h:735:23: error: 'dllexport' implies default visibility, but 'class pybind11::builtin_exception' has already been declared with a different visibility
  735 | class PYBIND11_EXPORT builtin_exception : public std::runtime_error {
      |                       ^~~~~~~~~~~~~~~~~

* GHA: Test Mingw64 build

* fix: avoid thin binaries on mingw

* fix: drop lto on MinGW

* Mingw64: disable PYBIND11_DEPRECATED

It trigger many warnings for unknown reasons

Co-authored-by: Henry Schreiner <[email protected]>
  • Loading branch information
jeromerobert and henryiii committed Jul 30, 2021
1 parent 46c51fc commit c80e059
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 8 deletions.
31 changes: 31 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -859,3 +859,34 @@ jobs:

- name: Run all checks
run: cmake --build build -t check

mingw:
runs-on: windows-latest
defaults:
run:
shell: msys2 {0}
steps:
- uses: msys2/setup-msys2@v2
with:
install: >-
mingw-w64-x86_64-gcc
mingw-w64-x86_64-python-pip
mingw-w64-x86_64-cmake
mingw-w64-x86_64-make
mingw-w64-x86_64-python-pytest
mingw-w64-x86_64-eigen3
mingw-w64-x86_64-boost
mingw-w64-x86_64-catch
- uses: actions/checkout@v1

- name: Configure
# LTO leads to many undefined reference like
# `pybind11::detail::function_call::function_call(pybind11::detail::function_call&&)
run: cmake -G "MinGW Makefiles" -S . -B build

- name: Build
run: cmake --build build -j 2

- name: Python tests
run: cmake --build build --target pytest
20 changes: 17 additions & 3 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,27 @@
# endif
#endif

#if !defined(PYBIND11_EXPORT_EXCEPTION)
# ifdef __MINGW32__
// workaround for:
// error: 'dllexport' implies default visibility, but xxx has already been declared with a different visibility
# define PYBIND11_EXPORT_EXCEPTION
# else
# define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT
# endif
#endif

#if defined(_MSC_VER)
# define PYBIND11_NOINLINE __declspec(noinline)
#else
# define PYBIND11_NOINLINE __attribute__ ((noinline))
#endif

#if defined(PYBIND11_CPP14)
#if defined(__MINGW32__)
// For unknown reasons all PYBIND11_DEPRECATED member trigger a warning when declared
// whether it is used or not
# define PYBIND11_DEPRECATED(reason)
#elif defined(PYBIND11_CPP14)
# define PYBIND11_DEPRECATED(reason) [[deprecated(reason)]]
#else
# define PYBIND11_DEPRECATED(reason) __attribute__((deprecated(reason)))
Expand Down Expand Up @@ -740,7 +754,7 @@ PYBIND11_NAMESPACE_END(detail)
# pragma warning(disable: 4275) // warning C4275: An exported class was derived from a class that wasn't exported. Can be ignored when derived from a STL class.
#endif
/// C++ bindings of builtin Python exceptions
class PYBIND11_EXPORT builtin_exception : public std::runtime_error {
class PYBIND11_EXPORT_EXCEPTION builtin_exception : public std::runtime_error {
public:
using std::runtime_error::runtime_error;
/// Set the error using the Python C API
Expand All @@ -751,7 +765,7 @@ class PYBIND11_EXPORT builtin_exception : public std::runtime_error {
#endif

#define PYBIND11_RUNTIME_EXCEPTION(name, type) \
class PYBIND11_EXPORT name : public builtin_exception { public: \
class PYBIND11_EXPORT_EXCEPTION name : public builtin_exception { public: \
using builtin_exception::builtin_exception; \
name() : name("") { } \
void set_error() const override { PyErr_SetString(type, what()); } \
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ PYBIND11_NAMESPACE_END(detail)
/// thrown to propagate python-side errors back through C++ which can either be caught manually or
/// else falls back to the function dispatcher (which then raises the captured error back to
/// python).
class PYBIND11_EXPORT error_already_set : public std::runtime_error {
class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error {
public:
/// Constructs a new exception from the current Python error indicator, if any. The current
/// Python error indicator will be cleared.
Expand Down
3 changes: 1 addition & 2 deletions pybind11/setup_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@
import distutils.errors
import distutils.ccompiler


WIN = sys.platform.startswith("win32") and sysconfig.get_platform() != "mingw"
WIN = sys.platform.startswith("win32") and "mingw" not in sysconfig.get_platform()
PY2 = sys.version_info[0] < 3
MACOS = sys.platform.startswith("darwin")
STD_TMPL = "/std:c++{}" if WIN else "-std=c++{}"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// shared exceptions for cross_module_tests

class PYBIND11_EXPORT shared_exception : public pybind11::builtin_exception {
class PYBIND11_EXPORT_EXCEPTION shared_exception : public pybind11::builtin_exception {
public:
using builtin_exception::builtin_exception;
explicit shared_exception() : shared_exception("") {}
Expand Down
7 changes: 6 additions & 1 deletion tools/pybind11Common.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -302,13 +302,18 @@ function(_pybind11_return_if_cxx_and_linker_flags_work result cxxflags linkerfla
endfunction()

function(_pybind11_generate_lto target prefer_thin_lto)
if(MINGW)
message(STATUS "${target} disabled (problems with undefined symbols for MinGW for now)")
return()
endif()

if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
set(cxx_append "")
set(linker_append "")
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT APPLE)
# Clang Gold plugin does not support -Os; append -O3 to MinSizeRel builds to override it
set(linker_append ";$<$<CONFIG:MinSizeRel>:-O3>")
elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU" AND NOT MINGW)
set(cxx_append ";-fno-fat-lto-objects")
endif()

Expand Down

0 comments on commit c80e059

Please sign in to comment.