Repository: qiniu/goc
Pull request #148 (2020-12-22 07:22:28)

Revert "optimize the file copy strategy of `goc build`"
Reverts qiniu/goc#143


Patch:
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/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)
 	}
-	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
-			}
+}
+
+// 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
 	}
 }
 
-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.
+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)
+			}
+			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/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/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


Test patch:
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_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,24 +17,23 @@
 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"},
+		Dir:  "not exit, iasdfs",
+		Name: "main",
 	}
 	pkgs["another"] = &cover.Package{}
 	b := &Build{
@@ -42,41 +41,31 @@ func TestLegacyProjectCopyWithUnexistedDir(t *testing.T) {
 		Pkgs:   pkgs,
 	}
 
-	output := captureOutput(b.cpProject)
+	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 goMod project without go.mod
-func TestGoModProjectCopyWithUnexistedModFile(t *testing.T) {
-	pkgs := make(map[string]*cover.Package)
-	pkgs["main"] = &cover.Package{
+// 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", // not real one, should fail copy
+			Dir: "not exied, ia mas duser",
 		},
-		Dir:  "not exit, iasdfs",
-		Name: "main",
+		Root: gopath,
+		Deps: []string{"qiniu.com", "ddfee 2344234"},
 	}
-	pkgs["another"] = &cover.Package{}
 	b := &Build{
-		TmpDir: "sdfsfev2234444", // not real one, should fail copy
-		Pkgs:   pkgs,
-		IsMod:  true,
+		TmpDir: "/", // "/" is invalid dst in Linux, it should fail
 	}
 
-	output := captureOutput(b.cpProject)
-	assert.Equal(t, strings.Contains(output, "Failed to Copy the go mod file"), true)
-}
+	output := captureOutput(func() {
+		visited := make(map[string]bool)
 
-// 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/"
-	b := &Build{
-		WorkingDir: "empty",
-		TmpDir:     tmpDir,
-	}
-	assert.NoError(t, os.MkdirAll(tmpDir, os.ModePerm))
-	defer os.RemoveAll(tmpDir)
-	assert.NoError(t, b.copyDir(pkg))
+		b.cpDepPackages(pkg, visited)
+	})
+	assert.Equal(t, strings.Contains(output, "Failed to Copy"), true)
 }
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())
-}