-
Notifications
You must be signed in to change notification settings - Fork 163
Add support for latest cmake #441
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
Conversation
| -DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES} | ||
| -DCMAKE_POLICY_DEFAULT_CMP0074=NEW | ||
| -DCMAKE_PREFIX_PATH=${EXTERNAL_PREFIX_PATH} | ||
| UPDATE_COMMAND ${CMAKE_COMMAND} -E echo "No update step" |
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.
Yeah this makes sense because these are vendored dependencies with SOURCE_DIR, I'm actually not even sure what the default update step would do here. This part seems good, I'm a little confused by the other change.
02b1dc3 to
3462879
Compare
|
Actually never mind, in a clean environment I still get the issue with it not finding skymarshal. I'll let you try this out and if it works on your end let's merge it, but I'll troubleshoot that for now. |
|
Figured it out, the pythonpath it was assigning was one directory too deep |
|
Looks good, thanks for fixing! I'll get this merged. We'll also want to update the max cmake version in setup.py, I can do that |
593cbb1 to
64e231b
Compare
|
I'm going ahead and updating a few other quite outdated dependencies that need to be fixed for NixOS packaging |
|
Whoops didn't see the approval. I'll revert that and open a new PR then. |
64e231b to
923ae87
Compare
|
Bump, I did confirm that this works as intended. |
See also #441 Also upgrade benchmark dependencies (sophus, gtsam, ceres) that didn't declare support for cmake newer than 3.5, which is deprecated, and fix GKlib. Also fixes rerun_if_needed for newer cmake (#441 was a partial fix for that I think). Next PR switches CI to CMake 4, this PR demos that we can still build on CMake < 4. Future TODO to test multiple CMake versions in CI. Topic: sf-pip-cmake Reviewers: nathan,ryan-b GitOrigin-RevId: 48352712573d66d6f85564a7956143bddb2f0cc0
Fixes #440
"Works on my machine"