Use go mod download -json (#710)

* use go mod download -json

* annotate json, fix build

* add comment

* address review feedback

pkg/module/all_test.go

@@ -10,8 +10,9 @@ import (
 
 const (
 	// these values need to point to a real repository that has a tag
-	repoURI = "github.com/arschles/assert"
-	version = "v1.0.0"
+	// github.com/NYTimes/gizmo is a example of a path that needs to be encoded so we can cover that case as well
+	repoURI = "github.com/NYTimes/gizmo"
+	version = "v0.1.4"
 )
 
 type ModuleSuite struct {

pkg/module/disk_ref.go

@@ -1,110 +0,0 @@
-package module
-
-import (
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
-
-	"github.com/gomods/athens/pkg/errors"
-	"github.com/gomods/athens/pkg/paths"
-	"github.com/gomods/athens/pkg/storage"
-	"github.com/spf13/afero"
-)
-
-// diskRef is a Ref implementation for modules on disk. It is not safe to use concurrently.
-//
-// Do not create this struct directly. use newDiskRef
-type diskRef struct {
-	root    string
-	module  string
-	fs      afero.Fs
-	version string
-}
-
-type zipReadCloser struct {
-	zip io.ReadCloser
-	ref *diskRef
-}
-
-// Close closes the zip file handle and clears up disk space used by the underlying disk ref
-// It is the caller's responsibility to call this method to free up utilized disk space
-func (rc *zipReadCloser) Close() error {
-	rc.zip.Close()
-	return ClearFiles(rc.ref.fs, rc.ref.root)
-}
-
-func (rc *zipReadCloser) Read(p []byte) (n int, err error) {
-	return rc.zip.Read(p)
-}
-
-func newDiskRef(fs afero.Fs, root, module, version string) *diskRef {
-	return &diskRef{
-		fs:      fs,
-		root:    root,
-		module:  module,
-		version: version,
-	}
-}
-
-// ClearFiles deletes all data from the given fs at path root
-// This function must be called when zip is closed to cleanup the entire GOPATH created by the diskref
-func ClearFiles(fs afero.Fs, root string) error {
-	const op errors.Op = "module.ClearFiles"
-	// This is required because vgo ensures dependencies are read-only
-	// See https://github.com/golang/go/issues/24111 and
-	// https://go-review.googlesource.com/c/vgo/+/96978
-	walkFn := func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		return fs.Chmod(path, 0770)
-	}
-	err := afero.Walk(fs, root, walkFn)
-	if err != nil {
-		return errors.E(op, err)
-	}
-	err = fs.RemoveAll(root)
-	if err != nil {
-		return errors.E(op, err)
-	}
-	return nil
-}
-
-// read is the Ref interface implementation.
-func (d *diskRef) Read() (*storage.Version, error) {
-	const op errors.Op = "diskRef.Read"
-	var ver storage.Version
-
-	encodedModulePath, err := paths.EncodePath(d.module)
-	if err != nil {
-		return nil, errors.E(op, err, errors.KindBadRequest)
-	}
-	packagePath := getPackagePath(d.root, encodedModulePath)
-
-	infoFile := filepath.Join(packagePath, fmt.Sprintf("%s.info", d.version))
-	info, err := afero.ReadFile(d.fs, infoFile)
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	ver.Info = info
-
-	modFile := filepath.Join(packagePath, fmt.Sprintf("%s.mod", d.version))
-	mod, err := afero.ReadFile(d.fs, modFile)
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	ver.Mod = mod
-
-	sourceFile, err := d.fs.Open(filepath.Join(packagePath, fmt.Sprintf("%s.zip", d.version)))
-	if err != nil {
-		return nil, errors.E(op, err)
-	}
-	// note: don't close sourceFile here so that the caller can read directly from disk.
-	//
-	// if we close, then the caller will panic, and the alternative to make this work is
-	// that we read into memory and return an io.ReadCloser that reads out of memory
-	ver.Zip = &zipReadCloser{sourceFile, d}
-
-	return &ver, nil
-}

pkg/module/disk_ref_test.go

@@ -1,110 +0,0 @@
-package module
-
-import (
-	"fmt"
-	"io/ioutil"
-	"path/filepath"
-
-	"github.com/spf13/afero"
-)
-
-func (m *ModuleSuite) TestDiskRefReadAndClear() {
-	const (
-		root    = "testroot"
-		version = "v1.0.0"
-		info    = "testinfo"
-		mod     = "testmod"
-		zip     = "testzip"
-	)
-	r := m.Require()
-
-	packagePath := getPackagePath(root, mod)
-	// create a new disk ref using the filesystem
-	diskRef := newDiskRef(m.fs, root, mod, version)
-
-	// ensure that reading fails, because there are no files
-	ver, err := diskRef.Read()
-	r.Nil(ver)
-	r.NotNil(err)
-
-	// create all the files the disk ref expects
-	r.NoError(createAndWriteFile(m.fs, filepath.Join(packagePath, version+".info"), info))
-	r.NoError(createAndWriteFile(m.fs, filepath.Join(packagePath, version+".mod"), mod))
-	r.NoError(createAndWriteFile(m.fs, filepath.Join(packagePath, version+".zip"), zip))
-
-	// read from the disk ref - this time it should succeed
-	ver, err = diskRef.Read()
-	r.NoError(err)
-	r.Equal(info, string(ver.Info))
-	r.Equal(mod, string(ver.Mod))
-	zipBytes, err := ioutil.ReadAll(ver.Zip)
-	r.NoError(err)
-	r.Equal(zip, string(zipBytes))
-
-	// Validate that the root dir still exists
-	fInfo, err := m.fs.Stat(root)
-	r.NotNil(fInfo)
-	r.Nil(err)
-
-	// close the version's zip file (which also cleans up the underlying diskref's GOPATH) and expect it to fail again
-	r.NoError(ver.Zip.Close())
-	ver, err = diskRef.Read()
-	r.Nil(ver)
-	r.NotNil(err)
-
-	// The root dir should not exist after a clear
-	fInfo, err = m.fs.Stat(root)
-	r.Nil(fInfo)
-	r.NotNil(err)
-
-}
-
-func (m *ModuleSuite) TestDiskRefClearFail() {
-	root := "testroot"
-	r := m.Require()
-	// This should fail because we haven't created any files
-	err := ClearFiles(m.fs, root)
-	r.EqualError(err, "open testroot: file does not exist")
-}
-
-func (m *ModuleSuite) TestDiskRefClearSuccess() {
-	const (
-		root = "testroot"
-		mod  = "testmod"
-		file = "testfile"
-		info = "testinfo"
-	)
-	r := m.Require()
-
-	// Create a single file
-	packagePath := getPackagePath(root, mod)
-	filePath := filepath.Join(packagePath, file)
-	r.NoError(createAndWriteFile(m.fs, filePath, info))
-
-	// Validate the file exists
-	_, err := m.fs.Stat(filePath)
-	r.NoError(err)
-
-	// Now clear the files
-	err = ClearFiles(m.fs, root)
-	r.NoError(err)
-
-	// Validate the file has been deleted
-	_, err = m.fs.Stat(filePath)
-	expErr := fmt.Sprintf("open %s: file does not exist", filePath)
-	r.EqualError(err, expErr)
-}
-
-// creates filename with fs, writes data to the file, and closes the file,
-//
-// returns a non-nil error if anything went wrong. the file will be closed
-// regardless of what this function returns
-func createAndWriteFile(fs afero.Fs, filename, data string) error {
-	fileHandle, err := fs.Create(filename)
-	if err != nil {
-		return err
-	}
-	defer fileHandle.Close()
-	_, err = fileHandle.Write([]byte(data))
-	return err
-}

pkg/module/go_get_fetcher.go

@@ -3,6 +3,7 @@ package module
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
@@ -12,7 +13,6 @@ import (
 
 	"github.com/gomods/athens/pkg/errors"
 	"github.com/gomods/athens/pkg/observ"
-	"github.com/gomods/athens/pkg/paths"
 	"github.com/gomods/athens/pkg/storage"
 	"github.com/spf13/afero"
 )
@@ -22,6 +22,18 @@ type goGetFetcher struct {
 	goBinaryName string
 }
 
+type goModule struct {
+	Path     string `json:"path"`     // module path
+	Version  string `json:"version"`  // module version
+	Error    string `json:"error"`    // error loading module
+	Info     string `json:"info"`     // absolute path to cached .info file
+	GoMod    string `json:"goMod"`    // absolute path to cached .mod file
+	Zip      string `json:"zip"`      // absolute path to cached .zip file
+	Dir      string `json:"dir"`      // absolute path to cached source root directory
+	Sum      string `json:"sum"`      // checksum for path, version (as in go.sum)
+	GoModSum string `json:"goModSum"` // checksum for go.mod (as in go.sum)
+}
+
 // NewGoGetFetcher creates fetcher which uses go get tool to fetch modules
 func NewGoGetFetcher(goBinaryName string, fs afero.Fs) (Fetcher, error) {
 	const op errors.Op = "module.NewGoGetFetcher"
@@ -59,14 +71,36 @@ func (g *goGetFetcher) Fetch(ctx context.Context, mod, ver string) (*storage.Ver
 		return nil, errors.E(op, err)
 	}
 
-	err = getSources(g.goBinaryName, g.fs, goPathRoot, modPath, mod, ver)
+	m, err := downloadModule(g.goBinaryName, g.fs, goPathRoot, modPath, mod, ver)
 	if err != nil {
 		ClearFiles(g.fs, goPathRoot)
 		return nil, errors.E(op, err)
 	}
 
-	dr := newDiskRef(g.fs, goPathRoot, mod, ver)
-	return dr.Read()
+	var storageVer storage.Version
+	info, err := afero.ReadFile(g.fs, m.Info)
+	if err != nil {
+		return nil, errors.E(op, err)
+	}
+	storageVer.Info = info
+
+	gomod, err := afero.ReadFile(g.fs, m.GoMod)
+	if err != nil {
+		return nil, errors.E(op, err)
+	}
+	storageVer.Mod = gomod
+
+	zip, err := g.fs.Open(m.Zip)
+	if err != nil {
+		return nil, errors.E(op, err)
+	}
+	// note: don't close zip here so that the caller can read directly from disk.
+	//
+	// if we close, then the caller will panic, and the alternative to make this work is
+	// that we read into memory and return an io.ReadCloser that reads out of memory
+	storageVer.Zip = &zipReadCloser{zip, g.fs, goPathRoot}
+
+	return &storageVer, nil
 }
 
 // Dummy Hacky thing makes vgo not to complain
@@ -86,38 +120,39 @@ func Dummy(fs afero.Fs, repoRoot string) error {
 	return nil
 }
 
-// given a filesystem, gopath, repository root, module and version, runs 'vgo get'
+// given a filesystem, gopath, repository root, module and version, runs 'go mod download -json'
 // on module@version from the repoRoot with GOPATH=gopath, and returns a non-nil error if anything went wrong.
-func getSources(goBinaryName string, fs afero.Fs, gopath, repoRoot, module, version string) error {
-	const op errors.Op = "module.getSources"
+func downloadModule(goBinaryName string, fs afero.Fs, gopath, repoRoot, module, version string) (goModule, error) {
+	const op errors.Op = "module.downloadModule"
 	uri := strings.TrimSuffix(module, "/")
 	fullURI := fmt.Sprintf("%s@%s", uri, version)
 
-	cmd := exec.Command(goBinaryName, "mod", "download", fullURI)
+	cmd := exec.Command(goBinaryName, "mod", "download", "-json", fullURI)
 	cmd.Env = PrepareEnv(gopath)
 	cmd.Dir = repoRoot
-	o, err := cmd.CombinedOutput()
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+	err := cmd.Run()
 	if err != nil {
-		errMsg := fmt.Sprintf("%v : %s", err, o)
+		err = fmt.Errorf("%v: %s", err, stderr)
 		// github quota exceeded
-		if isLimitHit(o) {
-			return errors.E(op, errMsg, errors.KindRateLimit)
+		if isLimitHit(err.Error()) {
+			return goModule{}, errors.E(op, err, errors.KindRateLimit)
 		}
-		// another error in the output
-		return errors.E(op, errMsg)
-	}
-	// make sure the expected files exist
-	encmod, err := paths.EncodePath(module)
-	if err != nil {
-		return errors.E(op, err)
-	}
-	packagePath := getPackagePath(gopath, encmod)
-	err = checkFiles(fs, packagePath, version)
-	if err != nil {
-		return errors.E(op, err)
+		return goModule{}, errors.E(op, err)
 	}
 
-	return nil
+	var m goModule
+	if err = json.NewDecoder(stdout).Decode(&m); err != nil {
+		return goModule{}, errors.E(op, err)
+	}
+	if m.Error != "" {
+		return goModule{}, errors.E(op, m.Error)
+	}
+
+	return m, nil
 }
 
 // PrepareEnv will return all the appropriate
@@ -146,25 +181,8 @@ func PrepareEnv(gopath string) []string {
 	return cmdEnv
 }
 
-func checkFiles(fs afero.Fs, path, version string) error {
-	const op errors.Op = "module.checkFiles"
-	if _, err := fs.Stat(filepath.Join(path, version+".mod")); err != nil {
-		return errors.E(op, fmt.Sprintf("%s.mod not found in %s", version, path))
-	}
-
-	if _, err := fs.Stat(filepath.Join(path, version+".zip")); err != nil {
-		return errors.E(op, fmt.Sprintf("%s.mod not found in %s", version, path))
-	}
-
-	if _, err := fs.Stat(filepath.Join(path, version+".info")); err != nil {
-		return errors.E(op, fmt.Sprintf("%s.mod not found in %s", version, path))
-	}
-
-	return nil
-}
-
-func isLimitHit(o []byte) bool {
-	return bytes.Contains(o, []byte("403 response from api.github.com"))
+func isLimitHit(o string) bool {
+	return strings.Contains(o, "403 response from api.github.com")
 }
 
 // getRepoDirName takes a raw repository URI and a version and creates a directory name that the
@@ -174,11 +192,6 @@ func getRepoDirName(repoURI, version string) string {
 	return fmt.Sprintf("%s-%s", escapedURI, version)
 }
 
-// getPackagePath returns the path to the module cache given the gopath and module name
-func getPackagePath(gopath, module string) string {
-	return filepath.Join(gopath, "pkg", "mod", "cache", "download", module, "@v")
-}
-
 func validGoBinary(name string) error {
 	const op errors.Op = "module.validGoBinary"
 	err := exec.Command(name).Run()

pkg/module/go_get_fetcher_test.go

@@ -2,11 +2,9 @@ package module
 
 import (
 	"context"
-	"fmt"
 	"io/ioutil"
-	"log"
+	"runtime"
 
-	"github.com/gobuffalo/envy"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 )
@@ -25,7 +23,11 @@ func (s *ModuleSuite) TestGoGetFetcherError() {
 	fetcher, err := NewGoGetFetcher("invalidpath", afero.NewOsFs())
 
 	assert.Nil(s.T(), fetcher)
-	assert.EqualError(s.T(), err, "exec: \"invalidpath\": executable file not found in $PATH")
+	if runtime.GOOS == "windows" {
+		assert.EqualError(s.T(), err, "exec: \"invalidpath\": executable file not found in %PATH%")
+	} else {
+		assert.EqualError(s.T(), err, "exec: \"invalidpath\": executable file not found in $PATH")
+	}
 }
 
 func (s *ModuleSuite) TestGoGetFetcherFetch() {
@@ -46,30 +48,6 @@ func (s *ModuleSuite) TestGoGetFetcherFetch() {
 	r.NoError(err)
 	r.True(len(zipBytes) > 0)
 
-	// close the version's zip file (which also cleans up the underlying diskref's GOPATH) and expect it to fail again
+	// close the version's zip file (which also cleans up the underlying GOPATH) and expect it to fail again
 	r.NoError(ver.Zip.Close())
 }
-
-func ExampleFetcher() {
-	repoURI := "github.com/arschles/assert"
-	version := "v1.0.0"
-	goBinaryName := envy.Get("GO_BINARY_PATH", "go")
-	fetcher, err := NewGoGetFetcher(goBinaryName, afero.NewOsFs())
-	if err != nil {
-		log.Fatal(err)
-	}
-	versionData, err := fetcher.Fetch(ctx, repoURI, version)
-	// handle errors if any
-	if err != nil {
-		return
-	}
-	// Close the handle to versionData.Zip once done
-	// This will also handle cleanup so it's important to call Close
-	defer versionData.Zip.Close()
-	if err != nil {
-		return
-	}
-	// Do something with versionData
-	fmt.Println(string(versionData.Mod))
-	// Output: module github.com/arschles/assert
-}

pkg/module/zip_read_closer.go

@@ -0,0 +1,50 @@
+package module
+
+import (
+	"io"
+	"os"
+
+	"github.com/gomods/athens/pkg/errors"
+	"github.com/spf13/afero"
+)
+
+type zipReadCloser struct {
+	zip    io.ReadCloser
+	fs     afero.Fs
+	goPath string
+}
+
+// Close closes the zip file handle and clears up disk space used by the underlying disk ref
+// It is the caller's responsibility to call this method to free up utilized disk space
+func (rc *zipReadCloser) Close() error {
+	rc.zip.Close()
+	return ClearFiles(rc.fs, rc.goPath)
+}
+
+func (rc *zipReadCloser) Read(p []byte) (n int, err error) {
+	return rc.zip.Read(p)
+}
+
+// ClearFiles deletes all data from the given fs at path root
+// This function must be called when zip is closed to cleanup the entire GOPATH created by the diskref
+func ClearFiles(fs afero.Fs, root string) error {
+	const op errors.Op = "module.ClearFiles"
+	// This is required because vgo ensures dependencies are read-only
+	// See https://github.com/golang/go/issues/24111 and
+	// https://go-review.googlesource.com/c/vgo/+/96978
+	walkFn := func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		return fs.Chmod(path, 0770)
+	}
+	err := afero.Walk(fs, root, walkFn)
+	if err != nil {
+		return errors.E(op, err)
+	}
+	err = fs.RemoveAll(root)
+	if err != nil {
+		return errors.E(op, err)
+	}
+	return nil
+}

pkg/module/zip_read_closer_test.go

@@ -0,0 +1,56 @@
+package module
+
+import (
+	"path/filepath"
+
+	"github.com/spf13/afero"
+)
+
+func (m *ModuleSuite) TestZipReadCloser() {
+	const (
+		root    = "testroot"
+		version = "v1.0.0"
+		info    = "testinfo"
+		mod     = "testmod"
+		zip     = "testzip"
+	)
+	r := m.Require()
+
+	fs := afero.NewMemMapFs()
+	gopath, err := afero.TempDir(fs, "", "athens-test")
+	r.NoError(err)
+	packagePath := filepath.Join(gopath, "pkg", "mod", "cache", "download", mod, "@v")
+	// create all the files the disk ref expects
+	r.NoError(createAndWriteFile(fs, filepath.Join(packagePath, version+".info"), info))
+	r.NoError(createAndWriteFile(fs, filepath.Join(packagePath, version+".mod"), mod))
+	r.NoError(createAndWriteFile(fs, filepath.Join(packagePath, version+".zip"), zip))
+
+	ziprc, err := fs.Open(filepath.Join(packagePath, version+".zip"))
+	r.NoError(err)
+	cl := &zipReadCloser{fs: fs, goPath: gopath, zip: ziprc}
+
+	fInfo, err := fs.Stat(gopath)
+	r.NotNil(fInfo)
+	r.Nil(err)
+
+	r.NoError(cl.Close())
+
+	// The root dir should not exist after a clear
+	fInfo, err = fs.Stat(gopath)
+	r.Nil(fInfo)
+	r.NotNil(err)
+}
+
+// creates filename with fs, writes data to the file, and closes the file,
+//
+// returns a non-nil error if anything went wrong. the file will be closed
+// regardless of what this function returns
+func createAndWriteFile(fs afero.Fs, filename, data string) error {
+	fileHandle, err := fs.Create(filename)
+	if err != nil {
+		return err
+	}
+	defer fileHandle.Close()
+	_, err = fileHandle.Write([]byte(data))
+	return err
+}

pkg/paths/encode.go

@@ -1,38 +0,0 @@
-package paths
-
-import (
-	"unicode/utf8"
-
-	"github.com/gomods/athens/pkg/errors"
-)
-
-// EncodePath is ripped from cmd/go -- it replaces upper case
-// letters with bang+lowercase.
-func EncodePath(s string) (encoding string, err error) {
-	const op errors.Op = "paths.EncodePath"
-	haveUpper := false
-	for _, r := range s {
-		if r == '!' || r >= utf8.RuneSelf {
-			// This should be disallowed by CheckPath, but diagnose anyway.
-			// The correctness of the encoding loop below depends on it.
-			return "", errors.E(op, "internal error: inconsistency in EncodePath")
-		}
-		if 'A' <= r && r <= 'Z' {
-			haveUpper = true
-		}
-	}
-
-	if !haveUpper {
-		return s, nil
-	}
-
-	var buf []byte
-	for _, r := range s {
-		if 'A' <= r && r <= 'Z' {
-			buf = append(buf, '!', byte(r+'a'-'A'))
-		} else {
-			buf = append(buf, byte(r))
-		}
-	}
-	return string(buf), nil
-}

pkg/paths/encode_decode_test.go

@@ -1,46 +0,0 @@
-package paths
-
-import "testing"
-
-var testCases = []struct {
-	name, decoded, encoded string
-}{
-	{
-		"happy path",
-		"github.com/a/b",
-		"github.com/a/b",
-	},
-	{
-		"capital letters",
-		"github.com/NYTimes/gizmo",
-		"github.com/!n!y!times/gizmo",
-	},
-}
-
-func TestEncodeDecode(t *testing.T) {
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			testEncodeDecode(t, tc.decoded, tc.encoded)
-		})
-	}
-}
-
-func testEncodeDecode(t *testing.T, exDec, exEnc string) {
-	t.Helper()
-	enc, err := EncodePath(exDec)
-	if err != nil {
-		t.Fatal(err)
-	}
-	dec, err := DecodePath(exEnc)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if exEnc != enc {
-		t.Fatalf("expected %v and %v to be equal", exEnc, enc)
-	}
-
-	if exDec != dec {
-		t.Fatalf("expected %v and %v to be equal", exDec, dec)
-	}
-}

pkg/paths/go_get.go

@@ -1,11 +0,0 @@
-package paths
-
-import (
-	"net/url"
-	"strings"
-)
-
-// IsGoGet returns true if u has ?go-get=1 in its query string
-func IsGoGet(u *url.URL) bool {
-	return strings.Contains(u.Query().Get("go-get"), "1")
-}