Commit 680ed10c authored by Bryan C. Mills's avatar Bryan C. Mills

cmd/go/internal/modfetch/codehost: remove invariantly-empty return value from Repo.ReadZip

Previously, codehost.Repo.ReadZip returned an 'actualSubdir' value
that was the empty string in all current implementations.

Updates #26092

Change-Id: I6708dd0f13ba88bcf1a1fb405e9d818fd6f9197e
Reviewed-on: https://go-review.googlesource.com/c/go/+/203277
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
parent c357b363
...@@ -73,11 +73,10 @@ type Repo interface { ...@@ -73,11 +73,10 @@ type Repo interface {
// ReadZip downloads a zip file for the subdir subdirectory // ReadZip downloads a zip file for the subdir subdirectory
// of the given revision to a new file in a given temporary directory. // of the given revision to a new file in a given temporary directory.
// It should refuse to read more than maxSize bytes. // It should refuse to read more than maxSize bytes.
// It returns a ReadCloser for a streamed copy of the zip file, // It returns a ReadCloser for a streamed copy of the zip file.
// along with the actual subdirectory (possibly shorter than subdir) // All files in the zip file are expected to be
// contained in the zip file. All files in the zip file are expected to be
// nested in a single top-level directory, whose name is not specified. // nested in a single top-level directory, whose name is not specified.
ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, actualSubdir string, err error) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, err error)
// RecentTag returns the most recent tag on rev or one of its predecessors // RecentTag returns the most recent tag on rev or one of its predecessors
// with the given prefix and major version. // with the given prefix and major version.
......
...@@ -795,7 +795,7 @@ func (r *gitRepo) DescendsFrom(rev, tag string) (bool, error) { ...@@ -795,7 +795,7 @@ func (r *gitRepo) DescendsFrom(rev, tag string) (bool, error) {
return false, err return false, err
} }
func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, actualSubdir string, err error) { func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, err error) {
// TODO: Use maxSize or drop it. // TODO: Use maxSize or drop it.
args := []string{} args := []string{}
if subdir != "" { if subdir != "" {
...@@ -803,17 +803,17 @@ func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, ...@@ -803,17 +803,17 @@ func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser,
} }
info, err := r.Stat(rev) // download rev into local git repo info, err := r.Stat(rev) // download rev into local git repo
if err != nil { if err != nil {
return nil, "", err return nil, err
} }
unlock, err := r.mu.Lock() unlock, err := r.mu.Lock()
if err != nil { if err != nil {
return nil, "", err return nil, err
} }
defer unlock() defer unlock()
if err := ensureGitAttributes(r.dir); err != nil { if err := ensureGitAttributes(r.dir); err != nil {
return nil, "", err return nil, err
} }
// Incredibly, git produces different archives depending on whether // Incredibly, git produces different archives depending on whether
...@@ -824,12 +824,12 @@ func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, ...@@ -824,12 +824,12 @@ func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser,
archive, err := Run(r.dir, "git", "-c", "core.autocrlf=input", "-c", "core.eol=lf", "archive", "--format=zip", "--prefix=prefix/", info.Name, args) archive, err := Run(r.dir, "git", "-c", "core.autocrlf=input", "-c", "core.eol=lf", "archive", "--format=zip", "--prefix=prefix/", info.Name, args)
if err != nil { if err != nil {
if bytes.Contains(err.(*RunError).Stderr, []byte("did not match any files")) { if bytes.Contains(err.(*RunError).Stderr, []byte("did not match any files")) {
return nil, "", os.ErrNotExist return nil, os.ErrNotExist
} }
return nil, "", err return nil, err
} }
return ioutil.NopCloser(bytes.NewReader(archive)), "", nil return ioutil.NopCloser(bytes.NewReader(archive)), nil
} }
// ensureGitAttributes makes sure export-subst and export-ignore features are // ensureGitAttributes makes sure export-subst and export-ignore features are
......
...@@ -246,12 +246,11 @@ func TestReadFile(t *testing.T) { ...@@ -246,12 +246,11 @@ func TestReadFile(t *testing.T) {
} }
var readZipTests = []struct { var readZipTests = []struct {
repo string repo string
rev string rev string
subdir string subdir string
actualSubdir string err string
err string files map[string]uint64
files map[string]uint64
}{ }{
{ {
repo: gitrepo1, repo: gitrepo1,
...@@ -408,7 +407,7 @@ func TestReadZip(t *testing.T) { ...@@ -408,7 +407,7 @@ func TestReadZip(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
rc, actualSubdir, err := r.ReadZip(tt.rev, tt.subdir, 100000) rc, err := r.ReadZip(tt.rev, tt.subdir, 100000)
if err != nil { if err != nil {
if tt.err == "" { if tt.err == "" {
t.Fatalf("ReadZip: unexpected error %v", err) t.Fatalf("ReadZip: unexpected error %v", err)
...@@ -425,9 +424,6 @@ func TestReadZip(t *testing.T) { ...@@ -425,9 +424,6 @@ func TestReadZip(t *testing.T) {
if tt.err != "" { if tt.err != "" {
t.Fatalf("ReadZip: no error, wanted %v", tt.err) t.Fatalf("ReadZip: no error, wanted %v", tt.err)
} }
if actualSubdir != tt.actualSubdir {
t.Fatalf("ReadZip: actualSubdir = %q, want %q", actualSubdir, tt.actualSubdir)
}
zipdata, err := ioutil.ReadAll(rc) zipdata, err := ioutil.ReadAll(rc)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
......
...@@ -109,7 +109,7 @@ func main() { ...@@ -109,7 +109,7 @@ func main() {
if subdir == "-" { if subdir == "-" {
subdir = "" subdir = ""
} }
rc, _, err := repo.ReadZip(f[1], subdir, 10<<20) rc, err := repo.ReadZip(f[1], subdir, 10<<20)
if err != nil { if err != nil {
fmt.Fprintf(os.Stderr, "?%s\n", err) fmt.Fprintf(os.Stderr, "?%s\n", err)
continue continue
......
...@@ -417,14 +417,14 @@ func (r *vcsRepo) DescendsFrom(rev, tag string) (bool, error) { ...@@ -417,14 +417,14 @@ func (r *vcsRepo) DescendsFrom(rev, tag string) (bool, error) {
return false, vcsErrorf("DescendsFrom not implemented") return false, vcsErrorf("DescendsFrom not implemented")
} }
func (r *vcsRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, actualSubdir string, err error) { func (r *vcsRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, err error) {
if r.cmd.readZip == nil { if r.cmd.readZip == nil {
return nil, "", vcsErrorf("ReadZip not implemented for %s", r.cmd.vcs) return nil, vcsErrorf("ReadZip not implemented for %s", r.cmd.vcs)
} }
unlock, err := r.mu.Lock() unlock, err := r.mu.Lock()
if err != nil { if err != nil {
return nil, "", err return nil, err
} }
defer unlock() defer unlock()
...@@ -433,7 +433,7 @@ func (r *vcsRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, ...@@ -433,7 +433,7 @@ func (r *vcsRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser,
} }
f, err := ioutil.TempFile("", "go-readzip-*.zip") f, err := ioutil.TempFile("", "go-readzip-*.zip")
if err != nil { if err != nil {
return nil, "", err return nil, err
} }
if r.cmd.vcs == "fossil" { if r.cmd.vcs == "fossil" {
// If you run // If you run
...@@ -454,9 +454,9 @@ func (r *vcsRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser, ...@@ -454,9 +454,9 @@ func (r *vcsRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser,
if err != nil { if err != nil {
f.Close() f.Close()
os.Remove(f.Name()) os.Remove(f.Name())
return nil, "", err return nil, err
} }
return &deleteCloser{f}, "", nil return &deleteCloser{f}, nil
} }
// deleteCloser is a file that gets deleted on Close. // deleteCloser is a file that gets deleted on Close.
......
...@@ -780,19 +780,16 @@ func (r *codeRepo) Zip(dst io.Writer, version string) error { ...@@ -780,19 +780,16 @@ func (r *codeRepo) Zip(dst io.Writer, version string) error {
} }
} }
rev, dir, _, err := r.findDir(version) rev, subdir, _, err := r.findDir(version)
if err != nil { if err != nil {
return err return err
} }
dl, actualDir, err := r.code.ReadZip(rev, dir, codehost.MaxZipFile) dl, err := r.code.ReadZip(rev, subdir, codehost.MaxZipFile)
if err != nil { if err != nil {
return err return err
} }
defer dl.Close() defer dl.Close()
if actualDir != "" && !hasPathPrefix(dir, actualDir) { subdir = strings.Trim(subdir, "/")
return fmt.Errorf("internal error: downloading %v %v: dir=%q but actualDir=%q", r.modPath, rev, dir, actualDir)
}
subdir := strings.Trim(strings.TrimPrefix(dir, actualDir), "/")
// Spool to local file. // Spool to local file.
f, err := ioutil.TempFile("", "go-codehost-") f, err := ioutil.TempFile("", "go-codehost-")
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment