zip: fix LICENSE file handling to match modfetch 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>
diff --git a/zip/zip.go b/zip/zip.go index 7b48a2a..27b6f4b 100644 --- a/zip/zip.go +++ b/zip/zip.go
@@ -56,6 +56,7 @@ "path" "path/filepath" "strings" + "time" "unicode" "unicode/utf8" @@ -653,6 +654,7 @@ return nil, err } + haveLICENSE := false var fs []File for _, zf := range zipReader.File { if !strings.HasPrefix(zf.Name, subdir) || strings.HasSuffix(zf.Name, "/") { @@ -669,6 +671,23 @@ 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 @@ -710,6 +729,26 @@ 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".
diff --git a/zip/zip_test.go b/zip/zip_test.go index 173cc65..8814ff8 100644 --- a/zip/zip_test.go +++ b/zip/zip_test.go
@@ -1466,6 +1466,8 @@ module example.com/foo/bar go 1.12 +-- LICENSE -- +root license -- a.go -- package a @@ -1482,6 +1484,20 @@ 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/`))) @@ -1494,31 +1510,60 @@ 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 { @@ -1529,10 +1574,28 @@ 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 } @@ -1549,6 +1612,11 @@ 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) + } + } }) } }