Fix lld macho standalone build by including llvm/Config/llvm-config.h instead of llvm/Config/config.h

[mceier]

Summary

lld/MachO/Driver.cpp and lld/MachO/SyntheticSections.cpp include llvm/Config/config.h which doesn't exist when building standalone lld.

This patch replaces llvm/Config/config.h include with llvm/Config/llvm-config.h just like it is in lld/ELF/Driver.cpp and HAVE_LIBXAR with LLVM_HAVE_LIXAR and moves LLVM_HAVE_LIBXAR from config.h to llvm-config.h

Also it adds LLVM_HAVE_LIBXAR to LLVMConfig.cmake and links liblldMachO2.so with XAR_LIB if LLVM_HAVE_LIBXAR is set.

[int3]

Thanks!

[thakis]

Wait, this isn't right, is it? We need this for #ifndef HAVE_LIBXAR and HAVE_LIBXAR is in config.h but not in llvm-config.h. This will silently break HAVE_LIBXAR checks.

[thakis]

(ie you'll have to move HAVE_LIBXAR from config.h to llvm-config.h too)

[mceier]

In D102084#2745000, @thakis wrote:

Wait, this isn't right, is it? We need this for #ifndef HAVE_LIBXAR and HAVE_LIBXAR is in config.h but not in llvm-config.h. This will silently break HAVE_LIBXAR checks.

I think you're right, though I have no idea how to move HAVE_LIBXAR correctly - I doubt that just moving it from include/llvm/Config/config.h.cmake to include/llvm/Config/llvm-config.h.cmake is a good idea.

llvm-config.h seems to create defines with LLVM_ prefix only.

[mceier]

Moved HAVE_LIBXAR from config.h to llvm-config.h

[thakis]

Lgtm assuming you checked that the XAR ifdefs in the two files you're changing still work (with a temporary local #error in the right place or similar)

llvm/include/llvm/Config/llvm-config.h.cmake

97

Maybe fix the missing word in the comment while here :)

[mceier]

Added missing word in a comment for HAVE_LIBXAR define

[mceier]

Verified that standalone lld builds with HAVE_LIBXAR set and unset.
Added HAVE_LIBXAR to LLVMConfig.cmake and linking liblldMachO2.so with XAR_LIB if HAVE_LIBXAR is set.

[thakis]

Looks great. Do you need me to land this for you, or can you commit yourself?

[mceier]

In D102084#2747801, @thakis wrote:

Looks great. Do you need me to land this for you, or can you commit yourself?

I don't have commit rights, so I need you to land this for me, if everything looks good to you ;)

[thakis]

Sorry about the delay here.

llvm/cmake/modules/LLVMConfig.cmake.in

114 ↗

(On Diff #343819)

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.)

[mceier]

In D102084#2759574, @thakis wrote:

Sorry about the delay here.

No problem ;)

llvm/cmake/modules/LLVMConfig.cmake.in

114 ↗

(On Diff #343819)

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 ;)

[mceier]

Added LLVM_ prefix to all occurrences of HAVE_LIBXAR; fixed one missing char.

lld/MachO/Driver.cpp

 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/BinaryFormat/Magic.h"
-#include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Option/ArgList.h"

lld/MachO/SyntheticSections.cpp

 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/LEB128.h"

llvm/include/llvm/Config/config.h.cmake

 /* Define if the setupterm() function is supported this platform. */
 #cmakedefine LLVM_ENABLE_TERMINFO ${LLVM_ENABLE_TERMINFO}
 
-/* Define if the xar_open() function is supported this platform. */
-#cmakedefine HAVE_LIBXAR ${HAVE_LIBXAR}
-
 /* Define to 1 if you have the <termios.h> header file. */
 #cmakedefine HAVE_TERMIOS_H ${HAVE_TERMIOS_H}
 

llvm/include/llvm/Config/llvm-config.h.cmake

 /* Define to 1 to enable the experimental new pass manager by default */
 #cmakedefine01 LLVM_ENABLE_NEW_PASS_MANAGER
 
+/* Define if the xar_open() function is supported on this platform. */
+#cmakedefine HAVE_LIBXAR ${HAVE_LIBXAR}
+
 #endif