This is an archive of the discontinued LLVM Phabricator instance.

[cmake] avoid use of undocumented FindThread module variable
AcceptedPublic

Authored by carenas on Apr 1 2022, 12:49 PM.

Details

Reviewers
tra
beanz
Summary

df7103ddc555 ([build] Make sure to link main executable with pthreads,
2016-06-21) add a reference to an internal variable that was meant to
ensure the returned flag would be a library name.

Change the variable name to the documented[1] one and that was made
available since release of cmake 3.1.

After this change; switching its value to "true" and removing the TODO
should be possible when using newer cmake, and indeed seems to work
fine under limited testing, but that change has been punted until a
need arises (ex: RISCV with gcc failing to link because of missing
libatomic) since it could also be problematic (ex: older clang warning
when it is used while linking)

[1] https://cmake.org/cmake/help/latest/module/FindThreads.html#variable:THREADS_PREFER_PTHREAD_FLAG

Diff Detail

Event Timeline

carenas created this revision.Apr 1 2022, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2022, 12:49 PM
carenas requested review of this revision.Apr 1 2022, 12:49 PM
carenas added a subscriber: davidlt.
beanz accepted this revision.Apr 1 2022, 1:45 PM

Looks good to me. Just to leave a paper trail, this is also documented in the CMake 3.13 documentation (which is our current minimum required version):
https://cmake.org/cmake/help/v3.13/module/FindThreads.html

This revision is now accepted and ready to land.Apr 1 2022, 1:45 PM
carenas added a comment.EditedApr 1 2022, 7:04 PM

Could someone merge this change for me (I have no merge rights)?, build errors in the precheck are unrelated, but more importantly have failed in different places on each run so they are not indicative of an issue that would block this AFAIK