From a2b2dbdbc784aeebab550a75d252925405558522 Mon Sep 17 00:00:00 2001 From: Herman Schaaf Date: Thu, 4 Aug 2016 23:50:18 +0800 Subject: [PATCH] Use VCS module to download repos instead of go get --- cmd/goreportcard.go | 20 +++++++ download/download.go | 120 ++++++++++++++++++++++++++++++++++++++ download/download_test.go | 42 +++++++++++++ handlers/check.go | 27 ++------- handlers/checks.go | 81 ++----------------------- 5 files changed, 192 insertions(+), 98 deletions(-) create mode 100644 cmd/goreportcard.go create mode 100644 download/download.go create mode 100644 download/download_test.go diff --git a/cmd/goreportcard.go b/cmd/goreportcard.go new file mode 100644 index 0000000..c02b47d --- /dev/null +++ b/cmd/goreportcard.go @@ -0,0 +1,20 @@ +package main + +import ( + "fmt" + "os" +) + +func printUsage() { + fmt.Println("\ngoreportcard command line tool") + fmt.Println("\nUsage:\n\n goreportcard [package names...]") +} + +func main() { + if len(os.Args) == 1 { + printUsage() + } + + names := := os.Args[1:] + +} diff --git a/download/download.go b/download/download.go new file mode 100644 index 0000000..eb73f35 --- /dev/null +++ b/download/download.go @@ -0,0 +1,120 @@ +package download + +import ( + "errors" + "log" + "os" + "path/filepath" + "strings" + + "golang.org/x/tools/go/vcs" +) + +// Download takes a user-provided string that represents a remote +// Go repository, and attempts to download it in a way similar to go get. +// It is forgiving in terms of the exact path given: the path may have +// a scheme or username, which will be trimmed. +func Download(path, dest string) (root *vcs.RepoRoot, err error) { + return download(path, dest, false) +} + +func download(path, dest string, firstAttempt bool) (root *vcs.RepoRoot, err error) { + vcs.ShowCmd = true + + path, err = Clean(path) + if err != nil { + return root, err + } + + root, err = vcs.RepoRootForImportPath(path, true) + if err != nil { + return root, err + } + + localDirPath := filepath.Join(dest, root.Root, "..") + + err = os.MkdirAll(localDirPath, 0777) + if err != nil { + return root, err + } + + fullLocalPath := filepath.Join(dest, root.Root) + ex, err := exists(fullLocalPath) + if err != nil { + return root, err + } + if ex { + log.Println("Update", root.Repo) + err = root.VCS.Download(fullLocalPath) + if err != nil && firstAttempt { + // may have been rebased; we delete the directory, then try one more time: + log.Printf("Failed to download %q (%v), trying again...", root.Repo, err.Error()) + err = os.Remove(fullLocalPath) + return download(path, dest, false) + } else if err != nil { + return root, err + } + } else { + log.Println("Create", root.Repo) + + err = root.VCS.Create(fullLocalPath, root.Repo) + if err != nil { + return root, err + } + } + err = root.VCS.TagSync(fullLocalPath, "") + if err != nil && firstAttempt { + // may have been rebased; we delete the directory, then try one more time: + log.Printf("Failed to update %q (%v), trying again...", root.Repo, err.Error()) + err = os.Remove(fullLocalPath) + return download(path, dest, false) + } + return root, err +} + +// Clean trims any URL parts, like the scheme or username, that might be present +// in a user-submitted URL +func Clean(path string) (string, error) { + importPath := trimUsername(trimScheme(path)) + root, err := vcs.RepoRootForImportPath(importPath, true) + if root.Root == "" || root.Repo == "" { + return root.Root, errors.New("Empty repo root") + } + return root.Root, err +} + +// trimScheme removes a scheme (e.g. https://) from the URL for more +// convenient pasting from browsers. +func trimScheme(repo string) string { + schemeSep := "://" + schemeSepIdx := strings.Index(repo, schemeSep) + if schemeSepIdx > -1 { + return repo[schemeSepIdx+len(schemeSep):] + } + + return repo +} + +// trimUsername removes the username for a URL, if it is present +func trimUsername(repo string) string { + usernameSep := "@" + usernameSepIdx := strings.Index(repo, usernameSep) + if usernameSepIdx > -1 { + return repo[usernameSepIdx+len(usernameSep):] + } + + return repo +} + +// exists returns whether the given file or directory exists or not +// from http://stackoverflow.com/a/10510783 +func exists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return true, err +} diff --git a/download/download_test.go b/download/download_test.go new file mode 100644 index 0000000..2b73fef --- /dev/null +++ b/download/download_test.go @@ -0,0 +1,42 @@ +package download + +import ( + "os" + "path/filepath" + "testing" +) + +var testDownloadDir = "test_downloads" + +func TestRepoRootForImportPath(t *testing.T) { + cases := []struct { + giveURL string + wantPath string + wantVCS string + }{ + {"github.com/gojp/goreportcard", "github.com/gojp/goreportcard", "git"}, + {"https://github.com/boltdb/bolt", "github.com/boltdb/bolt", "git"}, + {"https://bitbucket.org/rickb777/go-talk", "bitbucket.org/rickb777/go-talk", "hg"}, + {"ssh://hg@bitbucket.org/rickb777/go-talk", "bitbucket.org/rickb777/go-talk", "hg"}, + } + + for _, tt := range cases { + root, err := Download(tt.giveURL, testDownloadDir) + if err != nil { + t.Fatalf("Error calling Download(%q): %v", tt.giveURL, err) + } + + if root.Root != tt.wantPath { + t.Errorf("Download(%q): root.Repo = %q, want %q", tt.giveURL, root.Repo, tt.wantPath) + } + + wantPath := filepath.Join(testDownloadDir, tt.wantPath) + ex, _ := exists(wantPath) + if !ex { + t.Errorf("Download(%q): %q was not created", tt.giveURL, wantPath) + } + } + + // clean up the test + os.RemoveAll(testDownloadDir) +} diff --git a/handlers/check.go b/handlers/check.go index 133e0b2..3967ed1 100644 --- a/handlers/check.go +++ b/handlers/check.go @@ -9,9 +9,8 @@ import ( "strings" "time" - "golang.org/x/tools/go/vcs" - "github.com/boltdb/bolt" + "github.com/gojp/goreportcard/download" ) const ( @@ -26,29 +25,15 @@ const ( MetaBucket string = "meta" ) -// trimScheme removes a scheme (e.g. https://) from the URL for more -// convenient pasting from browsers. -func trimScheme(repo string) string { - schemeSep := "://" - schemeSepIdx := strings.Index(repo, schemeSep) - if schemeSepIdx > -1 { - return repo[schemeSepIdx+len(schemeSep):] - } - - return repo -} - // CheckHandler handles the request for checking a repo func CheckHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - repo := trimScheme(r.FormValue("repo")) - - repoRoot, err := vcs.RepoRootForImportPath(repo, true) - if err != nil || repoRoot.Root == "" || repoRoot.Repo == "" { - log.Println("Failed to create repoRoot:", repoRoot, err) + repo, err := download.Clean(r.FormValue("repo")) + if err != nil { + log.Println("ERROR: from download.Clean:", err) w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(`Please enter a valid 'go get'-able package name`)) + w.Write([]byte(`Could not download the repository: ` + err.Error())) return } @@ -59,7 +44,7 @@ func CheckHandler(w http.ResponseWriter, r *http.Request) { if err != nil { log.Println("ERROR: from newChecksResp:", err) w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(`Could not go get the repository.`)) + w.Write([]byte(`Could not download the repository.`)) return } diff --git a/handlers/checks.go b/handlers/checks.go index 2d2a735..ec17e33 100644 --- a/handlers/checks.go +++ b/handlers/checks.go @@ -4,19 +4,15 @@ import ( "encoding/json" "errors" "fmt" - "io/ioutil" "log" - "os" - "os/exec" - "path/filepath" "regexp" "sort" - "strings" "time" "github.com/boltdb/bolt" humanize "github.com/dustin/go-humanize" "github.com/gojp/goreportcard/check" + "github.com/gojp/goreportcard/download" ) var reBadRepo = regexp.MustCompile(`package\s([\w\/\.]+)\: exit status \d+`) @@ -81,77 +77,6 @@ type checksResp struct { HumanizedLastRefresh string `json:"humanized_last_refresh"` } -func goGet(repo string, prevError string) error { - log.Printf("Go getting %q...", repo) - if err := os.Mkdir("repos", 0755); err != nil && !os.IsExist(err) { - return fmt.Errorf("could not create dir: %v", err) - } - d, err := filepath.Abs("repos") - if err != nil { - return fmt.Errorf("could not fetch absolute path: %v", err) - } - - os.Setenv("GOPATH", d) - cmd := exec.Command("go", "get", "-d", "-u", repo) - cmd.Stdout = os.Stdout - stderr, err := cmd.StderrPipe() - if err != nil { - return fmt.Errorf("could not get stderr pipe: %v", err) - } - - err = cmd.Start() - if err != nil { - return fmt.Errorf("could not start command: %v", err) - } - - b, err := ioutil.ReadAll(stderr) - if err != nil { - return fmt.Errorf("could not read stderr: %v", err) - } - - err = cmd.Wait() - errStr := string(b) - - // we don't care if there are no buildable Go source files, we just need the source on disk - hadError := err != nil && !strings.Contains(errStr, "no buildable Go source files") - - if hadError { - log.Println("Go get error log:", string(b)) - if errStr != prevError { - // try again, this time deleting the cached directory, and also the - // package that caused the error in case our cache is stale - // (remote repository or one of its dependencices was force-pushed, - // replaced, etc) - err = os.RemoveAll(filepath.Join(d, "src", repo)) - if err != nil { - return fmt.Errorf("could not delete repo: %v", err) - } - - packageNames := reBadRepo.FindStringSubmatch(errStr) - if len(packageNames) >= 2 { - pkg := packageNames[1] - fp := filepath.Clean(filepath.Join(d, "src", pkg)) - if strings.HasPrefix(fp, filepath.Join(d, "src")) { - // if the path is prefixed with the path to our - // cached repos, then it's safe to delete it. - // These precautions are here so that someone can't - // craft a malicious package name with .. in it - // and cause us to delete our server's root directory. - log.Println("Cleaning out rebased dependency:", fp) - err = os.RemoveAll(fp) - if err != nil { - return err - } - } - } - return goGet(repo, errStr) - } - - return fmt.Errorf("could not run go get: %v", err) - } - return nil -} - func newChecksResp(repo string, forceRefresh bool) (checksResp, error) { if !forceRefresh { resp, err := getFromCache(repo) @@ -165,11 +90,13 @@ func newChecksResp(repo string, forceRefresh bool) (checksResp, error) { } // fetch the repo and grade it - err := goGet(repo, "") + repoRoot, err := download.Download(repo, "repos/src") if err != nil { return checksResp{}, fmt.Errorf("could not clone repo: %v", err) } + repo = repoRoot.Root + dir := dirName(repo) filenames, err := check.GoFiles(dir) if err != nil {