Skip to content

Commit

Permalink
internal/modules: exclude vendor directories in downloaded modules
Browse files Browse the repository at this point in the history
For modules that have vendor directories, ecosystem metrics always
results in a loading error. 1% of the whole ecosystem has vendored
dependencies.

Vendor directories in modules downloaded from module proxy can have only
one file: modules.txt. When package loading logic sees a vendor
directory, it assumes the dependencies are there. Because they are in
fact not, loading of packages fails.

We hence remove the vendor directories altogether. This also makes
sense because, starting from go1.24, we'll see modules with vendor
directories being in principle empty, hence not even appearing in
the downloaded zip files.

This change skips unzipping the vendor directory when the module is
downloaded. An alternative approach is to explicitly delete the vendor
directory when analyzing the module. However, that has experimentally
proven unsuccessful. There is likely a file permission error.

Change-Id: I49d1f60e0e1679e586b14724f9fb729b2a8738df
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/608095
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Zvonimir Pavlinovic <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
  • Loading branch information
zpavlinovic authored and gopherbot committed Aug 23, 2024
1 parent f38f699 commit ed78f5d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
12 changes: 12 additions & 0 deletions internal/modules/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ func writeZip(r *zip.Reader, destination, stripPrefix string) error {
if !strings.HasPrefix(fpath, filepath.Clean(destination)+string(os.PathSeparator)) {
return fmt.Errorf("%s is an illegal filepath", fpath)
}

// Do not include vendor directory. They currently contain only modules.txt,
// not the dependencies. This makes package loading fail. Starting with go1.24,
// there likely won't be any vendor directories at all.
if vendored(name) {
continue
}

if f.FileInfo().IsDir() {
if err := os.MkdirAll(fpath, os.ModePerm); err != nil {
return err
Expand Down Expand Up @@ -74,3 +82,7 @@ func writeZip(r *zip.Reader, destination, stripPrefix string) error {
}
return nil
}

func vendored(path string) bool {
return path == "vendor" || strings.HasPrefix(path, "vendor"+string(os.PathSeparator))
}
18 changes: 16 additions & 2 deletions internal/modules/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package modules
import (
"archive/zip"
"bytes"
"os"
"path/filepath"
"testing"
)
Expand All @@ -20,6 +21,8 @@ func TestWriteZip(t *testing.T) {
}{
{filepath.Join("[email protected]", "README"), "This is a readme."},
{filepath.Join("[email protected]", "main"), "package main"},
{filepath.Join("[email protected]", "vendor", "modules.txt"), "# golang.org v1.1.1"},
{filepath.Join("[email protected]", "vendorius"), "This is some file with vendor in its name"},
}
for _, file := range files {
f, err := w.Create(file.Name)
Expand All @@ -44,10 +47,21 @@ func TestWriteZip(t *testing.T) {
}

tempDir := t.TempDir()
if err := writeZip(r, tempDir, ""); err != nil {
if err := writeZip(r, tempDir, "[email protected]/"); err != nil {
t.Error(err)
}
if err := writeZip(r, tempDir, "[email protected]"); err != nil {
// make sure there are no vendor files
fs, err := os.ReadDir(tempDir)
if err != nil {
t.Fatal(err)
}
for _, f := range fs {
if f.IsDir() && f.Name() == "vendor" {
t.Errorf("found unexpected vendor file or dir: %s", f.Name())
}
}

if err := writeZip(r, tempDir, ""); err != nil {
t.Error(err)
}
}

0 comments on commit ed78f5d

Please sign in to comment.