[PATCH] D102084: Fix lld macho standalone build by including llvm/Config/llvm-config.h instead of llvm/Config/config.h

Mariusz Ceier via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 14 10:57:23 PDT 2021


mceier added a comment.

In D102084#2759574 <https://reviews.llvm.org/D102084#2759574>, @thakis wrote:

> Sorry about the delay here.

No problem ;)



================
Comment at: llvm/cmake/modules/LLVMConfig.cmake.in:114
 
+set(HAVE_LIBXAR "@HAVE_LIBXAR")
+
----------------
thakis wrote:
> While patching this in locally to land, I noticed that this doesn't have the trailing `@` that all other lines in this file have. Are you sure this does the right thing as-is?
> 
> Also, everything in this file and almost everything in llvm-config.h starts with `LLVM_` which makes sense if the purpose of this file is to include it in external codebases. Maybe we should add an `LLVM_` prefix to this and the other symbol in this change? (But not sure about this part.)
> While patching this in locally to land, I noticed that this doesn't have the trailing @ that all other lines in this file have. Are you sure this does the right thing as-is?

Hmm; it probably is not right - if(HAVE_LIBXAR) will always be true in this case and always link with xar, so I missed one case when xar is not available on the system when testing.

> Maybe we should add an LLVM_ prefix to this and the other symbol in this change?

Ok, I can add the prefix.

Will send patches soon. Thanks for the review ;)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102084



More information about the llvm-commits mailing list