Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using glide for go package vendor #2627

Merged
merged 9 commits into from
Jul 3, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ option(COVERALLS_UPLOAD "Package code coverage data to coveralls" OFF)
option(ON_TRAVIS "Exclude special unit test on Travis CI" OFF)
option(WITH_C_API "Compile PaddlePaddle with C-API(Prediction)" OFF)
option(WITH_GOLANG "Compile PaddlePaddle with GOLANG" OFF)
option(GLIDE_INSTALL "Download and install go dependencies " ON)
Copy link
Collaborator

@wangkuiyi wangkuiyi Jun 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR!

I think we need a design doc for this work. In particular,

  1. Why we need package manager for Go, or say, why the default toolchain is not enough.
  2. Why we choose glide, comparing with other possible choices.

The design doc, together with discussions with it, would be a deposit of the reasons/thoughts we have in mind, and would be valuable for future contributors to understand the situation so could they move on towards further improvements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll add a design doc before going on working with this PR.

I thought previous discussions in #2583 is the reason why I'm working on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted here: #2643

option(USE_NNPACK "Compile PaddlePaddle with NNPACK library" OFF)

# CMAKE_BUILD_TYPE
Expand Down Expand Up @@ -96,6 +97,7 @@ include(external/warpctc) # download, build, install warpctc
include(external/any) # download libn::any
include(external/eigen) # download eigen3

include(configure) # add paddle env configuration
include(generic) # simplify cmake module
include(package) # set paddle packages
include(cpplint) # set paddle c++ style
Expand All @@ -106,7 +108,7 @@ include(flags) # set paddle compile flags
include(cudnn) # set cudnn libraries
include(version) # set PADDLE_VERSION
include(coveralls) # set code coverage
include(configure) # add paddle env configuration


include_directories("${PROJ_ROOT}")
include_directories("${PROJ_ROOT}/paddle/cuda/include")
Expand Down Expand Up @@ -139,8 +141,7 @@ add_subdirectory(proto)
# "add_subdirectory(paddle)" and "add_subdirectory(python)" should be
# placed after this block, because they depends on it.
if(WITH_GOLANG)
add_subdirectory(go/master/c)
add_subdirectory(go/pserver/cclient)
add_subdirectory(go)
endif(WITH_GOLANG)

add_subdirectory(paddle)
Expand Down
7 changes: 4 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ RUN apt-get update && \
net-tools && \
apt-get clean -y

# Install Go
# Install Go and glide
RUN wget -O go.tgz https://storage.googleapis.com/golang/go1.8.1.linux-amd64.tar.gz && \
tar -C /usr/local -xzf go.tgz && \
mkdir /root/gopath && \
rm go.tgz
rm go.tgz \
curl https://glide.sh/get | sh
ENV GOROOT=/usr/local/go GOPATH=/root/gopath
# should not be in the same line with GOROOT definition, otherwise docker build could not find GOROOT.
ENV PATH=${PATH}:${GOROOT}/bin
Expand All @@ -57,7 +58,7 @@ RUN pip install --upgrade pip && \
pip install -U docopt PyYAML sphinx && \
pip install -U sphinx-rtd-theme==0.1.9 recommonmark && \
pip install pre-commit 'requests==2.9.2' 'ipython==5.3.0' && \
pip install 'ipykernel==4.6.0' 'jupyter==1.0.0' && \
pip install 'ipykernel==4.6.0' 'jupyter==1.0.0' && \
pip install rarfile

# To fix https://github.com/PaddlePaddle/Paddle/issues/1954, we use
Expand Down
26 changes: 22 additions & 4 deletions cmake/configure.cmake
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
#
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down Expand Up @@ -79,6 +79,9 @@ if(WITH_GOLANG)
set(GOPATH "${CMAKE_CURRENT_BINARY_DIR}/go")
file(MAKE_DIRECTORY ${GOPATH})
set(PADDLE_IN_GOPATH "${GOPATH}/src/github.com/PaddlePaddle/Paddle")
file(MAKE_DIRECTORY "${PADDLE_IN_GOPATH}")
set(PADDLE_GO_PATH "${CMAKE_SOURCE_DIR}/go")

add_custom_target(go_path)
add_custom_command(TARGET go_path
# Symlink Paddle directory into GOPATH
Expand All @@ -89,7 +92,22 @@ if(WITH_GOLANG)
# We can't run `go get -d ./...` for every target, because
# multiple `go get` can not run concurrently, but make need to be
# able to run with multiple jobs.
COMMAND env GOPATH=${GOPATH} ${CMAKE_Go_COMPILER} get -d ./go/...
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
)

if (GLIDE_INSTALL)
if(EXISTS $ENV{GOPATH}/bin/glide)
set(GLIDE "$ENV{GOPATH}/bin/glide")
else()
message(FATAL_ERROR "no glide executeble found: $ENV{GOPATH}/bin/glide")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires user to install glide, maybe we can install for them using go get github.com/Masterminds/glide?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. go get will always install the latest version from default branch, which does not guarantee the behavior.
  2. Thought glide is like cmake or gcc, put it into Dockerfileis better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, good point. Thanks.

endif()

add_custom_target(go_vendor)
add_custom_command(TARGET go_vendor
COMMAND env GOPATH=${GOPATH} ${GLIDE} install
WORKING_DIRECTORY "${PADDLE_IN_GOPATH}/go"
)
add_dependencies(go_vendor go_path)
endif()

endif(WITH_GOLANG)
59 changes: 32 additions & 27 deletions cmake/generic.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -17,59 +17,59 @@
# generic.cmake defines CMakes functions that look like Bazel's
# building rules (https://bazel.build/).
#
#
#
# -------------------------------------------
# C++ CUDA C++ Go
# -------------------------------------------
# cc_library nv_library go_library
# cc_binary nv_binary go_binary
# cc_test nv_test go_test
# -------------------------------------------
#
#
# To build a static library example.a from example.cc using the system
# compiler (like GCC):
#
#
# cc_library(example SRCS example.cc)
#
#
# To build a static library example.a from multiple source files
# example{1,2,3}.cc:
#
#
# cc_library(example SRCS example1.cc example2.cc example3.cc)
#
#
# To build a shared library example.so from example.cc:
#
#
# cc_library(example SHARED SRCS example.cc)
#
#
# To build a library using Nvidia's NVCC from .cu file(s), use the nv_
# prefixed version:
#
#
# nv_library(example SRCS example.cu)
#
#
# To specify that a library new_example.a depends on other libraies:
#
#
# cc_library(new_example SRCS new_example.cc DEPS example)
#
#
# Static libraries can be composed of other static libraries:
#
#
# cc_library(composed DEPS dependent1 dependent2 dependent3)
#
#
# To build an executable binary file from some source files and
# dependent libraries:
#
#
# cc_binary(example SRCS main.cc something.cc DEPS example1 example2)
#
#
# To build an executable binary file using NVCC, use the nv_ prefixed
# version:
#
#
# nv_binary(example SRCS main.cc something.cu DEPS example1 example2)
#
#
# To build a unit test binary, which is an executable binary with
# GoogleTest linked:
#
#
# cc_test(example_test SRCS example_test.cc DEPS example)
#
#
# To build a unit test binary using NVCC, use the nv_ prefixed version:
#
#
# nv_test(example_test SRCS example_test.cu DEPS example)
#
# It is pretty often that executable and test binaries depend on
Expand Down Expand Up @@ -278,27 +278,32 @@ function(go_library TARGET_NAME)
set(${TARGET_NAME}_LIB_PATH "${CMAKE_CURRENT_BINARY_DIR}/${${TARGET_NAME}_LIB_NAME}" CACHE STRING "output library path for target ${TARGET_NAME}")

file(GLOB GO_SOURCE RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "*.go")
string(REPLACE "${PADDLE_GO_PATH}/" "" CMAKE_CURRENT_SOURCE_REL_DIR ${CMAKE_CURRENT_SOURCE_DIR})
add_custom_command(TARGET ${TARGET_NAME} POST_BUILD
COMMAND rm "${${TARGET_NAME}_LIB_PATH}"
# Golang build source code
COMMAND env GOPATH=${GOPATH} ${CMAKE_Go_COMPILER} build ${BUILD_MODE}
-o "${${TARGET_NAME}_LIB_PATH}"
${GO_SOURCE}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
add_dependencies(${TARGET_NAME} go_path)
"./${CMAKE_CURRENT_SOURCE_REL_DIR}/${GO_SOURCE}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious the code works before, why now need to be under GOPATH?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When compiling with vendor package, go build must run under GOPATH or it will fail. And this also removes warnings when glide install.

# must run under GOPATH
WORKING_DIRECTORY "${PADDLE_IN_GOPATH}/go")
add_dependencies(${TARGET_NAME} go_vendor)
endfunction(go_library)

function(go_binary TARGET_NAME)
set(options OPTIONAL)
set(oneValueArgs "")
set(multiValueArgs SRCS DEPS)
cmake_parse_arguments(go_binary "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
string(REPLACE "${PADDLE_GO_PATH}/" "" CMAKE_CURRENT_SOURCE_REL_DIR ${CMAKE_CURRENT_SOURCE_DIR})

add_custom_command(OUTPUT ${TARGET_NAME}_timestamp
COMMAND env GOPATH=${GOPATH} ${CMAKE_Go_COMPILER} build
-o "${CMAKE_CURRENT_BINARY_DIR}/${TARGET_NAME}"
${go_library_SRCS}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
add_custom_target(${TARGET_NAME} ALL DEPENDS ${TARGET_NAME}_timestamp ${go_binary_DEPS})
"./${CMAKE_CURRENT_SOURCE_REL_DIR}/${go_binary_SRCS}"
WORKING_DIRECTORY "${PADDLE_IN_GOPATH}/go")
# TODO: don't know what ${TARGET_NAME}_link does
add_custom_target(${TARGET_NAME} ALL DEPENDS go_vendor ${TARGET_NAME}_timestamp ${go_binary_DEPS})
install(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${TARGET_NAME} DESTINATION bin)
endfunction(go_binary)

Expand Down
2 changes: 2 additions & 0 deletions go/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
vendor/
.glide/
19 changes: 19 additions & 0 deletions go/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

add_subdirectory(pserver/cclient)
add_subdirectory(cmd/pserver)
add_subdirectory(cmd/master)
add_subdirectory(master/c)
15 changes: 15 additions & 0 deletions go/cmd/master/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

go_binary(master SRC master.go)
15 changes: 15 additions & 0 deletions go/cmd/pserver/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

go_binary(pserver SRCS pserver.go)
61 changes: 61 additions & 0 deletions go/glide.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions go/glide.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package: github.com/PaddlePaddle/Paddle/go
import:
- package: github.com/PaddlePaddle/recordio
- package: github.com/coreos/etcd
version: ^3.2.1
subpackages:
- clientv3
- clientv3/concurrency
- package: github.com/namsral/flag
version: ^1.7.4-pre
- package: github.com/sirupsen/logrus
version: ^1.0.0
2 changes: 0 additions & 2 deletions go/master/c/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
cmake_minimum_required(VERSION 3.0)

go_library(paddle_master SHARED)