From 51601902e2bdb29bee83006a73b48d8aa7334a8d Mon Sep 17 00:00:00 2001 From: Changjun Ji Date: Tue, 22 Dec 2020 15:22:17 +0800 Subject: [PATCH] Revert "optimize the file copy strategy of `goc build`" --- pkg/build/build.go | 7 +-- pkg/build/gomodules.go | 18 +++++++ pkg/build/gomodules_test.go | 3 +- pkg/build/legacy.go | 94 +++++++++++++++++-------------------- pkg/build/legacy_test.go | 61 ++++++++++-------------- pkg/build/tmpfolder.go | 15 ++++-- pkg/build/tmpfolder_test.go | 35 ++------------ pkg/cover/cover.go | 15 +----- 8 files changed, 108 insertions(+), 140 deletions(-) diff --git a/pkg/build/build.go b/pkg/build/build.go index 4fcbe40..052c00d 100644 --- a/pkg/build/build.go +++ b/pkg/build/build.go @@ -133,11 +133,8 @@ func (b *Build) determineOutputDir(outputDir string) (string, error) { targetName := "" for _, pkg := range b.Pkgs { if pkg.Name == "main" { - if pkg.Target != "" { - targetName = filepath.Base(pkg.Target) - } else { - targetName = filepath.Base(pkg.Dir) - } + _, file := filepath.Split(pkg.Target) + targetName = file break } } diff --git a/pkg/build/gomodules.go b/pkg/build/gomodules.go index ac9c07c..32c9b79 100644 --- a/pkg/build/gomodules.go +++ b/pkg/build/gomodules.go @@ -20,9 +20,27 @@ import ( "io/ioutil" "path/filepath" + "github.com/otiai10/copy" + log "github.com/sirupsen/logrus" "golang.org/x/mod/modfile" ) +func (b *Build) cpGoModulesProject() { + for _, v := range b.Pkgs { + if v.Name == "main" { + dst := b.TmpDir + src := v.Module.Dir + + if err := copy.Copy(src, dst); err != nil { + log.Errorf("Failed to Copy the folder from %v to %v, the error is: %v ", src, dst, err) + } + break + } else { + continue + } + } +} + // updateGoModFile rewrites the go.mod file in the temporary directory, // if it has a 'replace' directive, and the directive has a relative local path // it will be rewritten with a absolute path. diff --git a/pkg/build/gomodules_test.go b/pkg/build/gomodules_test.go index 38d7e05..e0e6a6d 100644 --- a/pkg/build/gomodules_test.go +++ b/pkg/build/gomodules_test.go @@ -45,7 +45,6 @@ func TestModProjectCopyWithUnexistedDir(t *testing.T) { Module: &cover.ModulePublic{ Dir: "not exied, ia mas duser", // not real one, should fail copy }, - GoFiles: []string{"empty.go"}, } pkgs["another"] = &cover.Package{} b := &Build{ @@ -53,7 +52,7 @@ func TestModProjectCopyWithUnexistedDir(t *testing.T) { Pkgs: pkgs, } - output := captureOutput(b.cpProject) + output := captureOutput(b.cpGoModulesProject) assert.Equal(t, strings.Contains(output, "Failed to Copy"), true) } diff --git a/pkg/build/legacy.go b/pkg/build/legacy.go index 4b47fda..72f5c55 100644 --- a/pkg/build/legacy.go +++ b/pkg/build/legacy.go @@ -17,8 +17,8 @@ package build import ( + "os" "path/filepath" - "strings" log "github.com/sirupsen/logrus" @@ -26,7 +26,7 @@ import ( "github.com/qiniu/goc/pkg/cover" ) -func (b *Build) cpProject() { +func (b *Build) cpLegacyProject() { visited := make(map[string]bool) for k, v := range b.Pkgs { dst := filepath.Join(b.TmpDir, "src", k) @@ -37,63 +37,55 @@ func (b *Build) cpProject() { continue } - if err := b.copyDir(v); err != nil { + if err := copy.Copy(src, dst); err != nil { + log.Errorf("Failed to Copy the folder from %v to %v, the error is: %v ", src, dst, err) + } + + visited[src] = true + + b.cpDepPackages(v, visited) + } +} + +// only cp dependency in root(current gopath), +// skip deps in other GOPATHs +func (b *Build) cpDepPackages(pkg *cover.Package, visited map[string]bool) { + gopath := pkg.Root + for _, dep := range pkg.Deps { + src := filepath.Join(gopath, "src", dep) + // Check if copied + if _, ok := visited[src]; ok { + // Skip if already copied + continue + } + // Check if we can found in the root gopath + _, err := os.Stat(src) + if err != nil { + continue + } + + dst := filepath.Join(b.TmpDir, "src", dep) + + if err := copy.Copy(src, dst); err != nil { log.Errorf("Failed to Copy the folder from %v to %v, the error is: %v ", src, dst, err) } visited[src] = true } - if b.IsMod { - for _, v := range b.Pkgs { - if v.Name == "main" { - dst := filepath.Join(b.TmpDir, "go.mod") - src := filepath.Join(v.Module.Dir, "go.mod") - if err := copy.Copy(src, dst); err != nil { - log.Errorf("Failed to Copy the go mod file from %v to %v, the error is: %v ", src, dst, err) - } +} - dst = filepath.Join(b.TmpDir, "go.sum") - src = filepath.Join(v.Module.Dir, "go.sum") - if err := copy.Copy(src, dst); err != nil && !strings.Contains(err.Error(), "no such file or directory") { - log.Errorf("Failed to Copy the go mod file from %v to %v, the error is: %v ", src, dst, err) - } - break - } else { - continue +func (b *Build) cpNonStandardLegacy() { + for _, v := range b.Pkgs { + if v.Name == "main" { + dst := b.TmpDir + src := v.Dir + + if err := copy.Copy(src, dst); err != nil { + log.Printf("Failed to Copy the folder from %v to %v, the error is: %v ", src, dst, err) } - } - } -} - -func (b *Build) copyDir(pkg *cover.Package) error { - fileList := []string{} - dir := pkg.Dir - fileList = append(fileList, pkg.GoFiles...) - fileList = append(fileList, pkg.CompiledGoFiles...) - fileList = append(fileList, pkg.IgnoredGoFiles...) - fileList = append(fileList, pkg.CFiles...) - fileList = append(fileList, pkg.CXXFiles...) - fileList = append(fileList, pkg.MFiles...) - fileList = append(fileList, pkg.HFiles...) - fileList = append(fileList, pkg.FFiles...) - fileList = append(fileList, pkg.SFiles...) - fileList = append(fileList, pkg.SwigCXXFiles...) - fileList = append(fileList, pkg.SwigFiles...) - fileList = append(fileList, pkg.SysoFiles...) - for _, file := range fileList { - p := filepath.Join(dir, file) - var src, root string - if pkg.Root == "" { - root = b.WorkingDir // use workingDir instead when the root is empty. + break } else { - root = pkg.Root - } - src = strings.TrimPrefix(pkg.Dir, root) // get the relative path of the files - dst := filepath.Join(b.TmpDir, src, file) // it will adapt the case where src is "" - if err := copy.Copy(p, dst); err != nil { - log.Errorf("Failed to Copy the folder from %v to %v, the error is: %v ", src, dst, err) - return err + continue } } - return nil } diff --git a/pkg/build/legacy_test.go b/pkg/build/legacy_test.go index 48bfd6d..53f9cc1 100644 --- a/pkg/build/legacy_test.go +++ b/pkg/build/legacy_test.go @@ -17,37 +17,16 @@ package build import ( + "path/filepath" "strings" "testing" "github.com/qiniu/goc/pkg/cover" "github.com/stretchr/testify/assert" - "os" ) -// copy in cpProject of invalid src, dst name +// copy in cpLegacyProject/cpNonStandardLegacy of invalid src, dst name func TestLegacyProjectCopyWithUnexistedDir(t *testing.T) { - pkgs := make(map[string]*cover.Package) - pkgs["main"] = &cover.Package{ - Module: &cover.ModulePublic{ - Dir: "not exied, ia mas duser", // not real one, should fail copy - }, - Dir: "not exit, iasdfs", - Name: "main", - GoFiles: []string{"not_exist.go"}, - } - pkgs["another"] = &cover.Package{} - b := &Build{ - TmpDir: "sdfsfev2234444", // not real one, should fail copy - Pkgs: pkgs, - } - - output := captureOutput(b.cpProject) - assert.Equal(t, strings.Contains(output, "Failed to Copy"), true) -} - -// copy goMod project without go.mod -func TestGoModProjectCopyWithUnexistedModFile(t *testing.T) { pkgs := make(map[string]*cover.Package) pkgs["main"] = &cover.Package{ Module: &cover.ModulePublic{ @@ -60,23 +39,33 @@ func TestGoModProjectCopyWithUnexistedModFile(t *testing.T) { b := &Build{ TmpDir: "sdfsfev2234444", // not real one, should fail copy Pkgs: pkgs, - IsMod: true, } - output := captureOutput(b.cpProject) - assert.Equal(t, strings.Contains(output, "Failed to Copy the go mod file"), true) + output := captureOutput(b.cpLegacyProject) + assert.Equal(t, strings.Contains(output, "Failed to Copy"), true) + + output = captureOutput(b.cpNonStandardLegacy) + assert.Equal(t, strings.Contains(output, "Failed to Copy"), true) } -// copy needed files to tmpDir -func TestCopyDir(t *testing.T) { - wd, _ := os.Getwd() - pkg := &cover.Package{Dir: wd, Root: wd, GoFiles: []string{"build.go", "legacy.go"}, CgoFiles: []string{"run.go"}} - tmpDir := "/tmp/test/" +// copy in cpDepPackages of invalid dst name +func TestDepPackagesCopyWithInvalidDir(t *testing.T) { + gopath := filepath.Join(baseDir, "../../tests/samples/simple_gopath_project") + pkg := &cover.Package{ + Module: &cover.ModulePublic{ + Dir: "not exied, ia mas duser", + }, + Root: gopath, + Deps: []string{"qiniu.com", "ddfee 2344234"}, + } b := &Build{ - WorkingDir: "empty", - TmpDir: tmpDir, + TmpDir: "/", // "/" is invalid dst in Linux, it should fail } - assert.NoError(t, os.MkdirAll(tmpDir, os.ModePerm)) - defer os.RemoveAll(tmpDir) - assert.NoError(t, b.copyDir(pkg)) + + output := captureOutput(func() { + visited := make(map[string]bool) + + b.cpDepPackages(pkg, visited) + }) + assert.Equal(t, strings.Contains(output, "Failed to Copy"), true) } diff --git a/pkg/build/tmpfolder.go b/pkg/build/tmpfolder.go index ee76c9d..9cc8fbe 100644 --- a/pkg/build/tmpfolder.go +++ b/pkg/build/tmpfolder.go @@ -57,6 +57,13 @@ func (b *Build) MvProjectsToTmp() error { } else { b.NewGOPATH = fmt.Sprintf("%v:%v", b.TmpDir, b.OriGOPATH) } + // fix #14: unable to build project not in GOPATH in legacy mode + // this kind of project does not have a pkg.Root value + // go 1.11, 1.12 has no pkg.Root, + // so add b.IsMod == false as secondary judgement + if b.Root == "" && b.IsMod == false { + b.NewGOPATH = b.OriGOPATH + } log.Infof("New GOPATH: %v", b.NewGOPATH) return nil } @@ -90,9 +97,9 @@ func (b *Build) mvProjectsToTmp() error { // known cases: // 1. a legacy project, but not in any GOPATH, will cause the b.Root == "" if b.IsMod == false && b.Root != "" { - b.cpProject() + b.cpLegacyProject() } else if b.IsMod == true { // go 1.11, 1.12 has no Build.Root - b.cpProject() + b.cpGoModulesProject() updated, newGoModContent, err := b.updateGoModFile() if err != nil { return fmt.Errorf("fail to generate new go.mod: %v", err) @@ -107,7 +114,9 @@ func (b *Build) mvProjectsToTmp() error { } } else if b.IsMod == false && b.Root == "" { b.TmpWorkingDir = b.TmpDir - b.cpProject() + b.cpNonStandardLegacy() + } else { + return fmt.Errorf("unknown project type: %w", ErrShouldNotReached) } log.Infof("New workingdir in tmp directory in: %v", b.TmpWorkingDir) diff --git a/pkg/build/tmpfolder_test.go b/pkg/build/tmpfolder_test.go index a837366..2183280 100644 --- a/pkg/build/tmpfolder_test.go +++ b/pkg/build/tmpfolder_test.go @@ -23,7 +23,6 @@ import ( "strings" "testing" - "github.com/qiniu/goc/pkg/cover" "github.com/stretchr/testify/assert" ) @@ -96,7 +95,7 @@ func TestLegacyProjectNotInGoPATH(t *testing.T) { os.Setenv("GO111MODULE", "off") b, _ := NewBuild("", []string{"."}, workingDir, "") - if !strings.Contains(b.NewGOPATH, b.OriGOPATH) { + if b.OriGOPATH != b.NewGOPATH { t.Fatalf("New GOPATH should be same with old GOPATH, for this kind of project. New: %v, old: %v", b.NewGOPATH, b.OriGOPATH) } @@ -106,32 +105,13 @@ func TestLegacyProjectNotInGoPATH(t *testing.T) { } } -// test mvProjectsToTmp failed by traversePkgsList error -func TestMvProjectsToTmpFailByTraversePkgsList(t *testing.T) { +// test traversePkgsList error case +func TestTraversePkgsList(t *testing.T) { b := &Build{ Pkgs: nil, } - err := b.mvProjectsToTmp() - assert.Contains(t, err.Error(), ErrShouldNotReached.Error()) -} - -// test mvProjectsToTmp failed by getTmpwd error -func TestMvProjectsToTmpFailByGetTmpwd(t *testing.T) { - pkgs := make(map[string]*cover.Package) - pkgs["main"] = &cover.Package{ - Module: &cover.ModulePublic{ - Dir: "just for test", - Path: "just for test", - }, - Dir: "not exist", - Name: "main", - } - b := &Build{ - Pkgs: pkgs, - WorkingDir: "not exist", - } - err := b.mvProjectsToTmp() - assert.Contains(t, err.Error(), ErrGocShouldExecInProject.Error()) + _, _, err := b.traversePkgsList() + assert.EqualError(t, err, ErrShouldNotReached.Error()) } // test getTmpwd error case @@ -168,8 +148,3 @@ func TestFindWhereToInstall(t *testing.T) { expectedPlace := filepath.Join(os.Getenv("HOME"), "go", "bin") assert.Equal(t, placeToInstall, expectedPlace) } - -func TestMvProjectsToTmp(t *testing.T) { - b := &Build{TmpDir: "/tmp/test"} - fmt.Println(b.MvProjectsToTmp()) -} diff --git a/pkg/cover/cover.go b/pkg/cover/cover.go index f9fa12d..742e352 100644 --- a/pkg/cover/cover.go +++ b/pkg/cover/cover.go @@ -85,19 +85,8 @@ type Package struct { DepOnly bool `json:"DepOnly,omitempty"` // package is only a dependency, not explicitly listed // Source files - GoFiles []string `json:",omitempty"` // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) - CgoFiles []string `json:",omitempty"` // .go source files that import "C" - CompiledGoFiles []string `json:",omitempty"` // .go output from running cgo on CgoFiles - IgnoredGoFiles []string `json:",omitempty"` // .go source files ignored due to build constraints - CFiles []string `json:",omitempty"` // .c source files - CXXFiles []string `json:",omitempty"` // .cc, .cpp and .cxx source files - MFiles []string `json:",omitempty"` // .m source files - HFiles []string `json:",omitempty"` // .h, .hh, .hpp and .hxx source files - FFiles []string `json:",omitempty"` // .f, .F, .for and .f90 Fortran source files - SFiles []string `json:",omitempty"` // .s source files - SwigFiles []string `json:",omitempty"` // .swig files - SwigCXXFiles []string `json:",omitempty"` // .swigcxx files - SysoFiles []string `json:",omitempty"` // .syso system object files added to package + GoFiles []string `json:"GoFiles,omitempty"` // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles) + CgoFiles []string `json:"CgoFiles,omitempty"` // .go source files that import "C" // Dependency information Deps []string `json:"Deps,omitempty"` // all (recursively) imported dependencies