ldionne added inline comments.
================
Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
- DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+ DESTINATION
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
COMPONENT cxx-headers
----------------
Ericson2314 wrote:
> ldionne wrote:
> > compnerd wrote:
> > > Ericson2314 wrote:
> > > > compnerd wrote:
> > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used? Can
> > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > > > It is sometimes modified to be per target when multiple targets are
> > > > being used at once. All things `CMAKE_INSTALL_*` are globally scoped so
> > > > in general the combination builds are quite awkward.
> > > >
> > > > (Having worked on Meson, I am really missing
> > > > https://mesonbuild.com/Subprojects.html which is exactly what's needed
> > > > to do this without these bespoke idioms that never work well enough .
> > > > Alas...)
> > > I don't think that bringing up other build systems is particularly
> > > helpful.
> > >
> > > I do expect it to be modified, and I suspect that this is used
> > > specifically for builds that @ldionne supports.
> > Actually, I've never used it myself, but @phosek seems to be using it for
> > the Runtimes build to output one set of headers for each target, as
> > mentioned above.
> >
> > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving
> > the libc++ build from the runtimes build would be more in-line with the
> > CMake way of doing things (one configuration == one build), but I'm curious
> > to hear what @phosek has to say about that.
> > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving the
> > libc++ build from the runtimes build would be more in-line with the CMake
> > way of doing things (one configuration == one buid)
>
> You mean trying to mutate it during the libc++ CMake eval and then set it
> back after? That would keep the intended meaning of `CMAKE_INSTALL_PREFIX`
> being the actual prefix, but that feels awfully fragile to me. Or do you mean
> something else?
I keep forgetting that the runtimes build uses `add_subdirectory` to include
each sub-project instead of driving a proper CMake build for each of those.
Basically, what I'm saying is that whatever place we decide to build for N
multiple targets, we should perform N different CMake builds setting the
appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that discussion
should happen elsewhere, not on this review.
As far as this review is concerned, I do believe you want instead:
```
${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
```
(reversed the order of variables)
We should have @phosek confirm.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99484/new/
https://reviews.llvm.org/D99484
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
