Page MenuHomePhabricator

[runtimes] Remove FOO_TARGET_TRIPLE, FOO_SYSROOT and FOO_GCC_TOOLCHAIN
Needs ReviewPublic

Authored by ldionne on Oct 20 2021, 9:45 AM.

Details

Reviewers
phosek
compnerd
Group Reviewers
Restricted Project
Restricted Project
Restricted Project
Summary

Instead, folks can use the equivalent variables provided by CMake
to set those. This is an alternative to D111672.

Diff Detail

Event Timeline

ldionne created this revision.Oct 20 2021, 9:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 20 2021, 9:45 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested review of this revision.Oct 20 2021, 9:45 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 20 2021, 9:45 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript

I have the feeling that this is going to break people, but I'm investigating this patch as requested in D111672.

ldionne updated this revision to Diff 381005.Oct 20 2021, 9:47 AM

Rebase onto main

compnerd accepted this revision.Oct 26 2021, 11:11 AM
compnerd added a subscriber: compnerd.

I think that as long as the builders remain green, this is fine. Yes, this is very likely to break users. We should announce this change on llvm-dev at the very least, and you *could* possibly add a transition helper in the form of:

if(NOT CMAKE_SYSROOT AND LIBCXX_SYSROOT)
  message(SEND_WARNING "LIBCXX_SYSROOT is deprecated, please use CMAKE_SYSROOT")
  set(CMAKE_SYSROOT ${LIBCXX_SYSROOT} CACHE STRING "" FORCE)
endif()

I think that having a nice transition would be preferable, and we could drop it in the next release.

I went through my notes and the reason I introduced these flags was to support the scenario where you use LLVM_ENABLE_PROJECTS=clang;libcxx;... and you want to build libc++ (and other runtimes) for target other than host, so you cannot use CMAKE_CXX_COMPILER_TARGET or CMAKE_SYSROOT because that would also affect the tools build.

That predates the bootstrapping (née runtimes) build support and is very limited (you can only build runtimes for one target, you can only vary a few flags, you're still using the host compiler so you likely need a two-stage build) which is why we haven't used it ever since bootstrapping build became available.

I don't know if anyone still uses this support, but even if so we should steer them towards the bootstrapping build. I agree with @compnerd that providing a deprecation message for now and removing them in the next release might be preferable.