Skip to content

Fix CMake variable usage #459

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

Merged
merged 3 commits into from
Jul 14, 2021
Merged

Fix CMake variable usage #459

merged 3 commits into from
Jul 14, 2021

Conversation

wyphan
Copy link
Contributor

@wyphan wyphan commented Jul 11, 2021

CMAKE_MAXIMUM_RANK is now properly defined as a CMake cache variable of type string (even though it functions as an int). Thus, at configure time, it is passed on as cmake -DCMAKE_MAXIMUM_RANK:String=7, for instance.
I only changed the semantics of defining the variable -- use set instead of option -- and updated the README file and CI workflows accordingly.

wyphan and others added 3 commits July 11, 2021 13:49
... because using `option` will make it show up incorrectly as a checkbox in cmake-gui.
... to reflect proper CMake cache variable usage.
Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Thanks for sharing. I'm learning something new about CMake every day ;).

I'm intrigued, what is the difference between cached variables and options in CMake? Is there a reason to prefer cached variables over options?

@wyphan
Copy link
Contributor Author

wyphan commented Jul 11, 2021

@awvwgk The online documentation for CMake on option seems to suggest that it is used mainly for boolean ON/OFF values, not for integer data. It has its uses though, as you can see in this StackOverflow page. Don't worry, I'm still learning CMake too!

@wyphan
Copy link
Contributor Author

wyphan commented Jul 11, 2021

Also, the thing that made me recognize the issue in the first place is when I ran cmake-gui. If declared as an option, it shows up a checkmark, instead of a text field where I can input a number.

@jvdp1
Copy link
Member

jvdp1 commented Jul 11, 2021

Thank you for this PR. I am learning something too. However, I think that cmake -DCMAKE_MAXIMUM_RANK:String=4 is not really intuitive, and more "complex" than cmake -DCMAKE_MAXIMUM_RANK=4.

Are there any other alternatives?

@wyphan
Copy link
Contributor Author

wyphan commented Jul 11, 2021 via email

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

Thank you for your explanations @wyphan . LGTM.

@certik
Copy link
Member

certik commented Jul 11, 2021

Fine with me also.

@jvdp1
Copy link
Member

jvdp1 commented Jul 14, 2021

Fine for @certik + 2 approvals, I'll merge.

@jvdp1 jvdp1 merged commit d0f13b0 into fortran-lang:master Jul 14, 2021
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.

4 participants