Skip to content

Commit

Permalink
zip: fix LICENSE file handling to match modfetch
Browse files Browse the repository at this point in the history
go modfetch includes the LICENSE file from the repo root when fetching
a submodule if the submodule does not contain a LICENSE.

This updates CreateFromVCS to match that behavior.

Fixes golang/go#60442

Change-Id: I5f9edb775bb330325783e1c30d7d16e436bda74c
Reviewed-on: https://go-review.googlesource.com/c/mod/+/498935
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
  • Loading branch information
liggitt authored and gopherbot committed Jun 15, 2023
1 parent 62c7e57 commit bfed713
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 8 deletions.
39 changes: 39 additions & 0 deletions zip/zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import (
"path"
"path/filepath"
"strings"
"time"
"unicode"
"unicode/utf8"

Expand Down Expand Up @@ -653,6 +654,7 @@ func filesInGitRepo(dir, rev, subdir string) ([]File, error) {
return nil, err
}

haveLICENSE := false
var fs []File
for _, zf := range zipReader.File {
if !strings.HasPrefix(zf.Name, subdir) || strings.HasSuffix(zf.Name, "/") {
Expand All @@ -669,6 +671,23 @@ func filesInGitRepo(dir, rev, subdir string) ([]File, error) {
name: n,
f: zf,
})
if n == "LICENSE" {
haveLICENSE = true
}
}

if !haveLICENSE && subdir != "" {
// Note: this method of extracting the license from the root copied from
// https://go.googlesource.com/go/+/refs/tags/go1.20.4/src/cmd/go/internal/modfetch/coderepo.go#1118
// https://go.googlesource.com/go/+/refs/tags/go1.20.4/src/cmd/go/internal/modfetch/codehost/git.go#657
cmd := exec.Command("git", "cat-file", "blob", rev+":LICENSE")
cmd.Dir = dir
cmd.Env = append(os.Environ(), "PWD="+dir)
stdout := bytes.Buffer{}
cmd.Stdout = &stdout
if err := cmd.Run(); err == nil {
fs = append(fs, dataFile{name: "LICENSE", data: stdout.Bytes()})
}
}

return fs, nil
Expand Down Expand Up @@ -710,6 +729,26 @@ func (f zipFile) Path() string { return f.name }
func (f zipFile) Lstat() (os.FileInfo, error) { return f.f.FileInfo(), nil }
func (f zipFile) Open() (io.ReadCloser, error) { return f.f.Open() }

type dataFile struct {
name string
data []byte
}

func (f dataFile) Path() string { return f.name }
func (f dataFile) Lstat() (os.FileInfo, error) { return dataFileInfo{f}, nil }
func (f dataFile) Open() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(f.data)), nil }

type dataFileInfo struct {
f dataFile
}

func (fi dataFileInfo) Name() string { return path.Base(fi.f.name) }
func (fi dataFileInfo) Size() int64 { return int64(len(fi.f.data)) }
func (fi dataFileInfo) Mode() os.FileMode { return 0644 }
func (fi dataFileInfo) ModTime() time.Time { return time.Time{} }
func (fi dataFileInfo) IsDir() bool { return false }
func (fi dataFileInfo) Sys() interface{} { return nil }

// isVendoredPackage attempts to report whether the given filename is contained
// in a package whose import path contains (but does not end with) the component
// "vendor".
Expand Down
84 changes: 76 additions & 8 deletions zip/zip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,8 @@ func TestCreateFromVCS_basic(t *testing.T) {
module example.com/foo/bar
go 1.12
-- LICENSE --
root license
-- a.go --
package a
Expand All @@ -1482,6 +1484,20 @@ var C = 5
package c
var D = 5
-- e/LICENSE --
e license
-- e/e.go --
package e
var E = 5
-- f/go.mod --
module example.com/foo/bar/f
go 1.12
-- f/f.go --
package f
var F = 5
-- .gitignore --
b.go
c/`)))
Expand All @@ -1494,31 +1510,60 @@ c/`)))

for _, tc := range []struct {
desc string
version module.Version
subdir string
wantFiles []string
wantData map[string]string
}{
{
desc: "from root",
version: module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"},
subdir: "",
wantFiles: []string{"go.mod", "a.go", "d/d.go", ".gitignore"},
wantFiles: []string{"go.mod", "LICENSE", "a.go", "d/d.go", "e/LICENSE", "e/e.go", ".gitignore"},
wantData: map[string]string{"LICENSE": "root license\n"},
},
{
desc: "from subdir",
subdir: "d/",
desc: "from subdir",
version: module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"},
subdir: "d/",
// Note: File paths are zipped as if the subdir were the root. ie d.go instead of d/d.go.
wantFiles: []string{"d.go"},
// subdirs without a license hoist the license from the root
wantFiles: []string{"d.go", "LICENSE"},
wantData: map[string]string{"LICENSE": "root license\n"},
},
{
desc: "from subdir with license",
version: module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"},
subdir: "e/",
// Note: File paths are zipped as if the subdir were the root. ie e.go instead of e/e.go.
// subdirs with a license use their own
wantFiles: []string{"LICENSE", "e.go"},
wantData: map[string]string{"LICENSE": "e license\n"},
},
{
desc: "from submodule subdir",
version: module.Version{Path: "example.com/foo/bar/f", Version: "v0.0.1"},
subdir: "f/",
// Note: File paths are zipped as if the subdir were the root. ie f.go instead of f/f.go.
// subdirs without a license hoist the license from the root
wantFiles: []string{"go.mod", "f.go", "LICENSE"},
wantData: map[string]string{"LICENSE": "root license\n"},
},
} {
t.Run(tc.desc, func(t *testing.T) {
// Create zip from the directory.
tmpZip := &bytes.Buffer{}

m := module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"}

if err := modzip.CreateFromVCS(tmpZip, m, tmpDir, "HEAD", tc.subdir); err != nil {
if err := modzip.CreateFromVCS(tmpZip, tc.version, tmpDir, "HEAD", tc.subdir); err != nil {
t.Fatal(err)
}

wantData := map[string]string{}
for f, data := range tc.wantData {
p := path.Join(tc.version.String(), f)
wantData[p] = data
}

readerAt := bytes.NewReader(tmpZip.Bytes())
r, err := zip.NewReader(readerAt, int64(tmpZip.Len()))
if err != nil {
Expand All @@ -1529,10 +1574,28 @@ c/`)))
for _, f := range r.File {
gotMap[f.Name] = true
gotFiles = append(gotFiles, f.Name)

if want, ok := wantData[f.Name]; ok {
r, err := f.Open()
if err != nil {
t.Errorf("CreatedFromVCS: error opening %s: %v", f.Name, err)
continue
}
defer r.Close()
got, err := io.ReadAll(r)
if err != nil {
t.Errorf("CreatedFromVCS: error reading %s: %v", f.Name, err)
continue
}
if want != string(got) {
t.Errorf("CreatedFromVCS: zipped file %s contains %s, expected %s", f.Name, string(got), want)
continue
}
}
}
wantMap := map[string]bool{}
for _, f := range tc.wantFiles {
p := path.Join("example.com", "foo", "bar@v0.0.1", f)
p := path.Join(tc.version.String(), f)
wantMap[p] = true
}

Expand All @@ -1549,6 +1612,11 @@ c/`)))
t.Errorf("CreatedFromVCS: zipped file doesn't contain %s, but expected it to. all files: %v", f, gotFiles)
}
}
for f := range wantData {
if !gotMap[f] {
t.Errorf("CreatedFromVCS: zipped file doesn't contain %s, but expected it to. all files: %v", f, gotFiles)
}
}
})
}
}
Expand Down

0 comments on commit bfed713

Please sign in to comment.