Skip to content

Commit bfed713

Browse files
liggittgopherbot
authored andcommitted
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 <[email protected]> Reviewed-by: Bryan Mills <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]> Run-TryBot: Bryan Mills <[email protected]> Auto-Submit: Bryan Mills <[email protected]>
1 parent 62c7e57 commit bfed713

File tree

2 files changed

+115
-8
lines changed

2 files changed

+115
-8
lines changed

zip/zip.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import (
5656
"path"
5757
"path/filepath"
5858
"strings"
59+
"time"
5960
"unicode"
6061
"unicode/utf8"
6162

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

657+
haveLICENSE := false
656658
var fs []File
657659
for _, zf := range zipReader.File {
658660
if !strings.HasPrefix(zf.Name, subdir) || strings.HasSuffix(zf.Name, "/") {
@@ -669,6 +671,23 @@ func filesInGitRepo(dir, rev, subdir string) ([]File, error) {
669671
name: n,
670672
f: zf,
671673
})
674+
if n == "LICENSE" {
675+
haveLICENSE = true
676+
}
677+
}
678+
679+
if !haveLICENSE && subdir != "" {
680+
// Note: this method of extracting the license from the root copied from
681+
// https://go.googlesource.com/go/+/refs/tags/go1.20.4/src/cmd/go/internal/modfetch/coderepo.go#1118
682+
// https://go.googlesource.com/go/+/refs/tags/go1.20.4/src/cmd/go/internal/modfetch/codehost/git.go#657
683+
cmd := exec.Command("git", "cat-file", "blob", rev+":LICENSE")
684+
cmd.Dir = dir
685+
cmd.Env = append(os.Environ(), "PWD="+dir)
686+
stdout := bytes.Buffer{}
687+
cmd.Stdout = &stdout
688+
if err := cmd.Run(); err == nil {
689+
fs = append(fs, dataFile{name: "LICENSE", data: stdout.Bytes()})
690+
}
672691
}
673692

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

732+
type dataFile struct {
733+
name string
734+
data []byte
735+
}
736+
737+
func (f dataFile) Path() string { return f.name }
738+
func (f dataFile) Lstat() (os.FileInfo, error) { return dataFileInfo{f}, nil }
739+
func (f dataFile) Open() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(f.data)), nil }
740+
741+
type dataFileInfo struct {
742+
f dataFile
743+
}
744+
745+
func (fi dataFileInfo) Name() string { return path.Base(fi.f.name) }
746+
func (fi dataFileInfo) Size() int64 { return int64(len(fi.f.data)) }
747+
func (fi dataFileInfo) Mode() os.FileMode { return 0644 }
748+
func (fi dataFileInfo) ModTime() time.Time { return time.Time{} }
749+
func (fi dataFileInfo) IsDir() bool { return false }
750+
func (fi dataFileInfo) Sys() interface{} { return nil }
751+
713752
// isVendoredPackage attempts to report whether the given filename is contained
714753
// in a package whose import path contains (but does not end with) the component
715754
// "vendor".

zip/zip_test.go

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,6 +1466,8 @@ func TestCreateFromVCS_basic(t *testing.T) {
14661466
module example.com/foo/bar
14671467
14681468
go 1.12
1469+
-- LICENSE --
1470+
root license
14691471
-- a.go --
14701472
package a
14711473
@@ -1482,6 +1484,20 @@ var C = 5
14821484
package c
14831485
14841486
var D = 5
1487+
-- e/LICENSE --
1488+
e license
1489+
-- e/e.go --
1490+
package e
1491+
1492+
var E = 5
1493+
-- f/go.mod --
1494+
module example.com/foo/bar/f
1495+
1496+
go 1.12
1497+
-- f/f.go --
1498+
package f
1499+
1500+
var F = 5
14851501
-- .gitignore --
14861502
b.go
14871503
c/`)))
@@ -1494,31 +1510,60 @@ c/`)))
14941510

14951511
for _, tc := range []struct {
14961512
desc string
1513+
version module.Version
14971514
subdir string
14981515
wantFiles []string
1516+
wantData map[string]string
14991517
}{
15001518
{
15011519
desc: "from root",
1520+
version: module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"},
15021521
subdir: "",
1503-
wantFiles: []string{"go.mod", "a.go", "d/d.go", ".gitignore"},
1522+
wantFiles: []string{"go.mod", "LICENSE", "a.go", "d/d.go", "e/LICENSE", "e/e.go", ".gitignore"},
1523+
wantData: map[string]string{"LICENSE": "root license\n"},
15041524
},
15051525
{
1506-
desc: "from subdir",
1507-
subdir: "d/",
1526+
desc: "from subdir",
1527+
version: module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"},
1528+
subdir: "d/",
15081529
// Note: File paths are zipped as if the subdir were the root. ie d.go instead of d/d.go.
1509-
wantFiles: []string{"d.go"},
1530+
// subdirs without a license hoist the license from the root
1531+
wantFiles: []string{"d.go", "LICENSE"},
1532+
wantData: map[string]string{"LICENSE": "root license\n"},
1533+
},
1534+
{
1535+
desc: "from subdir with license",
1536+
version: module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"},
1537+
subdir: "e/",
1538+
// Note: File paths are zipped as if the subdir were the root. ie e.go instead of e/e.go.
1539+
// subdirs with a license use their own
1540+
wantFiles: []string{"LICENSE", "e.go"},
1541+
wantData: map[string]string{"LICENSE": "e license\n"},
1542+
},
1543+
{
1544+
desc: "from submodule subdir",
1545+
version: module.Version{Path: "example.com/foo/bar/f", Version: "v0.0.1"},
1546+
subdir: "f/",
1547+
// Note: File paths are zipped as if the subdir were the root. ie f.go instead of f/f.go.
1548+
// subdirs without a license hoist the license from the root
1549+
wantFiles: []string{"go.mod", "f.go", "LICENSE"},
1550+
wantData: map[string]string{"LICENSE": "root license\n"},
15101551
},
15111552
} {
15121553
t.Run(tc.desc, func(t *testing.T) {
15131554
// Create zip from the directory.
15141555
tmpZip := &bytes.Buffer{}
15151556

1516-
m := module.Version{Path: "example.com/foo/bar", Version: "v0.0.1"}
1517-
1518-
if err := modzip.CreateFromVCS(tmpZip, m, tmpDir, "HEAD", tc.subdir); err != nil {
1557+
if err := modzip.CreateFromVCS(tmpZip, tc.version, tmpDir, "HEAD", tc.subdir); err != nil {
15191558
t.Fatal(err)
15201559
}
15211560

1561+
wantData := map[string]string{}
1562+
for f, data := range tc.wantData {
1563+
p := path.Join(tc.version.String(), f)
1564+
wantData[p] = data
1565+
}
1566+
15221567
readerAt := bytes.NewReader(tmpZip.Bytes())
15231568
r, err := zip.NewReader(readerAt, int64(tmpZip.Len()))
15241569
if err != nil {
@@ -1529,10 +1574,28 @@ c/`)))
15291574
for _, f := range r.File {
15301575
gotMap[f.Name] = true
15311576
gotFiles = append(gotFiles, f.Name)
1577+
1578+
if want, ok := wantData[f.Name]; ok {
1579+
r, err := f.Open()
1580+
if err != nil {
1581+
t.Errorf("CreatedFromVCS: error opening %s: %v", f.Name, err)
1582+
continue
1583+
}
1584+
defer r.Close()
1585+
got, err := io.ReadAll(r)
1586+
if err != nil {
1587+
t.Errorf("CreatedFromVCS: error reading %s: %v", f.Name, err)
1588+
continue
1589+
}
1590+
if want != string(got) {
1591+
t.Errorf("CreatedFromVCS: zipped file %s contains %s, expected %s", f.Name, string(got), want)
1592+
continue
1593+
}
1594+
}
15321595
}
15331596
wantMap := map[string]bool{}
15341597
for _, f := range tc.wantFiles {
1535-
p := path.Join("example.com", "foo", "[email protected]", f)
1598+
p := path.Join(tc.version.String(), f)
15361599
wantMap[p] = true
15371600
}
15381601

@@ -1549,6 +1612,11 @@ c/`)))
15491612
t.Errorf("CreatedFromVCS: zipped file doesn't contain %s, but expected it to. all files: %v", f, gotFiles)
15501613
}
15511614
}
1615+
for f := range wantData {
1616+
if !gotMap[f] {
1617+
t.Errorf("CreatedFromVCS: zipped file doesn't contain %s, but expected it to. all files: %v", f, gotFiles)
1618+
}
1619+
}
15521620
})
15531621
}
15541622
}

0 commit comments

Comments
 (0)