[PATCH] D68833: [CMake] Re-order runtimes in the order of dependencies

[Chris Bieneman via Phabricator via llvm-commits]

beanz added a comment.

In D68833#1706315 <https://reviews.llvm.org/D68833#1706315>, @ldionne wrote:

> Yes, precisely. That's also the currently preferred way of building libc++: https://libcxx.llvm.org/docs/BuildingLibcxx.html


That documentation is more than a bit lacking, as is much of the LLVM documentation.

> Not everybody ships libc++/libc++abi as part of the toolchain, and for those, building with whatever `CMAKE_CXX_COMPILER` they specify is really the right thing to do.

While this is true, in many instances libc++ even when libc++ isn't shipped with a toolchain it is locked to one. Darwin is a prime example of this. On Darwin libc++ is shipped as part of the OS, but that cycle is closely coordinated with the toolchain updates and the two are usually kept in sync.

> Don't get me wrong, I'm 100% on board that there's value in having this runtime build, however let's not pretend that it's the only correct way to build libc++.

I would argue if you don't ship libc++ as part of the toolchain you shouldn't build it as part of the toolchain either. In which case the standalone build configuration is the correct way to build it. My intention isn't to say building libcxx as a runtime is the only correct way to build libc++, my intention is to state that *if* you are building libc++ with the toolchain, building it as a runtime is the only correct way to build it.

> Say I need to generate a file based on the properties of a target. I'll need to call `get_target_property` on a target that hasn't been defined yet, and there's no way around that because `file(GENERATE)` does not expand generator expressions.

Is this something you need to do? If so I'd question higher-level decisions about how the build is structured.

> Are you thinking about this?
> 
>   set(libname "$<IF:$<TARGET_EXISTS:${lib}>,$<TARGET_PROPERTY:${lib},OUTPUT_NAME>,${lib}>")
>    list(APPEND link_libraries "${CMAKE_LINK_LIBRARY_FLAG}${libname}")
> 
> 
> That is clever, I had not thought about it.
> 
> If the above workaround works, I don't care about this patch that much. I still think we need to clarify the status of the Runtimes build and document it, unless that's already done and I've missed it (in which case please point it to me).

That is a much better approach. CMake 3.11 is when the `TARGET_EXISTS` generator expression was added, although the documentation wasn't updated until CMake 3.15, so this change would require a CMake version update, which I don't think is unreasonable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68833/new/

https://reviews.llvm.org/D68833