From 0fb0487dbb96ed0a46f9807d81fd07c7dfacf2c9 Mon Sep 17 00:00:00 2001 From: jichangjun Date: Sun, 14 Jun 2020 19:37:27 +0800 Subject: [PATCH 1/7] refactor cover method --- cmd/build.go | 3 +- cmd/commonflags.go | 5 ++ cmd/cover.go | 170 +-------------------------------------------- cmd/install.go | 3 +- pkg/cover/cover.go | 164 ++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 174 insertions(+), 171 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index 1791313..3a5e3f8 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -18,6 +18,7 @@ package cmd import ( "github.com/qiniu/goc/pkg/build" + "github.com/qiniu/goc/pkg/cover" "github.com/spf13/cobra" ) @@ -52,7 +53,7 @@ goc build -- -ldflags "-extldflags -static" -tags="embed kodo" }() // doCover with original buildFlags, with new GOPATH( tmp:original ) // in the tmp directory - doCover(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir) + cover.Execute(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir, mode, center) // do install in the temporary directory gocBuild.Build() return diff --git a/cmd/commonflags.go b/cmd/commonflags.go index 1371929..f18f622 100644 --- a/cmd/commonflags.go +++ b/cmd/commonflags.go @@ -13,6 +13,9 @@ var ( buildFlags string packages string appArgs string + + goRunExecFlag string + goRunArguments string ) // addBasicFlags adds a @@ -41,6 +44,8 @@ func addRunFlags(cmdset *pflag.FlagSet) { addBuildFlags(cmdset) cmdset.Lookup("packages").Usage = "specify the package name, only ., ./... and *.go are supported" cmdset.StringVar(&appArgs, "appargs", "", "specify the application's arguments") + cmdset.StringVar(&goRunExecFlag, "exec", "", "same as -exec flag in 'go run' command") + cmdset.StringVar(&goRunArguments, "arguments", "", "same as 'arguments' in 'go run' command") // bind to viper viper.BindPFlags(cmdset) } diff --git a/cmd/cover.go b/cmd/cover.go index fef9874..c8eef85 100644 --- a/cmd/cover.go +++ b/cmd/cover.go @@ -17,13 +17,10 @@ package cmd import ( - "fmt" + "github.com/qiniu/goc/pkg/cover" log "github.com/sirupsen/logrus" "github.com/spf13/viper" - "os" - "strings" - "github.com/qiniu/goc/pkg/cover" "github.com/spf13/cobra" ) @@ -52,7 +49,7 @@ goc cover --center=http://127.0.0.1:7777 --target=/path/to/target --mode=atomic log.Fatalf("unknown -mode %v", mode) } - doCover(buildFlags, "", target) + cover.Execute(buildFlags, "", target, mode, center) }, } @@ -61,166 +58,3 @@ func init() { addCommonFlags(coverCmd.Flags()) rootCmd.AddCommand(coverCmd) } - -func doCover(args string, newgopath string, target string) { - if !isDirExist(target) { - log.Fatalf("target directory %s not exist", target) - } - - listArgs := []string{"-json"} - if len(args) != 0 { - listArgs = append(listArgs, args) - } - listArgs = append(listArgs, "./...") - pkgs := cover.ListPackages(target, strings.Join(listArgs, " "), newgopath) - - var seen = make(map[string]*cover.PackageCover) - var seenCache = make(map[string]*cover.PackageCover) - for _, pkg := range pkgs { - if pkg.Name == "main" { - log.Printf("handle package: %v", pkg.ImportPath) - // inject the main package - mainCover, err := cover.AddCounters(pkg, mode, newgopath) - if err != nil { - log.Fatalf("failed to add counters for pkg %s, err: %v", pkg.ImportPath, err) - } - - // new a testcover for this service - tc := cover.TestCover{ - Mode: mode, - Center: center, - MainPkgCover: mainCover, - } - - // handle its dependency - var internalPkgCache = make(map[string][]*cover.PackageCover) - tc.CacheCover = make(map[string]*cover.PackageCover) - for _, dep := range pkg.Deps { - - if packageCover, ok := seen[dep]; ok { - tc.DepsCover = append(tc.DepsCover, packageCover) - continue - } - - //only focus package neither standard Go library nor dependency library - if depPkg, ok := pkgs[dep]; ok { - - if findInternal(dep) { - - //scan exist cache cover to tc.CacheCover - if cache, ok := seenCache[dep]; ok { - log.Printf("cache cover exist: %s", cache.Package.ImportPath) - tc.CacheCover[cache.Package.Dir] = cache - continue - } - - // add counter for internal package - inPkgCover, err := cover.AddCounters(depPkg, mode, newgopath) - if err != nil { - log.Fatalf("failed to add counters for internal pkg %s, err: %v", depPkg.ImportPath, err) - } - parentDir := getInternalParent(depPkg.Dir) - parentImportPath := getInternalParent(depPkg.ImportPath) - - //if internal parent dir or import is root path, ignore the dep. the dep is Go library nor dependency library - if parentDir == "" { - continue - } - if parentImportPath == "" { - continue - } - - pkg := &cover.Package{ - ImportPath: parentImportPath, - Dir: parentDir, - } - - // Some internal package have same parent dir or import path - // Cache all vars by internal parent dir for all child internal counter vars - cacheCover := cover.AddCacheCover(pkg, inPkgCover) - if v, ok := tc.CacheCover[cacheCover.Package.Dir]; ok { - for cVar, val := range v.Vars { - cacheCover.Vars[cVar] = val - } - tc.CacheCover[cacheCover.Package.Dir] = cacheCover - } else { - tc.CacheCover[cacheCover.Package.Dir] = cacheCover - } - - // Cache all internal vars to internal parent package - inCover := cover.CacheInternalCover(inPkgCover) - if v, ok := internalPkgCache[cacheCover.Package.Dir]; ok { - v = append(v, inCover) - internalPkgCache[cacheCover.Package.Dir] = v - } else { - var covers []*cover.PackageCover - covers = append(covers, inCover) - internalPkgCache[cacheCover.Package.Dir] = covers - } - seenCache[dep] = cacheCover - continue - } - - packageCover, err := cover.AddCounters(depPkg, mode, newgopath) - if err != nil { - log.Fatalf("failed to add counters for pkg %s, err: %v", depPkg.ImportPath, err) - } - tc.DepsCover = append(tc.DepsCover, packageCover) - seen[dep] = packageCover - } - } - - if errs := cover.InjectCacheCounters(internalPkgCache, tc.CacheCover); len(errs) > 0 { - log.Fatalf("failed to inject cache counters for package: %s, err: %v", pkg.ImportPath, errs) - } - - // inject Http Cover APIs - var httpCoverApis = fmt.Sprintf("%s/http_cover_apis_auto_generated.go", pkg.Dir) - if err := cover.InjectCountersHandlers(tc, httpCoverApis); err != nil { - log.Fatalf("failed to inject counters for package: %s, err: %v", pkg.ImportPath, err) - } - } - } -} - -func isDirExist(path string) bool { - s, err := os.Stat(path) - if err != nil { - return false - } - return s.IsDir() -} - -// Refer: https://github.com/golang/go/blob/master/src/cmd/go/internal/load/pkg.go#L1334:6 -// findInternal looks for the final "internal" path element in the given import path. -// If there isn't one, findInternal returns ok=false. -// Otherwise, findInternal returns ok=true and the index of the "internal". -func findInternal(path string) bool { - // Three cases, depending on internal at start/end of string or not. - // The order matters: we must return the index of the final element, - // because the final one produces the most restrictive requirement - // on the importer. - switch { - case strings.HasSuffix(path, "/internal"): - return true - case strings.Contains(path, "/internal/"): - return true - case path == "internal", strings.HasPrefix(path, "internal/"): - return true - } - return false -} - -func getInternalParent(path string) string { - switch { - case strings.HasSuffix(path, "/internal"): - return strings.Split(path, "/internal")[0] - case strings.Contains(path, "/internal/"): - return strings.Split(path, "/internal/")[0] - case path == "internal": - return "" - case strings.HasPrefix(path, "internal/"): - return strings.Split(path, "internal/")[0] - } - return "" -} diff --git a/cmd/install.go b/cmd/install.go index 08659e2..625210b 100644 --- a/cmd/install.go +++ b/cmd/install.go @@ -18,6 +18,7 @@ package cmd import ( "github.com/qiniu/goc/pkg/build" + "github.com/qiniu/goc/pkg/cover" "github.com/spf13/cobra" ) @@ -49,7 +50,7 @@ goc build --buildflags="-ldflags '-extldflags -static' -tags='embed kodo'" }() // doCover with original buildFlags, with new GOPATH( tmp:original ) // in the tmp directory - doCover(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir) + cover.Execute(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir, mode, center) // do install in the temporary directory gocBuild.Install() }, diff --git a/pkg/cover/cover.go b/pkg/cover/cover.go index 3e8db5b..b013a6d 100644 --- a/pkg/cover/cover.go +++ b/pkg/cover/cover.go @@ -22,7 +22,6 @@ import ( "crypto/sha256" "encoding/json" "fmt" - log "github.com/sirupsen/logrus" "io" "io/ioutil" "os" @@ -33,6 +32,8 @@ import ( "strings" "time" + log "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus" ) @@ -112,6 +113,125 @@ type PackageError struct { Err string // the error itself } +//Execute execute go tool cover for all the .go files in the target folder +func Execute(args, newGopath, target, mode, center string) { + if !isDirExist(target) { + log.Fatalf("target directory %s not exist", target) + } + + listArgs := []string{"-json"} + if len(args) != 0 { + listArgs = append(listArgs, args) + } + listArgs = append(listArgs, "./...") + pkgs := ListPackages(target, strings.Join(listArgs, " "), newGopath) + + var seen = make(map[string]*PackageCover) + var seenCache = make(map[string]*PackageCover) + for _, pkg := range pkgs { + if pkg.Name == "main" { + log.Printf("handle package: %v", pkg.ImportPath) + // inject the main package + mainCover, err := AddCounters(pkg, mode, newGopath) + if err != nil { + log.Fatalf("failed to add counters for pkg %s, err: %v", pkg.ImportPath, err) + } + + // new a testcover for this service + tc := TestCover{ + Mode: mode, + Center: center, + MainPkgCover: mainCover, + } + + // handle its dependency + var internalPkgCache = make(map[string][]*PackageCover) + tc.CacheCover = make(map[string]*PackageCover) + for _, dep := range pkg.Deps { + if packageCover, ok := seen[dep]; ok { + tc.DepsCover = append(tc.DepsCover, packageCover) + continue + } + + //only focus package neither standard Go library nor dependency library + if depPkg, ok := pkgs[dep]; ok { + if findInternal(dep) { + //scan exist cache cover to tc.CacheCover + if cache, ok := seenCache[dep]; ok { + log.Printf("cache cover exist: %s", cache.Package.ImportPath) + tc.CacheCover[cache.Package.Dir] = cache + continue + } + + // add counter for internal package + inPkgCover, err := AddCounters(depPkg, mode, newGopath) + if err != nil { + log.Fatalf("failed to add counters for internal pkg %s, err: %v", depPkg.ImportPath, err) + } + parentDir := getInternalParent(depPkg.Dir) + parentImportPath := getInternalParent(depPkg.ImportPath) + + //if internal parent dir or import is root path, ignore the dep. the dep is Go library nor dependency library + if parentDir == "" { + continue + } + if parentImportPath == "" { + continue + } + + pkg := &Package{ + ImportPath: parentImportPath, + Dir: parentDir, + } + + // Some internal package have same parent dir or import path + // Cache all vars by internal parent dir for all child internal counter vars + cacheCover := AddCacheCover(pkg, inPkgCover) + if v, ok := tc.CacheCover[cacheCover.Package.Dir]; ok { + for cVar, val := range v.Vars { + cacheCover.Vars[cVar] = val + } + tc.CacheCover[cacheCover.Package.Dir] = cacheCover + } else { + tc.CacheCover[cacheCover.Package.Dir] = cacheCover + } + + // Cache all internal vars to internal parent package + inCover := CacheInternalCover(inPkgCover) + if v, ok := internalPkgCache[cacheCover.Package.Dir]; ok { + v = append(v, inCover) + internalPkgCache[cacheCover.Package.Dir] = v + } else { + var covers []*PackageCover + covers = append(covers, inCover) + internalPkgCache[cacheCover.Package.Dir] = covers + } + seenCache[dep] = cacheCover + continue + } + + packageCover, err := AddCounters(depPkg, mode, newGopath) + if err != nil { + log.Fatalf("failed to add counters for pkg %s, err: %v", depPkg.ImportPath, err) + } + tc.DepsCover = append(tc.DepsCover, packageCover) + seen[dep] = packageCover + } + } + + if errs := InjectCacheCounters(internalPkgCache, tc.CacheCover); len(errs) > 0 { + log.Fatalf("failed to inject cache counters for package: %s, err: %v", pkg.ImportPath, errs) + } + + // inject Http Cover APIs + var httpCoverApis = fmt.Sprintf("%s/http_cover_apis_auto_generated.go", pkg.Dir) + if err := InjectCountersHandlers(tc, httpCoverApis); err != nil { + log.Fatalf("failed to inject counters for package: %s, err: %v", pkg.ImportPath, err) + } + } + } +} + // ListPackages list all packages under specific via go list command // The argument newgopath is if you need to go list in a different GOPATH func ListPackages(dir string, args string, newgopath string) map[string]*Package { @@ -167,6 +287,48 @@ func AddCounters(pkg *Package, mode, newgopath string) (*PackageCover, error) { }, nil } +func isDirExist(path string) bool { + s, err := os.Stat(path) + if err != nil { + return false + } + return s.IsDir() +} + +// Refer: https://github.com/golang/go/blob/master/src/cmd/go/internal/load/pkg.go#L1334:6 +// findInternal looks for the final "internal" path element in the given import path. +// If there isn't one, findInternal returns ok=false. +// Otherwise, findInternal returns ok=true and the index of the "internal". +func findInternal(path string) bool { + // Three cases, depending on internal at start/end of string or not. + // The order matters: we must return the index of the final element, + // because the final one produces the most restrictive requirement + // on the importer. + switch { + case strings.HasSuffix(path, "/internal"): + return true + case strings.Contains(path, "/internal/"): + return true + case path == "internal", strings.HasPrefix(path, "internal/"): + return true + } + return false +} + +func getInternalParent(path string) string { + switch { + case strings.HasSuffix(path, "/internal"): + return strings.Split(path, "/internal")[0] + case strings.Contains(path, "/internal/"): + return strings.Split(path, "/internal/")[0] + case path == "internal": + return "" + case strings.HasPrefix(path, "internal/"): + return strings.Split(path, "internal/")[0] + } + return "" +} + func buildCoverCmd(file string, coverVar *FileVar, pkg *Package, mode, newgopath string) *exec.Cmd { // to construct: go tool cover -mode=atomic -o dest src (note: dest==src) var newArgs = []string{"tool", "cover"} From 4ad2bc3fd7b8302ed6ac5510efbd2a889dde454c Mon Sep 17 00:00:00 2001 From: jichangjun Date: Sun, 14 Jun 2020 19:38:41 +0800 Subject: [PATCH 2/7] feature: add run command --- cmd/cover.go | 3 +- cmd/run.go | 73 ++++++++++++++++++++++++++++++++++++++++++ pkg/build/build.go | 34 +++++++++++++++++--- pkg/build/gomodules.go | 3 +- pkg/build/install.go | 6 ++-- pkg/build/legacy.go | 2 +- pkg/build/tmpfolder.go | 3 +- pkg/cover/server.go | 3 +- 8 files changed, 112 insertions(+), 15 deletions(-) create mode 100644 cmd/run.go diff --git a/cmd/cover.go b/cmd/cover.go index c8eef85..a9ae61c 100644 --- a/cmd/cover.go +++ b/cmd/cover.go @@ -19,9 +19,8 @@ package cmd import ( "github.com/qiniu/goc/pkg/cover" log "github.com/sirupsen/logrus" - "github.com/spf13/viper" - "github.com/spf13/cobra" + "github.com/spf13/viper" ) var coverCmd = &cobra.Command{ diff --git a/cmd/run.go b/cmd/run.go new file mode 100644 index 0000000..6cc0187 --- /dev/null +++ b/cmd/run.go @@ -0,0 +1,73 @@ +/* + Copyright 2020 Qiniu Cloud (qiniu.com) + + 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. +*/ + +package cmd + +import ( + "fmt" + "io/ioutil" + "net" + + "github.com/qiniu/goc/pkg/build" + "github.com/qiniu/goc/pkg/cover" + log "github.com/sirupsen/logrus" + "github.com/spf13/cobra" +) + +var runCmd = &cobra.Command{ + Use: "run", + Short: "Run covers and runs the named main Go package", + Long: `Run covers and runs the named main Go package, +It is exactly behave as 'go run .' in addition of some internal goc features.`, + Example: ` +goc run . +`, + Run: func(cmd *cobra.Command, args []string) { + gocBuild := build.NewBuild(buildFlags, packages, buildOutput) + gocBuild.GoRunExecFlag = goRunExecFlag + gocBuild.GoRunArguments = goRunArguments + defer func() { + if !debugGoc { + gocBuild.Clean() + } + }() + // start goc server + var l = newLocalListener() + go cover.GocServer(ioutil.Discard).RunListener(l) + gocServer := fmt.Sprintf("http://%s", l.Addr().String()) + fmt.Printf("[goc] goc server started: %s \n", gocServer) + + // execute covers for the target source with original buildFlags and new GOPATH( tmp:original ) + cover.Execute(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir, mode, gocServer) + + gocBuild.Run() + }, +} + +func init() { + addRunFlags(runCmd.Flags()) + rootCmd.AddCommand(runCmd) +} + +func newLocalListener() net.Listener { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + if l, err = net.Listen("tcp6", "[::1]:0"); err != nil { + log.Fatalf("failed to listen on a port: %v", err) + } + } + return l +} diff --git a/pkg/build/build.go b/pkg/build/build.go index 854461b..983ddb2 100644 --- a/pkg/build/build.go +++ b/pkg/build/build.go @@ -35,10 +35,17 @@ type Build struct { TmpDir string // the temporary directory to build the project TmpWorkingDir string // the working directory in the temporary directory, which is corresponding to the current directory in the project directory IsMod bool // determine whether it is a Mod project - BuildFlags string // Build flags - Packages string // Packages that needs to build Root string // Project Root Target string // the binary name that go build generate + + // keep compatible with go commands: + // go run [build flags] [-exec xprog] package [arguments...] + // go build [-o output] [-i] [build flags] [packages] + // go install [-i] [build flags] [packages] + BuildFlags string // Build flags + Packages string // Packages that needs to build + GoRunExecFlag string // for the -exec flags in go run command + GoRunArguments string // for the '[arguments]' parameters in go run command } // NewBuild creates a Build struct which can build from goc temporary directory, @@ -108,7 +115,26 @@ func (b *Build) determineOutputDir(outputDir string) string { func (b *Build) validatePackageForBuild() bool { if b.Packages == "." { return true - } else { - return false + } + return false +} + +// Run excutes the main package in addition with the internal goc features +func (b *Build) Run() { + cmd := exec.Command("/bin/bash", "-c", "go run "+b.BuildFlags+" "+b.GoRunExecFlag+" "+b.Packages+" "+b.GoRunArguments) + cmd.Dir = b.TmpWorkingDir + + if b.NewGOPATH != "" { + // Change to temp GOPATH for go install command + cmd.Env = append(os.Environ(), fmt.Sprintf("GOPATH=%v", b.NewGOPATH)) + } + + log.Printf("go build cmd is: %v", cmd.Args) + out, err := cmd.CombinedOutput() + if err != nil { + log.Fatalf("Fail to execute: %v. The error is: %v, the stdout/stderr is: %v", cmd.Args, err, string(out)) + } + if len(out) > 0 { + fmt.Println(string(out)) } } diff --git a/pkg/build/gomodules.go b/pkg/build/gomodules.go index f593c8d..502a226 100644 --- a/pkg/build/gomodules.go +++ b/pkg/build/gomodules.go @@ -17,9 +17,8 @@ package build import ( - log "github.com/sirupsen/logrus" - "github.com/otiai10/copy" + log "github.com/sirupsen/logrus" ) func (b *Build) cpGoModulesProject() { diff --git a/pkg/build/install.go b/pkg/build/install.go index cb11720..8ab25d3 100644 --- a/pkg/build/install.go +++ b/pkg/build/install.go @@ -18,9 +18,10 @@ package build import ( "fmt" - log "github.com/sirupsen/logrus" "os" "os/exec" + + log "github.com/sirupsen/logrus" ) // NewInstall creates a Build struct which can install from goc temporary directory @@ -59,7 +60,6 @@ func (b *Build) Install() { func (b *Build) validatePackageForInstall() bool { if b.Packages == "." || b.Packages == "./..." { return true - } else { - return false } + return false } diff --git a/pkg/build/legacy.go b/pkg/build/legacy.go index ed6244a..962dea7 100644 --- a/pkg/build/legacy.go +++ b/pkg/build/legacy.go @@ -17,12 +17,12 @@ package build import ( - log "github.com/sirupsen/logrus" "os" "path/filepath" "github.com/otiai10/copy" "github.com/qiniu/goc/pkg/cover" + log "github.com/sirupsen/logrus" ) func (b *Build) cpLegacyProject() { diff --git a/pkg/build/tmpfolder.go b/pkg/build/tmpfolder.go index aff30bd..b80632f 100644 --- a/pkg/build/tmpfolder.go +++ b/pkg/build/tmpfolder.go @@ -23,9 +23,8 @@ import ( "path/filepath" "strings" - log "github.com/sirupsen/logrus" - "github.com/qiniu/goc/pkg/cover" + log "github.com/sirupsen/logrus" ) func (b *Build) MvProjectsToTmp() { diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 93e9038..0545f1f 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -19,7 +19,6 @@ package cover import ( "bytes" "fmt" - log "github.com/sirupsen/logrus" "io" "io/ioutil" "net" @@ -27,6 +26,8 @@ import ( "net/url" "os" + log "github.com/sirupsen/logrus" + "github.com/gin-gonic/gin" "golang.org/x/tools/cover" "k8s.io/test-infra/gopherage/pkg/cov" From c82e61dcfed5923ca7888ba8c9644e0ba9fc7c9e Mon Sep 17 00:00:00 2001 From: jichangjun Date: Mon, 15 Jun 2020 15:29:49 +0800 Subject: [PATCH 3/7] use viper to get value of debug flag --- cmd/build.go | 6 +----- cmd/install.go | 6 +----- cmd/run.go | 6 +----- pkg/build/tmpfolder.go | 6 +++++- pkg/cover/server.go | 3 +-- 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index 3a5e3f8..cce6cd6 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -46,11 +46,7 @@ goc build -- -ldflags "-extldflags -static" -tags="embed kodo" Run: func(cmd *cobra.Command, args []string) { gocBuild := build.NewBuild(buildFlags, packages, buildOutput) // remove temporary directory if needed - defer func() { - if !debugGoc { - gocBuild.Clean() - } - }() + defer gocBuild.Clean() // doCover with original buildFlags, with new GOPATH( tmp:original ) // in the tmp directory cover.Execute(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir, mode, center) diff --git a/cmd/install.go b/cmd/install.go index 625210b..66f1f5e 100644 --- a/cmd/install.go +++ b/cmd/install.go @@ -43,11 +43,7 @@ goc build --buildflags="-ldflags '-extldflags -static' -tags='embed kodo'" Run: func(cmd *cobra.Command, args []string) { gocBuild := build.NewInstall(buildFlags, packages) // remove temporary directory if needed - defer func() { - if !debugGoc { - gocBuild.Clean() - } - }() + defer gocBuild.Clean() // doCover with original buildFlags, with new GOPATH( tmp:original ) // in the tmp directory cover.Execute(buildFlags, gocBuild.NewGOPATH, gocBuild.TmpDir, mode, center) diff --git a/cmd/run.go b/cmd/run.go index 6cc0187..8863985 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -39,11 +39,7 @@ goc run . gocBuild := build.NewBuild(buildFlags, packages, buildOutput) gocBuild.GoRunExecFlag = goRunExecFlag gocBuild.GoRunArguments = goRunArguments - defer func() { - if !debugGoc { - gocBuild.Clean() - } - }() + defer gocBuild.Clean() // start goc server var l = newLocalListener() go cover.GocServer(ioutil.Discard).RunListener(l) diff --git a/pkg/build/tmpfolder.go b/pkg/build/tmpfolder.go index b80632f..d9de295 100644 --- a/pkg/build/tmpfolder.go +++ b/pkg/build/tmpfolder.go @@ -25,6 +25,7 @@ import ( "github.com/qiniu/goc/pkg/cover" log "github.com/sirupsen/logrus" + "github.com/spf13/viper" ) func (b *Build) MvProjectsToTmp() { @@ -155,5 +156,8 @@ func (b *Build) findWhereToInstall() string { // Clean clears up the temporary workspace func (b *Build) Clean() error { - return os.RemoveAll(b.TmpDir) + if !viper.GetBool("debug") { + return os.RemoveAll(b.TmpDir) + } + return nil } diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 0545f1f..2d5adb0 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -26,9 +26,8 @@ import ( "net/url" "os" - log "github.com/sirupsen/logrus" - "github.com/gin-gonic/gin" + log "github.com/sirupsen/logrus" "golang.org/x/tools/cover" "k8s.io/test-infra/gopherage/pkg/cov" ) From 3dd2c9cd212bebf53f0f8012b75a37014a618365 Mon Sep 17 00:00:00 2001 From: jichangjun Date: Mon, 15 Jun 2020 15:54:36 +0800 Subject: [PATCH 4/7] capture logs when executing goc run --- cmd/run.go | 1 + pkg/build/build.go | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmd/run.go b/cmd/run.go index 8863985..1bac99a 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -40,6 +40,7 @@ goc run . gocBuild.GoRunExecFlag = goRunExecFlag gocBuild.GoRunArguments = goRunArguments defer gocBuild.Clean() + // start goc server var l = newLocalListener() go cover.GocServer(ioutil.Discard).RunListener(l) diff --git a/pkg/build/build.go b/pkg/build/build.go index 983ddb2..8f62041 100644 --- a/pkg/build/build.go +++ b/pkg/build/build.go @@ -130,11 +130,15 @@ func (b *Build) Run() { } log.Printf("go build cmd is: %v", cmd.Args) - out, err := cmd.CombinedOutput() + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Start() if err != nil { - log.Fatalf("Fail to execute: %v. The error is: %v, the stdout/stderr is: %v", cmd.Args, err, string(out)) + log.Fatalf("Fail to start command: %v. The error is: %v", cmd.Args, err) } - if len(out) > 0 { - fmt.Println(string(out)) + + if err = cmd.Wait(); err != nil { + log.Fatalf("Fail to execute command: %v. The error is: %v", cmd.Args, err) } + } From 99fe3e5fe49bc654d64eb89fe0b787e67153440c Mon Sep 17 00:00:00 2001 From: jichangjun Date: Mon, 15 Jun 2020 17:16:50 +0800 Subject: [PATCH 5/7] only save services informaion into memory for goc run mode --- cmd/run.go | 3 + pkg/cover/server.go | 16 ++--- pkg/cover/store.go | 156 ++++++++++++++++++++++++++++------------ pkg/cover/store_test.go | 2 +- 4 files changed, 121 insertions(+), 56 deletions(-) diff --git a/cmd/run.go b/cmd/run.go index 1bac99a..ad00f79 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -41,6 +41,9 @@ goc run . gocBuild.GoRunArguments = goRunArguments defer gocBuild.Clean() + // only save services in memory + cover.DefaultStore = cover.NewMemoryStore() + // start goc server var l = newLocalListener() go cover.GocServer(ioutil.Discard).RunListener(l) diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 2d5adb0..e3bec00 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -32,14 +32,14 @@ import ( "k8s.io/test-infra/gopherage/pkg/cov" ) -// LocalStore implements the IPersistence interface -var LocalStore Store +// DefaultStore implements the IPersistence interface +var DefaultStore Store // LogFile a file to save log. const LogFile = "goc.log" func init() { - LocalStore = NewStore() + DefaultStore = NewFileStore() } // Run starts coverage host center @@ -82,7 +82,7 @@ type Service struct { //listServices list all the registered services func listServices(c *gin.Context) { - services := LocalStore.GetAll() + services := DefaultStore.GetAll() c.JSON(http.StatusOK, services) } @@ -109,7 +109,7 @@ func registerService(c *gin.Context) { log.Printf("the registed host %s of service %s is different with the real one %s, here we choose the real one", service.Name, host, realIP) service.Address = fmt.Sprintf("http://%s:%s", realIP, port) } - if err := LocalStore.Add(service); err != nil { + if err := DefaultStore.Add(service); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } @@ -118,7 +118,7 @@ func registerService(c *gin.Context) { } func profile(c *gin.Context) { - svrsUnderTest := LocalStore.GetAll() + svrsUnderTest := DefaultStore.GetAll() var mergedProfiles = make([][]*cover.Profile, len(svrsUnderTest)) for _, addrs := range svrsUnderTest { for _, addr := range addrs { @@ -154,7 +154,7 @@ func profile(c *gin.Context) { } func clear(c *gin.Context) { - svrsUnderTest := LocalStore.GetAll() + svrsUnderTest := DefaultStore.GetAll() for svc, addrs := range svrsUnderTest { for _, addr := range addrs { pp, err := NewWorker(addr).Clear() @@ -168,7 +168,7 @@ func clear(c *gin.Context) { } func initSystem(c *gin.Context) { - if err := LocalStore.Init(); err != nil { + if err := DefaultStore.Init(); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/pkg/cover/store.go b/pkg/cover/store.go index f69d51f..a18aa9a 100644 --- a/pkg/cover/store.go +++ b/pkg/cover/store.go @@ -19,10 +19,11 @@ package cover import ( "bufio" "fmt" - log "github.com/sirupsen/logrus" "os" "strings" "sync" + + log "github.com/sirupsen/logrus" ) // Store persistents the registered service information @@ -38,77 +39,82 @@ type Store interface { // Init cleanup all the registered service information Init() error + + // Set stores the services information into internal state + Set(services map[string][]string) } // PersistenceFile is the file to save services address information const PersistenceFile = "_svrs_address.txt" -// localStore holds the registered services into memory and persistent to a local file -type localStore struct { +// fileStore holds the registered services into memory and persistent to a local file +type fileStore struct { mu sync.RWMutex - servicesMap map[string][]string persistentFile string + + memoryStore Store } -// Add adds the given service to localStore -func (l *localStore) Add(s Service) error { - l.mu.Lock() - defer l.mu.Unlock() - // load to memory - if addrs, ok := l.servicesMap[s.Name]; ok { - for _, addr := range addrs { - if addr == s.Address { - log.Printf("service registered already, name: %s, address: %s", s.Name, s.Address) - return nil - } - } - addrs = append(addrs, s.Address) - l.servicesMap[s.Name] = addrs - } else { - l.servicesMap[s.Name] = []string{s.Address} +// NewFileStore creates a store using local file +func NewFileStore() Store { + l := &fileStore{ + persistentFile: PersistenceFile, + memoryStore: NewMemoryStore(), } + if err := l.load(); err != nil { + log.Fatalf("load failed, file: %s, err: %v", l.persistentFile, err) + } + + return l +} + +// Add adds the given service to file Store +func (l *fileStore) Add(s Service) error { + l.memoryStore.Add(s) + // persistent to local store + l.mu.Lock() + defer l.mu.Unlock() return l.appendToFile(s) } // Get returns the registered service information with the given name -func (l *localStore) Get(name string) []string { - l.mu.RLock() - defer l.mu.RUnlock() - return l.servicesMap[name] +func (l *fileStore) Get(name string) []string { + return l.memoryStore.Get(name) } // Get returns all the registered service information -func (l *localStore) GetAll() map[string][]string { - l.mu.RLock() - defer l.mu.RUnlock() - return l.servicesMap +func (l *fileStore) GetAll() map[string][]string { + return l.memoryStore.GetAll() } // Init cleanup all the registered service information // and the local persistent file -func (l *localStore) Init() error { +func (l *fileStore) Init() error { + if err := l.memoryStore.Init(); err != nil { + return err + } + l.mu.Lock() defer l.mu.Unlock() if err := os.Remove(l.persistentFile); err != nil && !os.IsNotExist(err) { return fmt.Errorf("failed to delete file %s, err: %v", l.persistentFile, err) } - l.servicesMap = make(map[string][]string, 0) return nil } // load all registered service from file to memory -func (l *localStore) load() (map[string][]string, error) { +func (l *fileStore) load() error { var svrsMap = make(map[string][]string, 0) f, err := os.Open(l.persistentFile) if err != nil { if os.IsNotExist(err) { - return svrsMap, nil + return nil } - return svrsMap, fmt.Errorf("failed to open file, path: %s, err: %v", l.persistentFile, err) + return fmt.Errorf("failed to open file, path: %s, err: %v", l.persistentFile, err) } defer f.Close() @@ -129,13 +135,19 @@ func (l *localStore) load() (map[string][]string, error) { } if err := ns.Err(); err != nil { - return svrsMap, fmt.Errorf("read file failed, file: %s, err: %v", l.persistentFile, err) + return fmt.Errorf("read file failed, file: %s, err: %v", l.persistentFile, err) } - return svrsMap, nil + // set information to memory + l.memoryStore.Set(svrsMap) + return nil } -func (l *localStore) appendToFile(s Service) error { +func (l *fileStore) Set(services map[string][]string) { + panic("TO BE IMPLEMENTED") +} + +func (l *fileStore) appendToFile(s Service) error { f, err := os.OpenFile(l.persistentFile, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600) if err != nil { return err @@ -159,17 +171,67 @@ func split(r rune) bool { return r == '&' } -// NewStore creates a store using local file -func NewStore() Store { - l := &localStore{ - persistentFile: PersistenceFile, - servicesMap: make(map[string][]string, 0), +// memoryStore holds the registered services only into memory +type memoryStore struct { + mu sync.RWMutex + servicesMap map[string][]string +} + +// NewMemoryStore creates a memory store +func NewMemoryStore() Store { + return &memoryStore{ + servicesMap: make(map[string][]string, 0), + } +} + +// Add adds the given service to MemoryStore +func (l *memoryStore) Add(s Service) error { + l.mu.Lock() + defer l.mu.Unlock() + // load to memory + if addrs, ok := l.servicesMap[s.Name]; ok { + for _, addr := range addrs { + if addr == s.Address { + log.Printf("service registered already, name: %s, address: %s", s.Name, s.Address) + return nil + } + } + addrs = append(addrs, s.Address) + l.servicesMap[s.Name] = addrs + } else { + l.servicesMap[s.Name] = []string{s.Address} } - services, err := l.load() - if err != nil { - log.Fatalf("load failed, file: %s, err: %v", l.persistentFile, err) - } - l.servicesMap = services - return l + return nil +} + +// Get returns the registered service information with the given name +func (l *memoryStore) Get(name string) []string { + l.mu.RLock() + defer l.mu.RUnlock() + return l.servicesMap[name] +} + +// Get returns all the registered service information +func (l *memoryStore) GetAll() map[string][]string { + l.mu.RLock() + defer l.mu.RUnlock() + return l.servicesMap +} + +// Init cleanup all the registered service information +// and the local persistent file +func (l *memoryStore) Init() error { + l.mu.Lock() + defer l.mu.Unlock() + + l.servicesMap = make(map[string][]string, 0) + return nil +} + +func (l *memoryStore) Set(services map[string][]string) { + l.mu.Lock() + defer l.mu.Unlock() + + l.servicesMap = services } diff --git a/pkg/cover/store_test.go b/pkg/cover/store_test.go index 5a5b1eb..bd61e9f 100644 --- a/pkg/cover/store_test.go +++ b/pkg/cover/store_test.go @@ -21,7 +21,7 @@ import ( ) func TestLocalStore(t *testing.T) { - localStore := NewStore() + localStore := NewFileStore() var tc1 = Service{ Name: "a", Address: "http://127.0.0.1", From 76b03178c054537ff957e33f73e96049a97836e0 Mon Sep 17 00:00:00 2001 From: jichangjun Date: Mon, 15 Jun 2020 17:34:54 +0800 Subject: [PATCH 6/7] adjust log behavior for goc server and agent --- pkg/cover/instrument.go | 7 ++++--- pkg/cover/server.go | 10 ++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/cover/instrument.go b/pkg/cover/instrument.go index 6932b94..1672480 100644 --- a/pkg/cover/instrument.go +++ b/pkg/cover/instrument.go @@ -145,9 +145,10 @@ func clearFileCover(counter []uint32) { func registerHandlers() { ln, host, err := listen() if err != nil { - log.Fatalf("profile listen failed, err:%v", err) + log.Fatalf("listen failed, err:%v", err) } - log.Println("profile listen on", host) + + fmt.Printf("[goc] goc agent listened on: %s \n", host) profileAddr := "http://" + host if resp, err := registerSelf(profileAddr); err != nil { log.Fatalf("register address %v failed, err: %v, response: %v", profileAddr, err, string(resp)) @@ -220,7 +221,7 @@ func registerSelf(address string) ([]byte, error) { resp, err := http.DefaultClient.Do(req) if err != nil && isNetworkError(err) { - log.Printf("[WARN]error occured:%v, try again", err) + log.Printf("[goc][WARN]error occured:%v, try again", err) resp, err = http.DefaultClient.Do(req) } defer resp.Body.Close() diff --git a/pkg/cover/server.go b/pkg/cover/server.go index e3bec00..996946e 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -48,16 +48,18 @@ func Run(port string) { if err != nil { log.Fatalf("failed to create log file %s, err: %v", LogFile, err) } - r := GocServer(f) + + // both log to stdout and file by default + mw := io.MultiWriter(f, os.Stdout) + r := GocServer(mw) log.Fatal(r.Run(port)) } // GocServer init goc server engine func GocServer(w io.Writer) *gin.Engine { - if w != nil && w != os.Stdout { - gin.DefaultWriter = io.MultiWriter(w, os.Stdout) + if w != nil { + gin.DefaultWriter = w } - r := gin.Default() // api to show the registered services r.StaticFile(PersistenceFile, "./"+PersistenceFile) From 2c101f62f10360bf6c71cf5ee58cb6f728c2e3ae Mon Sep 17 00:00:00 2001 From: jichangjun Date: Mon, 15 Jun 2020 19:22:38 +0800 Subject: [PATCH 7/7] add test cases --- pkg/cover/cover.go | 10 ++--- pkg/cover/cover_test.go | 81 ++++++++++++++++++++++++++++++++++++++++- pkg/cover/store_test.go | 2 +- 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/pkg/cover/cover.go b/pkg/cover/cover.go index b013a6d..e28ae66 100644 --- a/pkg/cover/cover.go +++ b/pkg/cover/cover.go @@ -155,7 +155,7 @@ func Execute(args, newGopath, target, mode, center string) { //only focus package neither standard Go library nor dependency library if depPkg, ok := pkgs[dep]; ok { - if findInternal(dep) { + if hasInternalPath(dep) { //scan exist cache cover to tc.CacheCover if cache, ok := seenCache[dep]; ok { log.Printf("cache cover exist: %s", cache.Package.ImportPath) @@ -296,10 +296,10 @@ func isDirExist(path string) bool { } // Refer: https://github.com/golang/go/blob/master/src/cmd/go/internal/load/pkg.go#L1334:6 -// findInternal looks for the final "internal" path element in the given import path. -// If there isn't one, findInternal returns ok=false. -// Otherwise, findInternal returns ok=true and the index of the "internal". -func findInternal(path string) bool { +// hasInternalPath looks for the final "internal" path element in the given import path. +// If there isn't one, hasInternalPath returns ok=false. +// Otherwise, hasInternalPath returns ok=true and the index of the "internal". +func hasInternalPath(path string) bool { // Three cases, depending on internal at start/end of string or not. // The order matters: we must return the index of the final element, // because the final one produces the most restrictive requirement diff --git a/pkg/cover/cover_test.go b/pkg/cover/cover_test.go index 05dfb3d..1113b87 100644 --- a/pkg/cover/cover_test.go +++ b/pkg/cover/cover_test.go @@ -18,7 +18,6 @@ package cover import ( "fmt" - log "github.com/sirupsen/logrus" "os" "os/exec" "path/filepath" @@ -26,6 +25,8 @@ import ( "strings" "testing" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" ) @@ -225,3 +226,81 @@ func TestDeclareCoverVars(t *testing.T) { } } + +func TestGetInternalParent(t *testing.T) { + var tcs = []struct { + ImportPath string + expectedParent string + }{ + { + ImportPath: "a/internal/b", + expectedParent: "a", + }, + { + ImportPath: "internal/b", + expectedParent: "", + }, + { + ImportPath: "a/b/internal/b", + expectedParent: "a/b", + }, + { + ImportPath: "a/b/internal", + expectedParent: "a/b", + }, + { + ImportPath: "a/b/internal/c", + expectedParent: "a/b", + }, + { + ImportPath: "a/b/c", + expectedParent: "", + }, + { + ImportPath: "", + expectedParent: "", + }, + } + + for _, tc := range tcs { + actual := getInternalParent(tc.ImportPath) + if actual != tc.expectedParent { + t.Errorf("getInternalParent failed for importPath %s, expected %s, got %s", tc.ImportPath, tc.expectedParent, actual) + } + } +} + +func TestFindInternal(t *testing.T) { + var tcs = []struct { + ImportPath string + expectedParent bool + }{ + { + ImportPath: "a/internal/b", + expectedParent: true, + }, + { + ImportPath: "internal/b", + expectedParent: true, + }, + { + ImportPath: "a/b/internal", + expectedParent: true, + }, + { + ImportPath: "a/b/c", + expectedParent: false, + }, + { + ImportPath: "internal", + expectedParent: true, + }, + } + + for _, tc := range tcs { + actual := hasInternalPath(tc.ImportPath) + if actual != tc.expectedParent { + t.Errorf("hasInternalPath check failed for importPath %s", tc.ImportPath) + } + } +} diff --git a/pkg/cover/store_test.go b/pkg/cover/store_test.go index bd61e9f..f4bd8aa 100644 --- a/pkg/cover/store_test.go +++ b/pkg/cover/store_test.go @@ -44,7 +44,7 @@ func TestLocalStore(t *testing.T) { localStore.Add(tc4) addrs := localStore.Get(tc1.Name) if len(addrs) != 2 { - t.Error("unexpect result") + t.Error("unexpected result") } for _, addr := range addrs {