-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat: integrate https://github.com/nilsnolde/osrm-bindings #7485
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
base: master
Are you sure you want to change the base?
Changes from all commits
7458ee1
d4e62f9
9fe055b
77de8bd
a752d92
e16b525
54ec941
0dd6b0d
91855bc
cfa1065
3f3e452
3713f44
2493aeb
d2f5e1a
a3d2ef7
da8e068
ab38ab8
3c5770f
062c020
216bc0d
7dbde9a
427b420
a1209b0
89b2322
636123b
f932c2e
1a6c95a
a964f66
74a9b4d
63c50f1
e203d40
5f713d0
4ccad42
25d54d4
f26f848
1f3c201
c958bf5
b57cd7b
54b9347
7eed6d3
ddd0a44
ee44b63
a1ba443
17174d1
e5a1e6c
6bacda3
b08cf08
d58f146
f86e062
b361809
9ab861d
5b8412d
2bcb003
b65c287
760b656
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,10 @@ concurrency: | |
| jobs: | ||
| conan-windows-release-node: | ||
| needs: format-taginfo-docs | ||
| runs-on: windows-2025 | ||
| strategy: | ||
| matrix: | ||
| os: [windows-2025] | ||
| runs-on: ${{ matrix.os }} | ||
| continue-on-error: false | ||
| env: | ||
| BUILD_TYPE: Release | ||
|
|
@@ -43,7 +46,16 @@ jobs: | |
| echo PUBLISH=$([[ "${GITHUB_REF:-}" == "refs/tags/v${PACKAGE_JSON_VERSION}" ]] && echo "On" || echo "Off") >> $GITHUB_ENV | ||
| - run: npm install --ignore-scripts | ||
| - run: npm link --ignore-scripts | ||
| - name: Restore Conan cache | ||
| id: conan-restore | ||
| uses: actions/cache/restore@v5 | ||
| with: | ||
| path: ~/.conan2 | ||
| key: v11-conan-${{ matrix.os }}-${{ hashFiles('conanfile.py') }} | ||
| restore-keys: | | ||
| v11-conan-${{ matrix.os }}- | ||
|
Comment on lines
+49
to
+56
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. restore/save conan cache for win
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this probably needs a tweak now that we have switched to vcpkg. |
||
| - name: Build | ||
| id: build | ||
| shell: bash | ||
| run: | | ||
| mkdir build | ||
|
|
@@ -56,6 +68,12 @@ jobs: | |
|
|
||
| cmake -DCMAKE_BUILD_TYPE=Release -DENABLE_CONAN=ON -DENABLE_NODE_BINDINGS=ON .. | ||
| cmake --build . --config Release | ||
| - name: Save Conan cache | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly needs a vcpkg related change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I was hoping I'd be quicker than the vcpkg PR, but wasn't ;) |
||
| if: steps.build.outcome == 'success' && steps.conan-restore.outputs.cache-hit != 'true' | ||
| uses: actions/cache/save@v5 | ||
| with: | ||
| path: ~/.conan2 | ||
| key: v11-conan-${{ matrix.name }}-${{ hashFiles('conanfile.py') }} | ||
|
|
||
| # TODO: MSVC goes out of memory when building our tests | ||
| # - name: Run tests | ||
|
|
@@ -91,6 +109,20 @@ jobs: | |
| omitNameDuringUpdate: true | ||
| replacesArtifacts: true | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Run CIBuildWheel | ||
| if: github.event_name != 'pull_request' | ||
| uses: pypa/cibuildwheel@v3.4.0 | ||
| with: | ||
| output-dir: dist | ||
| env: | ||
| CIBW_CONFIG_SETTINGS_WINDOWS: "cmake.define.ENABLE_CONAN=ON" | ||
| - name: Upload Windows wheel | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. release-monthly.yml publishes the wheels |
||
| if: startsWith(github.ref, 'refs/tags/v') | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: wheels-windows | ||
| path: dist/*.whl | ||
| if-no-files-found: error | ||
|
|
||
| format-taginfo-docs: | ||
| runs-on: ubuntu-slim | ||
|
|
@@ -119,6 +151,11 @@ jobs: | |
| PR_TITLE: ${{ github.event.pull_request.title }} | ||
| run: | | ||
| node ./scripts/check_pr_title.js | ||
| - uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: '3.12' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this resembling the minimally supported Python version?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, that's in the pyproject.toml. cibuildwheel builds the package. the wheels are only 3.12+, but any >=3.10 should be able to build from source/sdist. |
||
| - name: Run `pre-commit` | ||
| uses: pre-commit/action@v3.0.1 | ||
| - name: Run checks | ||
| run: | | ||
| ./scripts/check_taginfo.py taginfo.json profiles/car.lua | ||
|
|
@@ -313,7 +350,7 @@ jobs: | |
| ENABLE_LTO: OFF | ||
|
|
||
| - name: conan-linux-release-node | ||
| build_node_package: true | ||
| build_bindings: true | ||
| continue-on-error: false | ||
| node: 24 | ||
| runs-on: ubuntu-24.04 | ||
|
|
@@ -324,7 +361,6 @@ jobs: | |
| NODE_PACKAGE_TESTS_ONLY: ON | ||
|
|
||
| - name: conan-linux-debug-node | ||
| build_node_package: true | ||
| continue-on-error: false | ||
| node: 24 | ||
| runs-on: ubuntu-24.04 | ||
|
|
@@ -345,21 +381,21 @@ jobs: | |
| ENABLE_LTO: OFF | ||
|
|
||
| - name: conan-macos-x64-release-node | ||
| build_node_package: true | ||
| build_bindings: true | ||
| continue-on-error: true | ||
| node: 24 | ||
| runs-on: macos-26-intel # x86_64 | ||
| runs-on: macos-15-intel # x86_64 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the python bindings (and the node ones as well maybe? didn't look) prefer the oldest OS version we can build on, so the widest range of OSs are compatible (same story for linux, where we build on glibc 2.28, current 2.43). should I create a new job for macos-26 so we cover both min & max? (also creates more CI tension for concurrent branches)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably need an additional job for this |
||
| BUILD_TYPE: Release | ||
| CCOMPILER: clang | ||
| CXXCOMPILER: clang++ | ||
| ENABLE_ASSERTIONS: ON | ||
| ENABLE_CONAN: ON | ||
|
|
||
| - name: conan-macos-arm64-release-node | ||
| build_node_package: true | ||
| build_bindings: true | ||
| continue-on-error: true | ||
| node: 24 | ||
| runs-on: macos-26 # arm64 | ||
| runs-on: macos-15 # arm64 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly the macos-26 job should stay unchanged |
||
| BUILD_TYPE: Release | ||
| CCOMPILER: clang | ||
| CXXCOMPILER: clang++ | ||
|
|
@@ -386,8 +422,12 @@ jobs: | |
| OSRM_CONNECTION_RETRIES: ${{ matrix.OSRM_CONNECTION_RETRIES }} | ||
| OSRM_CONNECTION_EXP_BACKOFF_COEF: ${{ matrix.OSRM_CONNECTION_EXP_BACKOFF_COEF }} | ||
| ENABLE_LTO: ${{ matrix.ENABLE_LTO }} | ||
| BUILD_BINDINGS: ${{ matrix.build_bindings }} | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 | ||
| fetch-tags: true | ||
| - name: Build machine architecture | ||
| run: uname -m | ||
| - name: Use Node.js | ||
|
|
@@ -401,20 +441,22 @@ jobs: | |
| key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-node- | ||
| - name: Enable compiler cache | ||
| uses: actions/cache@v5 | ||
| - name: Restore compiler cache | ||
| id: ccache-restore | ||
| uses: actions/cache/restore@v5 | ||
|
Comment on lines
-404
to
+446
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I split these up into restore/save to have more control which cache doesn't get uploaded (unsuccessful build stage) |
||
| with: | ||
| path: ~/.ccache | ||
| key: ccache-${{ matrix.name }}-${{ github.sha }} | ||
| restore-keys: | | ||
| ccache-${{ matrix.name }}- | ||
| - name: Enable Conan cache | ||
| uses: actions/cache@v5 | ||
| - name: Restore Conan cache | ||
| id: conan-restore | ||
| uses: actions/cache/restore@v5 | ||
| with: | ||
| path: ~/.conan2 | ||
| key: v10-conan-${{ matrix.name }}-${{ github.sha }} | ||
| key: v11-conan-${{ matrix.name }}-${{ hashFiles('conanfile.py') }} | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. conan shouldn't build fresh on every PR, only when relevant files change |
||
| restore-keys: | | ||
| v10-conan-${{ matrix.name }}- | ||
| v11-conan-${{ matrix.name }}- | ||
| - name: Enable test cache | ||
| uses: actions/cache@v5 | ||
| with: | ||
|
|
@@ -484,17 +526,15 @@ jobs: | |
| fi | ||
| fi | ||
|
|
||
| # TBB | ||
| TBB_VERSION=2021.12.0 | ||
| # TBB (Linux only; macOS gets it from conan or brew) | ||
| if [[ "${RUNNER_OS}" == "Linux" ]]; then | ||
| TBB_VERSION=2021.12.0 | ||
| TBB_URL="https://github.com/oneapi-src/oneTBB/releases/download/v${TBB_VERSION}/oneapi-tbb-${TBB_VERSION}-lin.tgz" | ||
| elif [[ "${RUNNER_OS}" == "macOS" ]]; then | ||
| TBB_URL="https://github.com/oneapi-src/oneTBB/releases/download/v${TBB_VERSION}/oneapi-tbb-${TBB_VERSION}-mac.tgz" | ||
|
Comment on lines
-491
to
-492
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. installed with brew, so macos-15 is actually true. these releases were build against another macos version. for linux it doesn't matter because the manylinux docker image contains the deps like tbb to build the python extension. only on macos & win they build on the host |
||
| wget --tries 5 ${TBB_URL} -O onetbb.tgz | ||
| tar zxvf onetbb.tgz | ||
| sudo cp -a oneapi-tbb-${TBB_VERSION}/lib/. /usr/local/lib/ | ||
| sudo cp -a oneapi-tbb-${TBB_VERSION}/include/. /usr/local/include/ | ||
| fi | ||
| wget --tries 5 ${TBB_URL} -O onetbb.tgz | ||
| tar zxvf onetbb.tgz | ||
| sudo cp -a oneapi-tbb-${TBB_VERSION}/lib/. /usr/local/lib/ | ||
| sudo cp -a oneapi-tbb-${TBB_VERSION}/include/. /usr/local/include/ | ||
| - name: Prepare build | ||
| run: | | ||
| mkdir ${OSRM_BUILD_DIR} | ||
|
|
@@ -505,14 +545,9 @@ jobs: | |
| fi | ||
| echo "CC=${CCOMPILER}" >> $GITHUB_ENV | ||
| echo "CXX=${CXXCOMPILER}" >> $GITHUB_ENV | ||
| if [[ "${RUNNER_OS}" == "macOS" ]]; then | ||
| # missing from GCC path, needed for conan builds of libiconv, for example. | ||
| sudo xcode-select --switch /Library/Developer/CommandLineTools | ||
| echo "LIBRARY_PATH=${LIBRARY_PATH}:/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib" >> $GITHUB_ENV | ||
| echo "CPATH=${CPATH}:/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include" >> $GITHUB_ENV | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this created all kinds of problems with reproducible tool chain setups. I tried removing it and the problems went away. |
||
| fi | ||
|
|
||
| - name: Build and install OSRM | ||
| id: build | ||
| run: | | ||
| echo "Using ${JOBS} jobs" | ||
| pushd ${OSRM_BUILD_DIR} | ||
|
|
@@ -556,6 +591,18 @@ jobs: | |
| popd | ||
| env: | ||
| Boost_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} | ||
| - name: Save compiler cache | ||
| if: steps.build.outcome == 'success' | ||
| uses: actions/cache/save@v5 | ||
| with: | ||
| path: ~/.ccache | ||
| key: ccache-${{ matrix.name }}-${{ github.sha }} | ||
| - name: Save Conan cache | ||
| if: steps.build.outcome == 'success' && steps.conan-restore.outputs.cache-hit != 'true' | ||
| uses: actions/cache/save@v5 | ||
| with: | ||
| path: ~/.conan2 | ||
| key: v11-conan-${{ matrix.name }}-${{ hashFiles('conanfile.py') }} | ||
| - name: Run all tests | ||
| if: ${{ matrix.NODE_PACKAGE_TESTS_ONLY != 'ON' }} | ||
| run: | | ||
|
|
@@ -612,10 +659,10 @@ jobs: | |
| path: test/logs/ | ||
|
|
||
| - name: Build Node package | ||
| if: ${{ matrix.build_node_package }} | ||
| if: ${{ env.BUILD_BINDINGS }} | ||
| run: ./scripts/ci/node_package.sh | ||
| - name: Publish Node package | ||
| if: ${{ matrix.build_node_package && env.PUBLISH == 'On' }} | ||
| if: ${{ env.BUILD_BINDINGS && env.PUBLISH == 'On' }} | ||
| uses: ncipollo/release-action@v1 | ||
| with: | ||
| allowUpdates: true | ||
|
|
@@ -627,13 +674,111 @@ jobs: | |
| omitNameDuringUpdate: true | ||
| replacesArtifacts: true | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| # Python bindings | ||
|
|
||
| - name: Set up Python | ||
| if: ${{ env.BUILD_BINDINGS }} | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.12" | ||
|
|
||
| - name: Build sdist | ||
| if: ${{ env.BUILD_BINDINGS && runner.os == 'Linux' }} | ||
| run: | | ||
| python -m pip install build | ||
| python -m build --sdist | ||
|
|
||
| - name: Locate sdist | ||
| if: ${{ env.BUILD_BINDINGS && runner.os == 'Linux' }} | ||
| id: sdist | ||
| shell: bash | ||
| run: echo "path=$(ls dist/*.tar.gz | head -n1)" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Run cibuildwheel | ||
| uses: pypa/cibuildwheel@v3.4.1 | ||
| if: ${{ env.BUILD_BINDINGS }} | ||
| with: | ||
| # for linux build the wheel from the sdist | ||
| package-dir: ${{ runner.os == 'Linux' && steps.sdist.outputs.path || '.' }} | ||
| output-dir: dist | ||
| env: | ||
| # Mount the workflow-cached ~/.ccache into the container so Linux | ||
| # wheel builds share the main-build ccache across runs. | ||
| CIBW_CONTAINER_ENGINE: "docker; create_args: --volume /home/runner/.ccache:/ccache" | ||
| CIBW_ENVIRONMENT_LINUX: "LD_LIBRARY_PATH=/usr/local/lib64:${LD_LIBRARY_PATH} CCACHE_DIR=/ccache" | ||
| CIBW_CONFIG_SETTINGS_MACOS: "cmake.define.CMAKE_CXX_COMPILER_LAUNCHER=ccache cmake.define.CMAKE_C_COMPILER_LAUNCHER=ccache" | ||
|
|
||
| - name: Upload wheels and sdist | ||
| if: ${{ env.BUILD_BINDINGS && startsWith(github.ref, 'refs/tags/v') }} | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: wheels-${{ matrix.name }} | ||
| path: | | ||
| dist/*.whl | ||
| dist/*.tar.gz | ||
| if-no-files-found: error | ||
|
|
||
| - name: Upload Linux wheel for stub check | ||
| if: ${{ env.BUILD_BINDINGS && runner.os == 'Linux' && github.event_name == 'pull_request' }} | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: wheel-linux-stub-check | ||
| path: dist/*.whl | ||
| if-no-files-found: error | ||
| retention-days: 1 | ||
|
|
||
| - name: Show CCache statistics | ||
| run: | | ||
| ccache -p | ||
| ccache -s | ||
|
|
||
| # Verify that committed .pyi stubs match the actual C++ bindings when Python | ||
| # C++ sources changed. Installs the Linux wheel built by build-matrix, | ||
| # regenerates stubs via nanobind, ruff-formats them, and diffs against the repo. | ||
| check-python-stubs: | ||
| name: Check Python stubs are up to date | ||
| needs: build-matrix | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - uses: dorny/paths-filter@v3 | ||
| id: changes | ||
| with: | ||
| filters: | | ||
| python_cpp: | ||
| - 'src/python/src/**' | ||
| - 'src/python/include/**' | ||
| - 'src/python/CMakeLists.txt' | ||
| - uses: actions/setup-python@v6 | ||
|
Comment on lines
+750
to
+754
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only run the actual check if this file filter is true for changed files |
||
| if: steps.changes.outputs.python_cpp == 'true' | ||
| with: | ||
| python-version: '3.12' | ||
| - uses: actions/download-artifact@v4 | ||
| if: steps.changes.outputs.python_cpp == 'true' | ||
| with: | ||
| name: wheel-linux-stub-check | ||
| path: wheelhouse | ||
| - name: Install wheel and regenerate stubs | ||
| if: steps.changes.outputs.python_cpp == 'true' | ||
| run: | | ||
| pip install nanobind ruff | ||
| pip install wheelhouse/*.whl | ||
| python -m nanobind.stubgen -m osrm.osrm_ext -o src/python/osrm/osrm_ext.pyi | ||
| ruff format src/python/osrm/osrm_ext.pyi | ||
| - name: Check for differences | ||
| if: steps.changes.outputs.python_cpp == 'true' | ||
| run: | | ||
| git diff --exit-code src/python/osrm/osrm_ext.pyi \ | ||
| || (echo "::error::Stubs are out of date. Rebuild locally and commit the updated .pyi file." && exit 1) | ||
|
|
||
| ci-complete: | ||
| runs-on: ubuntu-latest | ||
| needs: [build-matrix, conan-windows-release-node, docker-image-matrix] | ||
| needs: [build-matrix, conan-windows-release-node, docker-image-matrix, check-python-stubs] | ||
| if: ${{ !cancelled() }} | ||
| steps: | ||
| - name: Fail if any dependency failed | ||
| if: ${{ contains(needs.*.result, 'failure') }} | ||
| run: exit 1 | ||
|
Comment on lines
+779
to
+783
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure why claude added this, but doesn't seem to hurt |
||
| - run: echo "CI complete" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just so we can use
${{ matrix.os }}in the cache key, for when we update the runner OS