-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Linux: Build CMake if the checked out version is newer than the installed version. #26704
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
Conversation
|
@swift-ci test |
|
Build failed |
|
Build failed |
|
@swift-ci test |
|
Build failed |
|
Build failed |
c0c054f to
35b37ef
Compare
|
@swift-ci test |
|
Build failed |
|
Build failed |
35b37ef to
82b2055
Compare
|
@swift-ci test |
|
Build failed |
|
Build failed |
e70a80b to
79ba077
Compare
compnerd
left a comment
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
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.
else is unnecessary after the return.
|
Build failed |
|
Build failed |
|
@swift-ci test macOS |
|
Build failed |
|
@swift-ci test macOS |
|
@swift-ci test |
|
@spevans can you please verify this works on Ubuntu 14.04 and 18.04? |
…lled version. - For Linux only, if the checked out CMake repository is a newer version than the installed CMake version or CMake is not installed, build and use CMake from source. - This does not affect macOS build or set any minimum required CMake version in CMakeLists.txt
79ba077 to
ded44f5
Compare
|
@swift-ci test |
|
Build failed |
|
Build failed |
|
@shahmishal Ive just tested it locally on 14.04 and 18.04 and it worked ok. |
| (i_major, i_minor, i_patch) = self.installed_cmake_version(cmake_binary) | ||
| (s_major, s_minor, s_patch) = self.cmake_source_version(cmake_source_dir) | ||
| if (i_major > s_major or (i_major == s_major and i_minor >= s_minor) or | ||
| (i_major == s_major and i_minor == s_minor and i_patch >= s_patch)): |
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.
In Python you can simply do self.installed_cmake_version(cmake_binary) > self.cmake_source_version(cmake_source_dir) and the tuples will be compared lexicographically.
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.
Thanks for the info I will sort out a fix.
|
I am going to merge this, but please create a new PR to simplify the suggestion @drodriguez had. Thanks! |
|
This breaks the incremental build because of python lint errors. I am going to revert. |
|
|
Please reapply with those errors fixed. |
|
@aschwaighofer I'll sort out the linter errors, but shouldnt the linter have run as part of the macOS ci-tests and failed earlier or is it run as a separate CI test? |
|
@spevans It should run as part of |
For Linux only, if the checked out CMake repository is a newer version than the installed CMake version, build and use CMake from source.
This does not affect macOS build or set any minimum required CMake version in CMakeLists.txt