[llvm-go] add component lib deps to llvm-go

[axw]

Summary

llvm-go will call llvm-config --libs to get the
library names for all of the components required
to link Go code using the LLVM Go bindings. This
fails if the component libraries are not all built,
because llvm-go does not have the dependencies.

We move the dependencies off targets built with
llvm-go, and onto llvm-go itself, since llvm-go
is practically useless without them. This enables
running the Go bindings tests without building the
component libraries explicitly.

[pcc]

Does this solve the problem? In Evgeniy's configuration the libraries llvm-config was looking for wasn't being built (i.e. there were no build rules for them).

Can't you just append components on line 92 of llvm-go.go?

[axw]

In D16387#331898, @pcc wrote:

Does this solve the problem?

With this change, check-llvm-bindings-go works in a clean build.

In Evgeniy's configuration the libraries llvm-config was looking for wasn't being built (i.e. there were no build rules for them).

For both libLLVMGlobalISel and libLLVMDebugInfoCodeView? The former would be lacking a build rule due it being optional, but was causing llvm-config to complain due to the component not being marked optional. I've fixed that in r258379.

Can't you just append components on line 92 of llvm-go.go?

I don't think that helps. As far as I can see, that list of components is only used by the standalone Go bindings build (bindings/go/build.sh).

The issue is that the lit tests are using llvm-go, but neither llvm-go nor the test targets currently depend on all of the components that it will use. I could add the dependencies to the test targets, but it seems a bit simpler just to have llvm-go depend on them. It's not much use without them.

[pcc]

For both libLLVMGlobalISel and libLLVMDebugInfoCodeView? The former would be lacking a build rule due it being optional, but was causing llvm-config to complain due to the component not being marked optional. I've fixed that in r258379.

Okay, I see.

I don't think that helps. As far as I can see, that list of components is only used by the standalone Go bindings build (bindings/go/build.sh).

It used to be passed to llvm-config by llvm-go.go until your change r258283, and things worked until then. As you are fixing build breakage I would have preferred a more minimal change that reverts things to the way they used to work. But this is fine as well, LGTM.

[axw]

In D16387#331920, @pcc wrote:

For both libLLVMGlobalISel and libLLVMDebugInfoCodeView? The former would be lacking a build rule due it being optional, but was causing llvm-config to complain due to the component not being marked optional. I've fixed that in r258379.

Okay, I see.

I don't think that helps. As far as I can see, that list of components is only used by the standalone Go bindings build (bindings/go/build.sh).

It used to be passed to llvm-config by llvm-go.go until your change r258283, and things worked until then. As you are fixing build breakage I would have preferred a more minimal change that reverts things to the way they used to work. But this is fine as well, LGTM.

Oops, so it was. That was an accident. I'll just add components back in there, and forget about this change.

[axw]

Superceded by D16392

cmake/modules/AddLLVM.cmake

if (NOT ${test_suite_folder} STREQUAL "NOTFOUND")
     set_property(TARGET ${test_name} PROPERTY FOLDER "${test_suite_folder}")
   endif ()
 endfunction()
 
 function(llvm_add_go_executable binary pkgpath)
   cmake_parse_arguments(ARG "ALL" "" "DEPENDS;GOFLAGS" ${ARGN})
 
   if(LLVM_BINDINGS MATCHES "go")
-    # FIXME: This should depend only on the libraries Go needs.
-    get_property(llvmlibs GLOBAL PROPERTY LLVM_LIBS)
     set(binpath ${CMAKE_BINARY_DIR}/bin/${binary}${CMAKE_EXECUTABLE_SUFFIX})
     set(cc "${CMAKE_C_COMPILER} ${CMAKE_C_COMPILER_ARG1}")
     set(cxx "${CMAKE_CXX_COMPILER} ${CMAKE_CXX_COMPILER_ARG1}")
     set(cppflags "")
     get_property(include_dirs DIRECTORY PROPERTY INCLUDE_DIRECTORIES)
     foreach(d ${include_dirs})
       set(cppflags "${cppflags} -I${d}")
     endforeach(d)
     set(ldflags "${CMAKE_EXE_LINKER_FLAGS}")
     add_custom_command(OUTPUT ${binpath}
       COMMAND ${CMAKE_BINARY_DIR}/bin/llvm-go "go=${GO_EXECUTABLE}" "cc=${cc}" "cxx=${cxx}" "cppflags=${cppflags}" "ldflags=${ldflags}"
               ${ARG_GOFLAGS} build -o ${binpath} ${pkgpath}
-      DEPENDS llvm-config ${CMAKE_BINARY_DIR}/bin/llvm-go${CMAKE_EXECUTABLE_SUFFIX}
-              ${llvmlibs} ${ARG_DEPENDS}
+      DEPENDS llvm-config ${CMAKE_BINARY_DIR}/bin/llvm-go${CMAKE_EXECUTABLE_SUFFIX} ${ARG_DEPENDS}
       COMMENT "Building Go executable ${binary}"
       VERBATIM)
     if (ARG_ALL)
       add_custom_target(${binary} ALL DEPENDS ${binpath})
     else()
       add_custom_target(${binary} DEPENDS ${binpath})
     endif()
   endif()

tools/llvm-go/CMakeLists.txt

 if(LLVM_BINDINGS MATCHES "go")
+  # FIXME: This should depend only on the libraries Go needs.
+  get_property(llvmlibs GLOBAL PROPERTY LLVM_LIBS)
   set(binpath ${CMAKE_BINARY_DIR}/bin/llvm-go${CMAKE_EXECUTABLE_SUFFIX})
   add_custom_command(OUTPUT ${binpath}
     COMMAND ${GO_EXECUTABLE} build -o ${binpath} llvm-go.go
     DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/llvm-go.go
+            ${llvmlibs}
     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
     COMMENT "Building Go executable llvm-go")
   add_custom_target(llvm-go ALL DEPENDS ${binpath})
 endif()