[libcxx-commits] [PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM, though you may wanna wait for @jdenny too.

In D59168#1423587 <https://reviews.llvm.org/D59168#1423587>, @phosek wrote:

> The layout currently looks as follows:
>
>   compiler-rt:
>     headers: $prefix/lib/clang/$version/include
>     libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>  
>   libc++, libc++abi, libunwind:
>     headers: $prefix/include/c++/v1
>     libraries: $prefix/lib/clang/$triple/$name.$ext
>
>
> So if we take `x86_64-linux-gnu` as an example target, it'd be:
>
>   include/c++/v1
>   lib/clang/x86_64-linux-gnu/{libc++.so,libc++abi.so,libunwind.so}
>   lib/clang/9.0.0/x86_64-linux-gnu/lib/libclang_rt.builtins.a
>
>
> I'm not super enthusiastic about the duplicated triple, but the only way to eliminate it would be to move the Clang resource directory inside of `lib/clang/x86_64-linux-gnu`, i.e. we'd have `lib/clang/x86_64-linux-gnu/9.0.0/{include,lib}`.


I don't think the duplicated triple is too huge of a deal. I think the layout where the resource directory is moved inside the triple directory is a bit nicer, but I also don't know how much work that change would be and if it's worth it.



================
Comment at: clang/test/Driver/linux-per-target-runtime-dir.c:15
 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]"
+// CHECK-PER-TARGET-RUNTIME: "-L{{.*}}{{/|\\\\}}..{{/|\\\\}}lib{{/|\\\\}}clang{{/|\\\\}}x86_64-linux-gnu"
 // CHECK-PER-TARGET-RUNTIME: "-L[[RESDIR]]{{/|\\\\}}x86_64-linux-gnu{{/|\\\\}}lib"
----------------
Idk if it's worth making this a bit more specific – `clang -###` should print out its InstallerDir, so you could capture that in a FileCheck regex and use it to check the exact path.


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

https://reviews.llvm.org/D59168