Skip to content

Update version check in ccache.cmake so ccache can work on older cmake versions#4567

Merged
firewave merged 3 commits into
cppcheck-opensource:mainfrom
pfultz2:ccache-cmake
Nov 14, 2022
Merged

Update version check in ccache.cmake so ccache can work on older cmake versions#4567
firewave merged 3 commits into
cppcheck-opensource:mainfrom
pfultz2:ccache-cmake

Conversation

@pfultz2
Copy link
Copy Markdown
Contributor

@pfultz2 pfultz2 commented Oct 27, 2022 •
edited
Loading

This causes ccache to be called twice, which fails since ccache is not a valid compiler. With cmake 3.16, I get this error from cmake:

ccache: error: Could not find compiler "ccache" in PATH

@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Oct 27, 2022 •
edited
Loading

That's weird - maybe I checked for the wrong version.

It's also seems to work without it which wasn't the case before. The ccache stats in the workflows are also strange. Will take a look over the weekend.

@pfultz2
Copy link
Copy Markdown
Contributor Author

pfultz2 commented Oct 28, 2022

Can this merged in now? I can't build cppcheck locally without this change.

The CI failure is because of an upload error(unrelated to this PR).

@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Oct 28, 2022 •
edited
Loading

Can this merged in now? I can't build cppcheck locally without this change.

I didn't get around to see if this actually affects something (I forgot about it earlier today) and won't until Sunday. Simply comment out the include for the time being.

The CI failure is because of an upload error(unrelated to this PR).

Yeah some upstream services are extremely flakey...I am usually on top for re-starting those builds.

@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Oct 29, 2022

This breaks ccache with build_cmake for centos:7 in CI-unixish-docker.yml. So the check should probably be changed to check for versions before CMake 3.

It is not working properly for the other platforms. That step lacks the PCH handling. I will address that in another PR.

firewave
firewave requested changes Oct 29, 2022
View reviewed changes
Comment thread cmake/ccache.cmake
@@ -1,7 +0,0 @@
if (CMAKE_VERSION VERSION_LESS "3.17")
Copy link
Copy Markdown
Collaborator

@firewave firewave Oct 29, 2022

Choose a reason for hiding this comment

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

Please restore this file and change the version to check for to 3.0.

Copy link
Copy Markdown
Contributor Author

@pfultz2 pfultz2 Nov 13, 2022

Choose a reason for hiding this comment

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

Done.

@pfultz2
Copy link
Copy Markdown
Contributor Author

pfultz2 commented Nov 13, 2022

Can this be merged? As cppcheck cant built on older versions of cmake without it.

@firewave firewave mentioned this pull request Nov 14, 2022
Closed
@firewave firewave changed the title Remove ccache.cmake so ccache can work on older cmake versions Update version check in ccache.cmake so ccache can work on older cmake versions Nov 14, 2022
@firewave firewave merged commit 436c5c8 into cppcheck-opensource:main Nov 14, 2022
@pfultz2 pfultz2 deleted the ccache-cmake branch November 14, 2022 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@firewave firewave firewave requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pfultz2 @firewave