-
Notifications
You must be signed in to change notification settings - Fork 16
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
Explicitly disable the configure command for eigen (ext) #115
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
=======================================
Coverage 65.50% 65.50%
=======================================
Files 86 86
Lines 10893 10893
=======================================
Hits 7136 7136
Misses 3757 3757
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM, thanks!
I am undecided whether the version number of libCZI should be bumped or not. So far I went with the rule "if in doubt, increment the patch". I'm ok with both, but if you decide to change the version, don't forget to add a line to "version history" as well.
I opted for incrementing the version number. It is for sure an arguably small and potentially harmless change, but it mingles with the build environment nonetheless. Therefore, I'd say it's good to have it documented in the version history. |
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.
Please fix version number in version-history.
Co-authored-by: ptahmose <jbohl@h-quer.de>
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.
LGTM, thanks!
Description
Building with clang (tested with version 18.1.0rc) yields an error in the configuration step of the external eigen package.
Explicitly disabling this (it is implicitly skipped when building with msvc) solves this problem and the project builds fine with clang on windows.
Fixes # (issue)
Type of change
How Has This Been Tested?
Locally tested using clang from the official LLVM releases. Configuring the project with
export CC=$(which clang) && export CXX=$(which clang++) && cmake -B build -S . -G Ninja
(ensuring thewhich clang
andwhich clang++
calls point to the correct locations of the compilers).I've tested both, Release and Debug configurations + used this fork in an external project that includes libczi via
FetchContent_Declare
.Checklist: