Skip to content

Conversation

@Kallistos
Copy link
Contributor

No description provided.

@Kallistos Kallistos requested a review from vulder April 10, 2025 10:18
elseif(UNIX AND NOT APPLE)
# Clang on Linux can use libstdc++ (usually default) No need to explicitly
# add this unless required
add_compile_options(-stdlib=libstdc++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this optional or maybe don't set it at all, as it is currently not necessary to be specified for UNIX systems where the system default gets chosen.

PS: I compile with libc++ on some UNIX setup 🥲

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I do not need it, was not sure if that should be in it.
Should I remove it or what would be the way to make it optional to be helpful for Linux?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two options, either remove the line for unix and only keep it for apple or introduce a cmake options that let's one "choose" one of the two libs. something like VARA_FEATURE_USE_STD_LIB where one then can insert the lib wanted.

set(Python3_ROOT_DIR "/opt/homebrew") # cmake-lint: disable=C0103
endif()

find_package(Python3 REQUIRED COMPONENTS Interpreter Development)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this always necessary here? If I remember correctly, we only searched for python where we needed it for the bindings but I'm not sure if we did this our selfs or if we just used what pybind found.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

endif()

if(APPLE)
set(Python3_ROOT_DIR "/opt/homebrew") # cmake-lint: disable=C0103
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh boy, someone should fix the cmake find package for python 😆 or why is it necessary to specify the actual root beforehand where it's not on other systems?

Kallistos and others added 2 commits April 15, 2025 10:14
remove unnecessary commands as comment in CMakelists

Co-authored-by: Florian Sattler <vuld3r@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants