[libcxx-commits] [libcxx] 69c2087 - [libc++] Fix compiler-rt build by copying libc++ headers to <build>/include

[Louis Dionne via libcxx-commits]

> On Oct 21, 2020, at 19:22, Nico Weber <thakis at chromium.org> wrote:
> 
> The contract is enshrined in the clang driver. Requiring an install as part of the build/run dev loop seems suboptimal maybe?

The Clang driver says that there can (not even must) be libc++ headers alongside the Clang __installation__ path. The LLVM build directory is all but an installation path. It's a temporary, dirty directory containing temporary build results. This seems to confirm my first impression that this has never been a "contract", but instead just something people relied upon because it worked. That's fine, but if projects need access to an installed version of the libc++ headers during the build, there's a proper way of doing that.

Regarding the optimality of requiring an install during the build/run dev loop: we already effectively require one, since we require copying the libc++ headers to <build>/include/c++/v1. So doing it properly by actually calling the installation scripts shouldn't be much of a change, except we'll know what we get is the same headers as when we release libc++ proper.

> 
> Anyhow, is there a reason the headers couldn't keep being copied to builddir/include/... (where they used to be)? From my read of your change, I think your patch is independent of the actual copy destination, so why not just keep the old location? I.e. set LIBCXX_GENERATED_INCLUDE_DIR to LIBCXX_HEADER_DIR/include/c++/v1 ?

That's exactly what we're doing now. I had moved it to <build>/projects/libcxx/include/c++/v1 because it made more sense to avoid cluttering the top-level build directory with libc++ specific stuff, however we now learned that other projects want to be cluttered :-).

Louis

> 
> On Wed, Oct 21, 2020 at 7:15 PM Louis Dionne <ldionne at apple.com <mailto:ldionne at apple.com>> wrote:
> 
> 
> > On Oct 21, 2020, at 19:05, Nico Weber <thakis at chromium.org <mailto:thakis at chromium.org>> wrote:
> > 
> > Not sure I'd call this an "implementation detail". The build dir used to have clang in 'bin/clang', lib stuff in 'lib/...', and this in 'include/...'. That's also the dir layout when installing, and it means that clang's built-in search logic would find libc++ headers before. That seems like a bad thing to break?
> 
> 
> I wasn't aware this was a convention at a point -- I must have missed it, or it isn't something that's widely known.
> 
> In all cases, I would argue that the proper way to create something that looks like an installation root is to actually perform the installation to a directory using the scripts that CMake gives you. You can even do it from within CMake itself. That way, you are guaranteed that the proper installation steps are performed, for example:
> - Setting up the config_site header properly
> - Replacing libc++.so by a linker script on platforms that do that
> 
> So it looks like I broke something that is (or once was) a "contract", however I definitely think that contract should be more explicit. I'll see what I can do to improve the situation there.
> 
> Louis
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20201021/00b7df05/attachment.html>