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

Design Doc: New Building System #2187

Merged
merged 2 commits into from
May 18, 2017

Conversation

wangkuiyi
Copy link
Collaborator

Fixes #2154


go_test(api_test
SRCS
api_test.go
Copy link
Contributor

@helinwang helinwang May 18, 2017

Choose a reason for hiding this comment

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

There will be a lot of go files to test, can we not manually add them? I think go test is a much more descriptive, maybe we can integrate it into cmake?

This should run all tests in current directory and all of its subdirectories:

$ go test ./...
This should run all tests with import path prefixed with foo/:

$ go test foo/...
This should run all tests import path prefixed with foo:

$ go test foo...
This should run all tests in your $GOPATH:

$ go test ...

reference: http://stackoverflow.com/a/16353449/852385

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we should use ctest instead of go test to run tests in Go.

We can make go_test call

go test -c hello_test.go -o hello_test

generated at build-time are place in `/build`.

Under `/build/go`, our build system creates a symoblic link `src`
pointing to `/go`, where all Go source code resides.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add content in #2182 (comment) to this design doc.

# api.go is changed, api need to be re-built.
go_library(api
SRCS
api.go
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the library should be added by package, not by file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right -- Go toolchain can and can only build Go packages/binaries by packages, but not by files.

I verified this property by creating /tmp/a.go, which includes a function Add, and /tmp/main.go, which includes function main and run

cd /tmp
$ go build a.go -o lia.a
named files must be .go files


go_test(api_test
SRCS
api_test.go
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the test should be added by package, not by file.

This assumes that all Go code, including third-party packages, must
be in the local repo. Actually, it assumes that `$GOPATH` must be
in the local repo. This would affect how we write `import`
statemetns, and how we maintain third-party packages.
Copy link
Contributor

Choose a reason for hiding this comment

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

statemetns -> statements

1. Each library package is a sub-directory under `/go`.
1. Each (executable) binary package is a sub-directory under
`/go/cmd`. This is the source tree convention of Go itself.
1. Each 3rd-party Go package is a Git submodule under `/go`.
Copy link
Contributor

@helinwang helinwang May 18, 2017

Choose a reason for hiding this comment

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

There are many pros for using this submodule based solution:

  1. dependency version management is easy (by git hash).
  2. can modify dependency when doing experiments in the local filesystem.

Here are the cons that I can think of:

  1. Go import path support version control system like "git", "hg", "svn", (go help importpath), but using submodule is limited on git.
  2. need to manually add submodule for every dependency, when the dependency is already specified in the go source code.

I vote for the approach of set GOPATH to ./build/go and use go get to fetch dependency.

  1. more go idiomatic, dependency only specified in go source code.
  2. can still modify dependency when doing experiments in the local filesystem.
  3. if we want dependency management, we can use go community's spec and solutions: https://github.com/golang/go/wiki/PackageManagementTools

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let us assume that we use Git submodule at the current moment, because this make 3rd party packages look like our own packages, thus doesn't block the development of our new building system.

We can go on learning Go's dependency versioning mechanism and redesign this later in necessary.

@wangkuiyi
Copy link
Collaborator Author

@helinwang I temporarily removed go.md and marked the work in #2210.

Copy link
Contributor

@helinwang helinwang left a comment

Choose a reason for hiding this comment

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

LGTM.

@wangkuiyi wangkuiyi merged commit 9ab4bd8 into PaddlePaddle:develop May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants