Fix Windows build (#56)

* cmake: Don't use Clang/GNU command line options with MSVC * changelog: Update changelog * cmake: Define NOMINMAX on Windows * cmake: Use CXX_COMPILER_ID instead of COMPILE_LANG_AND_ID * core: Add export symbol * [core] Remove a warning * [core] Don't add extern with dllexport * core: Split explicit template instantiation for constraints * [core] Fix missing definition * [core] Update constraints.hpp * [core] Add missing includes * [python] Add windows_dll_manager module to manage add_dll_directory on Windows * [python] Fix __init__.py * [cmake] Build Windows pyd in the right directory * [python] Remove pinocchio reference * [cmake] Fix path used to launch generate_stubs * [core] Use explicit instantiation macro from jrl-cmakemodules * cmake: Generate binaries in bin and lib * [core] Use new define name * [cmake] Update jrl-cmakemodules * [changelog] Fix changelog * [python] Add support to other site-package structure * [ci] Update MacOS/Linux recipe * [ci] Add Windows * [core] Fix pre-commit * [cmake] Use standard jrl-cmakemodules * [ci] Fix Windows workflow * [core] Fix some warnings * [ci] Try to publish on Windows * [core] Fix clang error * [ci] Add windows to publish workflow * [ci] Try to publish on Windows * [core] Update LogRecord iter and al_iter instead of casting * [cmake] Update jrl-cmakemodules to remove a warnings under ClangCl * [ci] Add pkg-config as Windows dependency * [cmake] Update jrl-cmakemodules to try to fix template instantiation on Windows * [core] Use new DLLAPI macro * [cmake] Update jrl-cmakemodules * [core] Add dllexport on template definition * [core] Forget some definition * [core] Fix another missing definition * [core] Manage MSVC error on template instantiation * [python] Fix dll relative directory * [ci] Test publish script on non release version * [cmake] Use standard jrl-cmakemodules * [changelog] Add changelog entry
Simple-Robotics · Jan 19, 2024 · 5a4f937 · 5a4f937
1 parent 5c5de63
commit 5a4f937
Show file tree

Hide file tree

Showing 56 changed files with 496 additions and 108 deletions.
diff --git a/.github/workflows/conda/conda-env.yml → ...rkflows/conda/environment_macos_linux.yml b/.github/workflows/conda/conda-env.yml → ...rkflows/conda/environment_macos_linux.yml
@@ -6,6 +6,7 @@ dependencies:
   - cmake
   - ninja
   - cxx-compiler
+  - pkg-config
   - ccache
   - eigenpy>=3.2
   - pinocchio

diff --git a/.github/workflows/conda/environment_windows.yml b/.github/workflows/conda/environment_windows.yml
@@ -0,0 +1,16 @@
+name: proxsuite-nlp
+channels:
+  - conda-forge
+dependencies:
+  - python
+  - cmake
+  - ninja
+  - pkg-config
+  - ccache
+  - eigenpy>=3.2
+  - pinocchio
+  - eigen
+  - fmt
+  - example-robot-data
+  - pytest
+  - casadi
diff --git a/.github/workflows/macos-linux-conda.yml b/.github/workflows/macos-linux-conda.yml
@@ -45,20 +45,21 @@ jobs:
     - uses: actions/cache@v3
       with:
         path: .ccache
-        key: ccache-conda-${{ matrix.os }}-${{ matrix.build_type }}-${{ github.sha }}
-        restore-keys: ccache-conda-${{ matrix.os }}-${{ matrix.build_type }}-
+        key: ccache-macos-linux-conda-${{ matrix.os }}-${{ matrix.build_type }}-${{ matrix.python-version }}-${{ github.sha }}
+        restore-keys: ccache-macos-linux-conda-${{ matrix.os }}-${{ matrix.build_type }}-${{ matrix.python-version }}-
 
     - uses: conda-incubator/setup-miniconda@v3
       with:
         activate-environment: proxsuite-nlp
         auto-update-conda: true
-        environment-file: .github/workflows/conda/conda-env.yml
+        environment-file: .github/workflows/conda/environment_macos_linux.yml
         python-version: ${{ matrix.python-version }}
+        auto-activate-base: false
 
     - name: Build proxsuite-nlp
-      shell: bash -l {0}
+      shell: bash -el {0}
       run: |
-        conda activate proxsuite-nlp
+        conda list
         echo $CONDA_PREFIX
 
         mkdir build
@@ -78,9 +79,8 @@ jobs:
         cmake --install .
 
     - name: Uninstall proxsuite-nlp
-      shell: bash -l {0}
+      shell: bash -el {0}
       run: |
-        conda activate proxsuite-nlp
         cd build
         cmake --build . --target uninstall
 

diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
@@ -11,14 +11,16 @@ on:
   pull_request:
   release:
     types: published
+env:
+  FROM_TAG: ${{ inputs.publish && '1' || '' }}
 
 jobs:
   upload-on-anaconda:
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: false
       matrix:
-        os: ["ubuntu-latest", "macos-latest"]
+        os: ["ubuntu-latest", "macos-latest", "windows-2019"]
         build_type: [Release]
         python-version: ["3.8", "3.10", "3.12"]
         include:
@@ -41,10 +43,25 @@ jobs:
         python-version: "3.10"
 
     - name: Install conda-build and anaconda-client
+      if: contains(matrix.os, 'macos') || contains(matrix.os, 'ubuntu')
       shell: bash -el {0}
       run: conda install conda-build anaconda-client
 
+    - name: Install conda-build and anaconda-client
+      if: contains(matrix.os, 'windows')
+      shell: cmd /C CALL {0}
+      run: conda install conda-build anaconda-client
+
+    - name: Set version
+      if: contains(matrix.os, 'macos') || contains(matrix.os, 'ubuntu')
+      shell: bash -el {0}
+      run: |
+        VER=$(grep "<version>" package.xml | sed 's/  <version>//g' | sed 's/<\/version>//g')
+        echo "VERSION=$VER" >> $GITHUB_ENV
+
     - name: Set version
+      if: contains(matrix.os, 'windows')
+      shell: bash
       run: |
         VER=$(grep "<version>" package.xml | sed 's/  <version>//g' | sed 's/<\/version>//g')
         echo "VERSION=$VER" >> $GITHUB_ENV

diff --git a/.github/workflows/windows-conda.yml b/.github/workflows/windows-conda.yml
@@ -0,0 +1,113 @@
+name: CI - Windows via Conda
+on:
+  push:
+    paths-ignore:
+      - 'doc/**'
+      - 'scripts/**'
+      - '.gitlab-ci.yml'
+      - '.gitignore'
+      - '*.md'
+      - 'CITATION.cff'
+      - 'CITATIONS.bib'
+  pull_request:
+    paths-ignore:
+      - 'doc/'
+      - 'scripts/'
+      - '.gitlab-ci.yml'
+      - '.gitignore'
+      - '*.md'
+      - 'CITATION.cff'
+      - 'CITATIONS.bib'
+
+jobs:
+  build:
+    name: ${{ matrix.os }} - ${{ matrix.compiler }}
+    runs-on: ${{ matrix.os }}
+    env:
+      CCACHE_BASEDIR: "${GITHUB_WORKSPACE}"
+      CCACHE_DIR: "${GITHUB_WORKSPACE}/.ccache"
+      CCACHE_COMPRESS: true
+      CCACHE_COMPRESSLEVEL: 6
+
+    strategy:
+      fail-fast: false
+      matrix:
+        os: ["windows-2019"]
+        compiler: ["cl", "clang-cl"]
+
+    steps:
+    - uses: actions/checkout@v4
+      with:
+        submodules: recursive
+
+    - uses: actions/cache@v3
+      with:
+        path: .ccache
+        key: ccache-windows-conda-${{ matrix.compiler }}-${{ matrix.os }}-${{ github.sha }}
+        restore-keys: ccache-windows-conda-${{ matrix.compiler }}-${{ matrix.os }}-
+
+    - uses: conda-incubator/setup-miniconda@v3
+      with:
+        activate-environment: proxsuite-nlp
+        auto-update-conda: true
+        environment-file: .github/workflows/conda/environment_windows.yml
+        python-version: "3.10"
+        auto-activate-base: false
+
+    - name: Build proxsuite-nlp
+      shell: cmd /C CALL {0}
+      env:
+        CC: ${{ matrix.compiler }}
+        CXX: ${{ matrix.compiler }}
+      run: |
+        call conda list
+
+        call "%programfiles(x86)%\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" amd64
+
+        :: Create build directory
+        mkdir build
+        pushd build
+
+        :: Configure (Ninja use CC and CXX to determine the compiler)
+        cmake ^
+          -G "Ninja" ^
+          -DCMAKE_INSTALL_PREFIX=%CONDA_PREFIX%\Library ^
+          -DCMAKE_CXX_COMPILER_LAUNCHER=ccache ^
+          -DCMAKE_BUILD_TYPE=Release ^
+          -DPYTHON_SITELIB=%CONDA_PREFIX%\Lib\site-packages ^
+          -DPYTHON_EXECUTABLE=%CONDA_PREFIX%\python.exe ^
+          -DBUILD_PYTHON_INTERFACE=ON ^
+          -DGENERATE_PYTHON_STUBS=ON ^
+          -DBUILD_BENCHMARK=ON ^
+          ..
+
+        :: Build
+        cmake --build .
+        if errorlevel 1 exit 1
+
+        :: Testing
+        ctest --output-on-failure
+        if errorlevel 1 exit 1
+
+        :: Install
+        cmake --install .
+
+        :: Test Python import
+        cd ..
+        python -c "import proxsuite_nlp"
+        if errorlevel 1 exit 1
+
+  check:
+    if: always()
+    name: check-windows-conda
+
+    needs:
+    - build
+
+    runs-on: Ubuntu-latest
+
+    steps:
+    - name: Decide whether the needed jobs succeeded or failed
+      uses: re-actors/alls-green@release/v1
+      with:
+        jobs: ${{ toJSON(needs) }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 * Don't use virtual inheritance to implement `finite_difference_wrapper`
+* Build on Windows
+* Fix Armijo lineseach exit condition
 
 ## [0.3.2] - 2024-01-12
 

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -180,14 +180,24 @@ if(ENABLE_TEMPLATE_INSTANTIATION)
 
 endif(ENABLE_TEMPLATE_INSTANTIATION)
 
+function(set_standard_output_directory target)
+  set_target_properties(
+    ${target}
+    PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin
+               LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib
+               ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)
+endfunction()
+
 function(create_library)
   # get the compile defs
   get_directory_property(CURRENT_COMPILE_DEFINITIONS COMPILE_DEFINITIONS)
 
   add_library(${PROJECT_NAME} SHARED ${LIB_HEADERS} ${LIB_SOURCES})
   add_library(proxsuite::nlp ALIAS ${PROJECT_NAME})
+  set_standard_output_directory(${PROJECT_NAME})
   set_target_properties(${PROJECT_NAME} PROPERTIES LINKER_LANGUAGE CXX INSTALL_RPATH "\$ORIGIN")
-  target_compile_definitions(${PROJECT_NAME} PUBLIC ${CURRENT_COMPILE_DEFINITIONS})
+  target_compile_definitions(${PROJECT_NAME} PUBLIC ${CURRENT_COMPILE_DEFINITIONS}
+                                                    $<$<PLATFORM_ID:Windows>:NOMINMAX>)
 
   set_target_properties(${PROJECT_NAME} PROPERTIES LINKER_LANGUAGE CXX VERSION ${PROJECT_VERSION})
   if(BUILD_LIBRARY_WITH_VERSION_SUFFIX)
@@ -206,9 +216,8 @@ function(create_library)
   endif(BUILD_WITH_PROXSUITE)
   target_include_directories(${PROJECT_NAME} PUBLIC $<INSTALL_INTERFACE:include>)
 
-  if(MSVC)
-    target_compile_options(${PROJECT_NAME} PUBLIC "/bigobj")
-  endif(MSVC)
+  set(MSVC_CMD_OPTIONS /bigobj)
+  target_compile_options(${PROJECT_NAME} PUBLIC $<$<CXX_COMPILER_ID:MSVC>:${MSVC_CMD_OPTIONS}>)
 
   foreach(headerFile ${LIB_HEADERS})
     string(REGEX REPLACE "${PROJECT_SOURCE_DIR}/" "" headerFileRelative ${headerFile})

diff --git a/bindings/python/CMakeLists.txt b/bindings/python/CMakeLists.txt
@@ -22,6 +22,7 @@ function(MAKE_BINDINGS)
   set_target_properties(
     ${PYLIB_NAME}
     PROPERTIES LIBRARY_OUTPUT_DIRECTORY "proxsuite_nlp"
+               RUNTIME_OUTPUT_DIRECTORY "proxsuite_nlp"
                PREFIX ""
                SUFFIX ${PYTHON_EXT_SUFFIX}
                VERSION ${PROJECT_VERSION})
@@ -30,7 +31,10 @@ function(MAKE_BINDINGS)
     set_target_properties(${PYLIB_NAME} PROPERTIES INSTALL_RPATH "${PYLIB_INSTALL_RPATH}")
   endif()
 
-  target_compile_options(${PYLIB_NAME} PRIVATE -Wno-conversion -Wno-comment)
+  set(GCC_CMD_OPTIONS -Wno-conversion -Wno-comment)
+  set(GCC_CMD_ID AppleClang,Clang,GNU)
+  target_compile_options(${PYLIB_NAME}
+                         PRIVATE $<$<CXX_COMPILER_ID:${GCC_CMD_ID}>:${GCC_CMD_OPTIONS}>)
   target_compile_definitions(${PYLIB_NAME} PRIVATE MODULE_NAME=${PYLIB_NAME})
   target_include_directories(
     ${PYLIB_NAME} PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/>
@@ -53,7 +57,7 @@ install(
   PATTERN "*.hpp")
 
 # COPY PYTHON FILES INTO OUTPUT FOLDER
-set(${PYLIB_NAME}_PYFILES __init__.py casadi_utils.py utils.py)
+set(${PYLIB_NAME}_PYFILES __init__.py casadi_utils.py utils.py windows_dll_manager.py)
 
 foreach(pyfile ${${PYLIB_NAME}_PYFILES})
   python_install_on_site(proxsuite_nlp ${pyfile})
@@ -62,5 +66,6 @@ endforeach()
 # Generate stubs
 if(GENERATE_PYTHON_STUBS)
   load_stubgen()
-  generate_stubs(${CMAKE_CURRENT_BINARY_DIR} proxsuite_nlp ${PYTHON_SITELIB} ${PYLIB_NAME})
+  generate_stubs(${CMAKE_CURRENT_BINARY_DIR} proxsuite_nlp ${PYTHON_SITELIB} ${PYLIB_NAME}
+                 ${PROJECT_NAME})
 endif(GENERATE_PYTHON_STUBS)
diff --git a/bindings/python/proxsuite_nlp/__init__.py b/bindings/python/proxsuite_nlp/__init__.py
@@ -1,8 +1,33 @@
 """
 Copyright (C) 2022 LAAS-CNRS, INRIA
 """
-from .pyproxsuite_nlp import *
-from .pyproxsuite_nlp import __version__
+
+# On Windows, if proxsuite-nlp.dll is not in the same directory than
+# the .pyd, it will not be loaded.
+# We first try to load proxsuite-nlp, then, if it fail and we are on Windows:
+#  1. We add all paths inside proxsuite-nlp_WINDOWS_DLL_PATH to DllDirectory
+#  2. If proxsuite-nlp_WINDOWS_DLL_PATH we add the relative path from the
+#     package directory to the bin directory to DllDirectory
+# This solution is inspired from:
+#  - https://github.com/PixarAnimationStudios/OpenUSD/pull/1511/files
+#  - https://stackoverflow.com/questions/65334494/python-c-extension-packaging-dll-along-with-pyd
+# More resources on https://github.com/diffpy/pyobjcryst/issues/33
+try:
+    from .pyproxsuite_nlp import *
+    from .pyproxsuite_nlp import __version__
+except ImportError:
+    import platform
+
+    if platform.system() == "Windows":
+        from .windows_dll_manager import get_dll_paths, build_directory_manager
+
+        with build_directory_manager() as dll_dir_manager:
+            for p in get_dll_paths():
+                dll_dir_manager.add_dll_directory(p)
+            from .pyproxsuite_nlp import *
+            from .pyproxsuite_nlp import __version__
+    else:
+        raise
 
 from . import utils