Merge pull request #210 from vgteam/fix-binder

Fix Binder for GCC 13

Author: adamnovak

CMakeLists.txt

@@ -189,10 +189,13 @@ target_include_directories(sparsepp INTERFACE "${bdsg_DIR}/deps/sparsepp/")
 add_subdirectory("${bdsg_DIR}/deps/mio")
 
 if (BUILD_PYTHON_BINDINGS)
+
     # Binder (because some generated bindings depend on headers packaged with Binder)
+    # See also: Binder commit defined in make_and_run_binder.py which actually generates bindings.
+    set(BINDER_COMMIT b6cac94c78ade6c6ffcbda629ffa520561a31788)
     ExternalProject_Add(binder
-      GIT_REPOSITORY "https://github.com/RosettaCommons/binder.git"
-      GIT_TAG "ee2ecff151d125c3add072a7765aebad6f42a70d"
+      GIT_REPOSITORY "https://github.com/adamnovak/binder.git"
+      GIT_TAG "${BINDER_COMMIT}"
       # we don't actually build or install Binder via its CMake because we just need its headers
       #CMAKE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=${INSTALL_DIR}
       CONFIGURE_COMMAND ""
@@ -202,14 +205,16 @@ if (BUILD_PYTHON_BINDINGS)
     set(binder_INCLUDE "${INSTALL_DIR}/${CMAKE_INSTALL_INCLUDEDIR}")
 
     # pybind11
+    # See also: pybind11 commit defined in make_and_run_binder.py.
+    set(PYBIND11_COMMIT 5b0a6fc2017fcc176545afe3e09c9f9885283242)
     if (CMAKE_MAJOR_VERSION EQUAL "3" AND CMAKE_MINOR_VERSION EQUAL "10")
         # We need pybind11 installed in ./pybind11 *before* CMake can finish processing this file.
         # On CMake 3.11+ we can do that with FetchContent
         # But on CMake 3.10, available on Ubuntu 18.04, we have to just call git ourselves.
         if (NOT EXISTS "${PROJECT_SOURCE_DIR}/pybind11")
             message(WARNING "Running on CMake without FetchContent_Declare; attempting to download pybind11 manually")
             execute_process(COMMAND git clone https://github.com/RosettaCommons/pybind11.git "${PROJECT_SOURCE_DIR}/pybind11")
-            execute_process(COMMAND git checkout 5b0a6fc2017fcc176545afe3e09c9f9885283242
+            execute_process(COMMAND git checkout "${PYBIND11_COMMIT}"
                             WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}/pybind11")
         endif()
 
@@ -225,7 +230,7 @@ if (BUILD_PYTHON_BINDINGS)
         FetchContent_Declare(
             pybind11
             GIT_REPOSITORY https://github.com/RosettaCommons/pybind11.git
-            GIT_TAG 5b0a6fc2017fcc176545afe3e09c9f9885283242
+            GIT_TAG "${PYBIND11_COMMIT}"
         )
         FetchContent_GetProperties(pybind11)
         if (NOT pybind11_POPULATED)

bdsg/include/bdsg/internal/binder_hook_bind.hpp

@@ -4,6 +4,14 @@
 // Components needed at binding generation time to make pybind11/Binder work for the library.
 // Forced to be used as a source for things to bind, even though nothing includes it.
 
+// We need to include the OpenMP header with compiler attribute support
+// disabled, because Binder can't understand the malloc attribute used in GCC
+// 13's omp.h. See <https://github.com/llvm/llvm-project/issues/51607>.
+// Do this before anything else can use omp.h.
+#define __attribute__(...)
+#include <omp.h>
+#undef __attribute__
+
 #include <string.h>
 
 #include <vector>

make_and_run_binder.py

@@ -23,19 +23,31 @@
 python_module_name = 'bdsg'
 
 # We have one global notion of what an include looks like
-INCLUDE_REGEX = re.compile('^\s*#include\s+(["<])(.*)([">])')
+INCLUDE_REGEX = re.compile(r'^\s*#include\s+(["<])(.*)([">])')
 # We have one master list of source code extensions
 SOURCE_EXTENSIONS = ['hpp', 'cpp', 'h', 'cc', 'c']
 
 def clone_repos():
-    ''' download the most recent copy of binder from git '''
+    ''' download the most correct binder and pybind11 from git '''
     if not glob.glob("binder"):
         print("Binder not found, cloning repo...")
-        subprocess.check_call(['git', 'clone', 'https://github.com/RosettaCommons/binder.git', 'binder'])
+        subprocess.check_call(['git', 'clone', 'https://github.com/adamnovak/binder.git', 'binder'])
         parent = os.getcwd()
         os.chdir('binder')
-        subprocess.check_call(['git', 'checkout', 'ee2ecff151d125c3add072a7765aebad6f42a70d'])
+        # See also: Binder commit defined in CMakeLists.txt for header files.
+        subprocess.check_call(['git', 'checkout', 'b6cac94c78ade6c6ffcbda629ffa520561a31788'])
         os.chdir(parent)
+    if not glob.glob("binder/build/pybind11"):
+        print("pybind11 not found, cloning repo...")
+        parent = os.getcwd()
+        os.chdir('binder')
+        os.makedirs('build', exist_ok=True)
+        subprocess.check_call(['git', 'clone', 'https://github.com/RosettaCommons/pybind11.git', 'build/pybind11'])
+        os.chdir('build/pybind11')
+        # See also: pybind11 commit defined in CMakeLists.txt
+        subprocess.check_call(['git', 'checkout', '5b0a6fc2017fcc176545afe3e09c9f9885283242'])
+        os.chdir(parent)
+
 
 def build_binder():
     '''
@@ -46,10 +58,19 @@ def build_binder():
     '''
     if not glob.glob("./build/*/*/bin/*"):
         print("Binder not compiled, using packaged build.py...")
-        # Make Binder use out pybind11 version
-        subprocess.check_call(['sed', '-i', "s/^_pybind11_version_ = .*/_pybind11_version_ = '5b0a6fc2017fcc176545afe3e09c9f9885283242'/g", 'build.py'])
         # TODO: Use CPU counting that accounts for container quotas?
-        subprocess.check_call([sys.executable, 'build.py', '--jobs', str(multiprocessing.cpu_count())])
+        subprocess.check_call(
+            [
+                sys.executable,
+                'build.py',
+                '--compiler',
+                'clang' if platform.system() == 'Darwin' else 'gcc',
+                '--jobs',
+                str(multiprocessing.cpu_count()),
+                '--pybind11',
+                os.path.join(os.getcwd(), 'build/pybind11')
+            ]
+        )
     return "binder/" + glob.glob('./build/*/*/bin/')[0] + "binder"
 
 def all_sources_and_headers(include_deps=False):
@@ -228,6 +249,16 @@ def make_bindings_code(all_includes_fn, binder_executable):
         # Also make sure to look for libomp from macports or homebrew, like CMakeLists.txt does
         command.append('-I/opt/local/include/libomp')
         command.append('-I/usr/local/include')
+    else:
+        # With current GCC, Clang can't find the multiarch-specific *and*
+        # GCC-version-specific include path where the OpenMP headers live.
+        # So help it out.
+        # TODO: We're assuming we're using GCC.
+        compiler_version = int(subprocess.check_output(["gcc", "-dumpversion"]).decode('utf-8').split('.')[0])
+        compiler_triple = subprocess.check_output(["gcc", "-dumpmachine"]).decode('utf-8').strip()
+        command.append('-I' + f"/usr/lib/gcc/{compiler_triple}/{compiler_version}/include")
+        # We rely on macro hacks in binder_hook_bind.hpp to translate the file
+        # into something Binder can understand.
 
     # Find Jansson
     jansson_flags = subprocess.check_output(['pkg-config', '--cflags', 'jansson']).decode('utf-8').strip().split(' ')