Skip to content
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

ADDED: a check for cmake version #203

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

NehilDanis
Copy link
Contributor

resolves the issue described in #201

@nachovizzo
Copy link
Collaborator

Hi @NehilDanis, thanks for putting an eye on this!

I was aware of the availability of SYSTEM since cmake 3.25, but I don't see how this made your build fail. If the flag is not available cmake should "ignore it". Or was it failing somehow?

1 similar comment
@nachovizzo
Copy link
Collaborator

Hi @NehilDanis, thanks for putting an eye on this!

I was aware of the availability of SYSTEM since cmake 3.25, but I don't see how this made your build fail. If the flag is not available cmake should "ignore it". Or was it failing somehow?

@NehilDanis
Copy link
Contributor Author

Hi @NehilDanis, thanks for putting an eye on this!

I was aware of the availability of SYSTEM since cmake 3.25, but I don't see how this made your build fail. If the flag is not available cmake should "ignore it". Or was it failing somehow?

It did not ignore it, that might have been the logical choice so that they would give backward compatibility. However, in my case it was failing. Below I attached the error I got. You can see that it is complaining about the incorrect number of arguments.

add_sub_directory_issue

Ps. I didn't get the issue for Eigen, because I had it installed already, so it just didn't need to include the eigen.cmake

@nachovizzo
Copy link
Collaborator

@NehilDanis I reproduced the setup, and it was also failing. Thanks for fixing this.

I only changed a bit of the logic to avoid having to if in the code.

PS: If you have python available, you can now do pip install cmake and get the latest versions without hassle!!!

@nachovizzo nachovizzo merged commit 06f7243 into PRBonn:main Aug 17, 2023
17 checks passed
@NehilDanis NehilDanis deleted the nehil/fix-add-sub-dir branch August 17, 2023 19:24
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.

None yet

2 participants