add go_xxx to simplify cmake #2182
Conversation
|
|
||
| set(Go_BIN_PATH | ||
| $ENV{GOPATH} | ||
| $ENV{GOROOT} |
There was a problem hiding this comment.
go bin path 一般在$ENV{GOROOT}/bin
There was a problem hiding this comment.
LGTM, only one comment https://github.com/PaddlePaddle/Paddle/pull/2182/files#r117021742 , can be easily fixed if the comment is correct.
| set(LIB_NAME "lib${TARGET_NAME}.so") | ||
| endif() | ||
| else() | ||
| set(BUILD_MODE "-buildmode=c-archive") |
There was a problem hiding this comment.
I think we might have to support more types of work in addition to c-shared and c-archive in go_library? These two types are Go libraries that can be called from C, and there are special requirements with the source code -- e.g., all exported symbols must be in package main, and there must be a main function defined. I think most Go libraries are supposed to be called only from Go code, and should be in some other type -- archive maybe? @helinwang
There was a problem hiding this comment.
@wangkuiyi Good question! You mean how to specify Go dependencies when using go_binary? I think it would be painful to specify Go dependencies manually in cmake (unlike C++ needs to use cmake or other tools for dependency management, Go already have it's dependency management tool). Maybe in cmake we can just let the go tool do it for us:
export GOPATH=./build/go
go get .path_to_go_binary_project/...in this way, go_library is only for the library used by other languages.
reference: http://stackoverflow.com/a/30296041/852385
There was a problem hiding this comment.
What I was talking about is that go_library should be able to generate the following types of outputs:
-
A static library which is callable from other Go code. To build it, we need something like the following:
go build \ -o ~/work/paddle/build/libpaddle_go_pserver_libpserver.a \ ~/work/paddle/paddle/go/pserver/pserver.go
Please be aware of the correspondence between source file path and output archive file path.
-
A static library which is callable from C/C++. We need something like
go build -buildmode=c-archive \ -o ~/work/paddle/build/libpaddle_go_pserver_cclient_cclient.a ~/work/paddle/paddle/go/pserver/cclient/cclient.go
-
A dynamic library which is callable from C/C++.
go build -buildmode=c-shared \ -o ~/work/paddle/build/libpaddle_go_pserver_cclient_cclient.so ~/work/paddle/paddle/go/pserver/cclient/cclient.go
| #include "gtest/gtest.h" | ||
| #include "libadder.h" | ||
|
|
||
| TEST(Cgo, Invoke) { EXPECT_EQ(GoAdder(30, 12), 42); } |
There was a problem hiding this comment.
For the linking of this file's executable to work, we will need set (CMAKE_EXE_LINKER_FLAGS "-pthread"), because when linking a binary that contains Go code built with cgo enabled, pthread need to be linked. (I think now CI is failing because of this).
| -o "${CMAKE_CURRENT_BINARY_DIR}/${LIB_NAME}" | ||
| ${go_library_SRCS} | ||
| WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}) | ||
| add_custom_target(${TARGET_NAME}_lib ALL DEPENDS ${TARGET_NAME}_timestamp ${go_library_DEPS}) |
There was a problem hiding this comment.
Have not found where go_library_DEPS is set?
Also please consider let the Go tool to get Go dependency rather than manually specifying it in cmake: https://github.com/PaddlePaddle/Paddle/pull/2182/files/af065196400ca0254e61df3916888f4341306e97#r117078644
|
I wrote a design doc #2187 in hope that we all on the same page with the implementation of |
Travis CI already included golang