Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Minimum CMake version is too low #54099

Closed
Kobzol opened this issue Feb 26, 2022 · 6 comments
Closed

Minimum CMake version is too low #54099

Kobzol opened this issue Feb 26, 2022 · 6 comments
Labels
BOLT cmake Build system in general and CMake in particular

Comments

@Kobzol
Copy link
Contributor

Kobzol commented Feb 26, 2022

The minimum supported CMake version is currently set to 3.13.4 (https://github.com/llvm/llvm-project/blob/main/llvm/CMakeLists.txt#L3). However, the bolt subproject uses the TYPE parameter for the install command (https://github.com/llvm/llvm-project/blob/main/bolt/tools/driver/CMakeLists.txt#L38), which does not seem to work in this version. It seems that the minimum CMake version should be bumped to 3.14.0.

CMake 3.13: https://cmake.org/cmake/help/v3.13/command/install.html#programs (TYPE is missing)
CMake 3.14: https://cmake.org/cmake/help/v3.14/command/install.html#programs (TYPE is present)

I hit this while trying to build BOLT with LLVM 3.13.4 in a rustc CI workflow.

This has also been noticed in other projects: https://github.com/coelckers/prboom-plus/pull/126/files.
@EugeneZelenko EugeneZelenko added BOLT cmake Build system in general and CMake in particular and removed new issue labels Feb 27, 2022
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2022

@llvm/issue-subscribers-bolt

@nikic
Copy link
Contributor

nikic commented Feb 27, 2022

It would be preferable to make it compatible with cmake 3.13.4 instead. It's annoying/confusing to have a single subproject with a different cmake version requirement.

@tschuett
Copy link

https://reviews.llvm.org/D120625

@aaupov
Copy link
Contributor

aaupov commented Feb 28, 2022

@tschuett, @nikic
I've put up this diff before I saw the comment about different cmake requirements. I agree with it, so will try to remove/rework the incompatible line to stay compatible with the LLVM CMake requirement.

@aaupov
Copy link
Contributor

aaupov commented Mar 4, 2022 •
edited
Loading

It would be preferable to make it compatible with cmake 3.13.4 instead. It's annoying/confusing to have a single subproject with a different cmake version requirement.

https://reviews.llvm.org/D121012

@EugeneZelenko EugeneZelenko removed the cmake Build system in general and CMake in particular label Mar 4, 2022
aaupov added a commit that referenced this issue Mar 8, 2022
@aaupov
[BOLT][CMAKE] Remove CMake 3.13.4 incompatible parameter
018ad03 
Remove `TYPE BIN` parameter that is introduced in CMake 3.14 and revert back to
the equivalent compatible form `DESTINATION ${CMAKE_INSTALL_BINDIR}`.

Addresses #54099

Reviewed By: rafauler

Differential Revision: https://reviews.llvm.org/D121012
aaupov added a commit that referenced this issue Mar 8, 2022
@aaupov
[BOLT][CMAKE] Remove CMake 3.13.4 incompatible parameter
bfaed48 
Remove `TYPE BIN` parameter that is introduced in CMake 3.14 and revert back to
the equivalent compatible form `DESTINATION ${CMAKE_INSTALL_BINDIR}`.

Addresses #54099

Reviewed By: rafauler

Differential Revision: https://reviews.llvm.org/D121012

(cherry picked from commit 018ad03)
@aaupov
Copy link
Contributor

aaupov commented Mar 8, 2022

Committed to both main and release/14.x

@aaupov aaupov closed this as completed Mar 8, 2022
@EugeneZelenko EugeneZelenko added the cmake Build system in general and CMake in particular label Mar 8, 2022
@Kobzol Kobzol mentioned this issue Mar 21, 2022
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
BOLT cmake Build system in general and CMake in particular
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@nikic @aaupov @Kobzol @tschuett @EugeneZelenko @llvmbot