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

Improve workspace support #115

Merged
merged 21 commits into from
Jun 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
8 changes: 2 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,9 @@ before_install:
- sudo dpkg -i wrk_4.0.1-2_amd64.deb
- rm wrk_4.0.1-2_amd64.deb
install:
- make install
- make jenkins-install
script:
- make generate
- make check-generate
- make lint
- make test-only
- make cover
- make jenkins-test
- make fast-bench
- make bins
- make test-benchmark-runner
Expand Down
53 changes: 40 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
PKGS = $(shell glide novendor)
PKGS = $(shell glide novendor | grep -v "workspace/...")
PKG_FILES = benchmarks codegen examples runtime test

COVER_PKGS = $(shell glide novendor | grep -v "test/..." | \
grep -v "main/..." | grep -v "benchmarks/..." | \
awk -vORS=, '{ print $1 }' | sed 's/,$$/\n/')
grep -v "workspace/..." | awk -vORS=, '{ print $1 }' | sed 's/,$$/\n/')

GO_FILES := $(shell \
find . '(' -path '*/.*' -o -path './vendor' ')' -prune \
-o -name '*.go' -print | cut -b3-)
find . '(' -path '*/.*' -o -path './vendor' -o -path './workspace' ')' -prune -o -name '*.go' -print | cut -b3-)

FILTER_LINT := grep -v -e "vendor/" -e "third_party/" -e "gen-code/" -e "codegen/templates/" -e "codegen/template_bundle/"

Expand All @@ -23,12 +22,12 @@ EXAMPLE_SERVICES = $(sort $(dir $(wildcard $(EXAMPLE_SERVICES_DIR)*/)))
check-licence:
@echo "Checking uber-licence..."
@ls ./node_modules/.bin/uber-licence >/dev/null 2>&1 || npm i uber-licence
@./node_modules/.bin/uber-licence --dry --file '*.go' --dir '!vendor' --dir '!examples' --dir '!.tmp_gen' --dir '!template_bundle'
@./node_modules/.bin/uber-licence --dry --file '*.go' --dir '!workspace' --dir '!vendor' --dir '!examples' --dir '!.tmp_gen' --dir '!template_bundle'

.PHONY: fix-licence
fix-licence:
@ls ./node_modules/.bin/uber-licence >/dev/null 2>&1 || npm i uber-licence
./node_modules/.bin/uber-licence --file '*.go' --dir '!vendor' --dir '!examples' --dir '!.tmp_gen' --dir '!template_bundle'
./node_modules/.bin/uber-licence --file '*.go' --dir '!vendor' --dir '!workspace' --dir '!examples' --dir '!.tmp_gen' --dir '!template_bundle'

.PHONY: install
install:
Expand Down Expand Up @@ -95,15 +94,11 @@ check-generate:
rm -rf ./examples/example-gateway/build
make generate
git status --porcelain > git-status.log
@[ ! -s git-status.log ] || ( cat git-status.log ; git diff ; [ ! -s git-status.log ] );
@[ ! -s git-status.log ] || ( cat git-status.log ; git --no-pager diff ; [ ! -s git-status.log ] );

.PHONY: test-all
test-all:
$(MAKE) generate
$(MAKE) check-generate
$(MAKE) lint
$(MAKE) test-only
$(MAKE) cover
$(MAKE) jenkins
$(MAKE) fast-bench
$(MAKE) bins
$(MAKE) install-wrk
Expand Down Expand Up @@ -225,7 +220,6 @@ view-gocov:
open coverage/index.html; \
fi


.PHONY: view-cover
view-cover:
go tool cover -html=./coverage/cover.out
Expand All @@ -245,3 +239,36 @@ tag-build:
git add VERSION
cat VERSION | xargs -I{} git commit -m "build: {}"
cat VERSION | xargs -I{} git tag -m "build: {}" "build-{}"

.PHONY: jenkins-install
jenkins-install:
PWD=$(pwd)
@rm -rf ./vendor/
@rm -rf ./workspace/
@mkdir -p ./workspace/src/github.com/uber/
@ln -s $(PWD) workspace/src/github.com/uber/zanzibar
cd workspace/src/github.com/uber/zanzibar && \
GOPATH=$(PWD)/workspace \
PATH=$(PWD)/workspace/bin:$(PATH) \
make install

.PHONY: jenkins-test
jenkins-test:
PWD=$(pwd)
cd workspace/src/github.com/uber/zanzibar && \
GOPATH=$(PWD)/workspace \
PATH=$(PWD)/workspace/bin:$(PATH) \
make check-generate
cd workspace/src/github.com/uber/zanzibar && \
GOPATH=$(PWD)/workspace \
PATH=$(PWD)/workspace/bin:$(PATH) \
make lint
cd workspace/src/github.com/uber/zanzibar && \
GOPATH=$(PWD)/workspace \
PATH=$(PWD)/workspace/bin:$(PATH) \
make test-only

.PHONY: jenkins
jenkins:
$(MAKE) jenkins-install
$(MAKE) jenkins-test
4 changes: 4 additions & 0 deletions codegen/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,8 @@ type EndpointSpec struct {
ClientName string
// if "httpClient", which client method to call.
ClientMethod string
// The client for this endpoint if httpClient or tchannelClient
ClientSpec *ClientSpec
}

func ensureFields(config map[string]interface{}, mandatoryFields []string, jsonFile string) error {
Expand Down Expand Up @@ -708,6 +710,8 @@ func (e *EndpointSpec) SetDownstream(
)
}

e.ClientSpec = clientSpec

return e.ModuleSpec.SetDownstream(
e.ThriftServiceName, e.ThriftMethodName,
clientSpec, e.ClientMethod, h,
Expand Down
12 changes: 8 additions & 4 deletions codegen/module_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -853,10 +853,14 @@ func (g *EndpointGenerator) generateEndpointTestFile(
tempName := "endpoint_test.tmpl"
if e.WorkflowType == "tchannelClient" {
meta.ClientName = e.ClientName
meta.IncludedPackages = append(method.Downstream.IncludedPackages, GoPackageImport{
AliasName: method.Downstream.PackageName,
PackageName: method.Downstream.GoPackage,
})

meta.IncludedPackages = append(
method.Downstream.IncludedPackages,
GoPackageImport{
AliasName: method.Downstream.PackageName,
PackageName: e.ClientSpec.GoPackageName,
},
)
tempName = "endpoint_test_tchannel_client.tmpl"
}

Expand Down
64 changes: 22 additions & 42 deletions codegen/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ type PackageHelper struct {
packageRoot string
// The root directory containing thrift files.
thriftRootDir string
// Namespace under thrift folder
gatewayNamespace string
// The go package name of where all the generated structs are
genCodePackage string
// The directory to put the generated service code.
targetGenDir string
// The go package name where all the generated code is
goGatewayNamespace string
// The root directory for the gateway test config files.
testConfigsRootDir string
// String containing copyright header to add to generated code.
Expand All @@ -57,23 +57,29 @@ func NewPackageHelper(
thriftRootDir string,
genCodePackage string,
targetGenDir string,
gatewayNamespace string,
copyrightHeader string,
) (*PackageHelper, error) {
genDir, err := filepath.Abs(targetGenDir)
if err != nil {
return nil, errors.Errorf("%s is not valid path: %s", targetGenDir, err)
}

genDirIndex := strings.Index(genDir, gatewayNamespace)
if genDirIndex == -1 {
absConfigDir, err := filepath.Abs(configDirName)
if err != nil {
return nil, errors.Errorf(
"gatewayNamespace (%s) must be inside targetGenDir (%s)",
gatewayNamespace,
genDir,
"%s is not valid path: %s", configDirName, err,
)
}

relativeGenDir, err := filepath.Rel(absConfigDir, genDir)
if err != nil {
return nil, errors.Wrapf(
err, "Could not compute relative dir for %q", genDir,
)
}

goGatewayNamespace := filepath.Join(packageRoot, relativeGenDir)

middlewareSpecs := map[string]*MiddlewareSpec{}
middleConfig := filepath.Join(
configDirName,
Expand All @@ -90,13 +96,13 @@ func NewPackageHelper(
}

p := &PackageHelper{
packageRoot: packageRoot,
thriftRootDir: path.Clean(thriftRootDir),
genCodePackage: genCodePackage,
gatewayNamespace: gatewayNamespace,
targetGenDir: genDir,
copyrightHeader: copyrightHeader,
middlewareSpecs: middlewareSpecs,
packageRoot: packageRoot,
thriftRootDir: path.Clean(thriftRootDir),
genCodePackage: genCodePackage,
goGatewayNamespace: goGatewayNamespace,
targetGenDir: genDir,
copyrightHeader: copyrightHeader,
middlewareSpecs: middlewareSpecs,
}
return p, nil
}
Expand Down Expand Up @@ -132,11 +138,7 @@ func (p PackageHelper) TypeImportPath(thrift string) (string, error) {

// GoGatewayPackageName returns the name of the gateway package
func (p PackageHelper) GoGatewayPackageName() string {
nsIndex := strings.Index(p.targetGenDir, p.gatewayNamespace)
return path.Join(
p.gatewayNamespace,
p.targetGenDir[nsIndex+len(p.gatewayNamespace):],
)
return p.goGatewayNamespace
}

// ThriftIDLPath returns the file path to the thrift idl folder
Expand All @@ -150,28 +152,6 @@ func (p PackageHelper) CodeGenTargetPath() string {
return p.targetGenDir
}

// PackageGenPath returns the Go package path for generated code from a thrift file.
func (p PackageHelper) PackageGenPath(thrift string) (string, error) {
if !strings.HasSuffix(thrift, ".thrift") {
return "", errors.Errorf("file %s is not .thrift", thrift)
}

idx := strings.Index(thrift, p.thriftRootDir)
if idx == -1 {
return "", errors.Errorf(
"file %s is not in thrift dir (%s)",
thrift, p.thriftRootDir,
)
}

nsIndex := strings.Index(p.targetGenDir, p.gatewayNamespace)
return path.Join(
p.gatewayNamespace,
p.targetGenDir[nsIndex+len(p.gatewayNamespace):],
filepath.Dir(thrift[idx+len(p.thriftRootDir):]),
), nil
}

// TypePackageName returns the package name that defines the type.
func (p PackageHelper) TypePackageName(thrift string) (string, error) {
if !strings.HasSuffix(thrift, ".thrift") {
Expand Down
13 changes: 0 additions & 13 deletions codegen/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func newPackageHelper(t *testing.T) *codegen.PackageHelper {
filepath.Join(absGatewayPath, "idl"),
"github.com/uber/zanzibar/examples/example-gateway/build/gen-code",
tmpDir,
"github.com/uber/zanzibar",
testCopyrightHeader,
)
if !assert.NoError(t, err, "failed to create package helper") {
Expand Down Expand Up @@ -98,18 +97,6 @@ func TestTypePackageName(t *testing.T) {
assert.Error(t, err, "should return error for not a thrift file")
}

func TestPackageGenPath(t *testing.T) {
h := newPackageHelper(t)
p, err := h.PackageGenPath(fooThrift)
assert.Nil(t, err, "should not return error")
exp := "github.com/uber/zanzibar/.tmp_gen/clients/foo"
assert.Equal(t, exp, p, "wrong generated Go package path")
_, err = h.PackageGenPath("/Users/xxx/go/src/github.com/uber/zanzibar/examples/example-gateway/build/idl/github.com/uber/zanzibar/clients/foo/foo.go")
assert.Error(t, err, "should return error for not a thrift file")
_, err = h.PackageGenPath("/Users/xxx/go/src/github.com/uber/zanzibar/examples/example-gateway/build/zanzibar/clients/foo/foo.thrift")
assert.Error(t, err, "should return error for not in IDL dir")
}

func TestEndpointTestConfigPath(t *testing.T) {
h := newPackageHelper(t)
p := h.EndpointTestConfigPath("foo", "bar")
Expand Down
1 change: 0 additions & 1 deletion codegen/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ func main() {
filepath.Join(configDirName, config.MustGetString("thriftRootDir")),
config.MustGetString("genCodePackage"),
filepath.Join(configDirName, config.MustGetString("targetGenDir")),
config.MustGetString("gatewayNamespace"),
string(copyright),
)
checkError(
Expand Down
13 changes: 3 additions & 10 deletions codegen/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ type ModuleSpec struct {
ThriftFile string
// Whether the ThriftFile should have annotations or not
WantAnnot bool
// Go package path of this module.
GoPackage string
// Go package name, generated base on module name.
PackageName string
// Go client types file path, generated from thrift file.
Expand Down Expand Up @@ -71,15 +69,10 @@ func NewModuleSpec(thrift string, wantAnnot bool, packageHelper *PackageHelper)
if err != nil {
return nil, errors.Wrap(err, "failed parse thrift file")
}
targetPackage, err := packageHelper.PackageGenPath(module.ThriftPath)
if err != nil {
return nil, errors.Wrap(err, "failed to generate target package path")
}

moduleSpec := &ModuleSpec{
WantAnnot: wantAnnot,
ThriftFile: module.ThriftPath,
GoPackage: targetPackage,
PackageName: camelCase(module.GetName()),
}
if err := moduleSpec.AddServices(module, packageHelper); err != nil {
Expand Down Expand Up @@ -244,15 +237,15 @@ func (ms *ModuleSpec) SetDownstream(
for _, service := range ms.Services {
for _, method := range service.Methods {
d := method.Downstream
if d != nil && !ms.isPackageIncluded(d.GoPackage) {
if d != nil && !ms.isPackageIncluded(d.GoThriftTypesFilePath) {
// thrift types file is optional...
if method.Downstream.GoThriftTypesFilePath == "" {
if d.GoThriftTypesFilePath == "" {
continue
}

ms.IncludedPackages = append(
ms.IncludedPackages, GoPackageImport{
PackageName: method.Downstream.GoThriftTypesFilePath,
PackageName: d.GoThriftTypesFilePath,
AliasName: "",
},
)
Expand Down
2 changes: 1 addition & 1 deletion codegen/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestModuleSpec(t *testing.T) {
}

func convertThriftPathToRelative(m *codegen.ModuleSpec) {
index := strings.Index(m.ThriftFile, "zanzibar")
index := strings.LastIndex(m.ThriftFile, "zanzibar")
m.ThriftFile = m.ThriftFile[index:]

for _, service := range m.Services {
Expand Down
1 change: 0 additions & 1 deletion codegen/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ func TestGenerateBar(t *testing.T) {
filepath.Join(absGatewayPath, "idl"),
"github.com/uber/zanzibar/examples/example-gateway/build/gen-code",
tmpDir,
"github.com/uber/zanzibar",
testCopyrightHeader,
)
if !assert.NoError(t, err, "failed to create package helper", err) {
Expand Down
1 change: 0 additions & 1 deletion codegen/test_data/bar.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"ThriftFile": "zanzibar/examples/example-gateway/idl/endpoints/bar/bar.thrift",
"WantAnnot": true,
"GoPackage": "github.com/uber/zanzibar/.tmp_gen/endpoints/bar",
"PackageName": "bar",
"GoThriftTypesFilePath": "",
"IncludedPackages": [
Expand Down
1 change: 0 additions & 1 deletion examples/example-gateway/gateway.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"gatewayName": "example-gateway",
"packageRoot": "github.com/uber/zanzibar/examples/example-gateway",
"thriftRootDir": "./idl",
"gatewayNamespace": "github.com/uber/zanzibar",
"genCodePackage": "github.com/uber/zanzibar/examples/example-gateway/build/gen-code",
"targetGenDir": "./build",

Expand Down
7 changes: 4 additions & 3 deletions scripts/cover.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ start=`date +%s`

if [ $# -eq 0 ]
then
FILES=$(go list ./... | grep -v "vendor" | \
FILES=$(go list -e ./... | grep -v "vendor" | \
grep -v "workspace" | \
grep "test\|examples\|runtime\|codegen")
elif [ $# -eq 1 ]
then
FILES=$(go list ./... | grep -v "vendor" | \
grep "$1")
FILES=$(go list -e ./... | grep -v "vendor" | \
grep -v "workspace" | grep "$1")
else
FILES=$@
fi
Expand Down
1 change: 1 addition & 0 deletions scripts/easy_json/easy_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ func main() {
return
}

// Do not paralleliz, not worth it.
Copy link
Contributor

Choose a reason for hiding this comment

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

sp : parallelize

for _, file := range files {
easyJSONFile := file[0:len(file)-3] + "_easyjson.go"
oldChecksum := getOldChecksum(easyJSONFile)
Expand Down
Loading