internal/modindex: better behavior in edge cases

Write an index even if there is nothing in the module cache. Change-Id: Ia98f8825d9914a0d4bd2ee9ff1bccf8519b91f37 Reviewed-on: https://go-review.googlesource.com/c/tools/+/626735 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
jacobzim-stl · Nov 12, 2024 · 12610a1 · 12610a1
1 parent 06a498a
commit 12610a1
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 44 deletions.
diff --git a/internal/modindex/dir_test.go b/internal/modindex/dir_test.go
@@ -205,41 +205,42 @@ func TestDirsSinglePath(t *testing.T) {
 	}
 }
 
-/* more data for tests
-
-directories.go:169: WEIRD cloud.google.com/go/iam/admin/apiv1
-map[cloud.google.com/go:1 cloud.google.com/go/iam:5]:
-[cloud.google.com/go/[email protected]/admin/apiv1
-cloud.google.com/go/[email protected]/admin/apiv1
-cloud.google.com/go/[email protected]/admin/apiv1
-cloud.google.com/go/[email protected]/admin/apiv1
-cloud.google.com/go/[email protected]/admin/apiv1
-cloud.google.com/[email protected]/iam/admin/apiv1]
-directories.go:169: WEIRD cloud.google.com/go/iam
-map[cloud.google.com/go:1 cloud.google.com/go/iam:5]:
-[cloud.google.com/go/[email protected] cloud.google.com/go/[email protected]
-cloud.google.com/go/[email protected] cloud.google.com/go/[email protected]
-cloud.google.com/go/[email protected] cloud.google.com/[email protected]/iam]
-directories.go:169: WEIRD cloud.google.com/go/compute/apiv1
-map[cloud.google.com/go:1 cloud.google.com/go/compute:4]:
-[cloud.google.com/go/[email protected]/apiv1
-cloud.google.com/go/[email protected]/apiv1
-cloud.google.com/go/[email protected]/apiv1
-cloud.google.com/go/[email protected]/apiv1
-cloud.google.com/[email protected]/compute/apiv1]
-directories.go:169: WEIRD cloud.google.com/go/longrunning/autogen
-map[cloud.google.com/go:2 cloud.google.com/go/longrunning:2]:
-[cloud.google.com/go/[email protected]/autogen
-cloud.google.com/go/[email protected]/autogen
-cloud.google.com/[email protected]/longrunning/autogen
-cloud.google.com/[email protected]/longrunning/autogen]
-directories.go:169: WEIRD cloud.google.com/go/iam/credentials/apiv1
-map[cloud.google.com/go:1 cloud.google.com/go/iam:5]:
-[cloud.google.com/go/[email protected]/credentials/apiv1
-cloud.google.com/go/[email protected]/credentials/apiv1
-cloud.google.com/go/[email protected]/credentials/apiv1
-cloud.google.com/go/[email protected]/credentials/apiv1
-cloud.google.com/go/[email protected]/credentials/apiv1
-cloud.google.com/[email protected]/iam/credentials/apiv1]
+func TestMissingCachedir(t *testing.T) {
+	// behave properly if the cached dir is empty
+	dir := testModCache(t)
+	if err := Create(dir); err != nil {
+		t.Fatal(err)
+	}
+	ixd, err := IndexDir()
+	if err != nil {
+		t.Fatal(err)
+	}
+	des, err := os.ReadDir(ixd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(des) != 2 {
+		t.Errorf("got %d, butexpected two entries in index dir", len(des))
+	}
+}
 
-*/
+func TestMissingIndex(t *testing.T) {
+	// behave properly if there is no existing index
+	dir := testModCache(t)
+	if ok, err := Update(dir); err != nil {
+		t.Fatal(err)
+	} else if !ok {
+		t.Error("Update returned !ok")
+	}
+	ixd, err := IndexDir()
+	if err != nil {
+		t.Fatal(err)
+	}
+	des, err := os.ReadDir(ixd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(des) != 2 {
+		t.Errorf("got %d, butexpected two entries in index dir", len(des))
+	}
+}
diff --git a/internal/modindex/index.go b/internal/modindex/index.go
@@ -7,9 +7,11 @@ package modindex
 import (
 	"bufio"
 	"encoding/csv"
+	"errors"
 	"fmt"
 	"hash/crc64"
 	"io"
+	"io/fs"
 	"log"
 	"os"
 	"path/filepath"
@@ -85,7 +87,8 @@ type Entry struct {
 
 // ReadIndex reads the latest version of the on-disk index
 // for the cache directory cd.
-// It returns nil if there is none, or if there is an error.
+// It returns (nil, nil) if there is no index, but returns
+// a non-nil error if the index exists but could not be read.
 func ReadIndex(cachedir string) (*Index, error) {
 	cachedir, err := filepath.Abs(cachedir)
 	if err != nil {
@@ -100,10 +103,10 @@ func ReadIndex(cachedir string) (*Index, error) {
 	iname := filepath.Join(dir, base)
 	buf, err := os.ReadFile(iname)
 	if err != nil {
-		if err == os.ErrNotExist {
+		if errors.Is(err, fs.ErrNotExist) {
 			return nil, nil
 		}
-		return nil, fmt.Errorf("reading %s: %s %T", iname, err, err)
+		return nil, fmt.Errorf("cannot read %s: %w", iname, err)
 	}
 	fname := filepath.Join(dir, string(buf))
 	fd, err := os.Open(fname)
@@ -235,7 +238,6 @@ func writeIndexToFile(x *Index, fd *os.File) error {
 	if err := w.Flush(); err != nil {
 		return err
 	}
-	log.Printf("%d Entries %d names", len(x.Entries), cnt)
 	return nil
 }
 

diff --git a/internal/modindex/modindex.go b/internal/modindex/modindex.go
@@ -67,7 +67,7 @@ func modindexTimed(onlyBefore time.Time, cachedir Abspath, clear bool) (bool, er
 		if clear && err != nil {
 			return false, err
 		}
-		// TODO(pjw): check that most of those directorie still exist
+		// TODO(pjw): check that most of those directories still exist
 	}
 	cfg := &work{
 		onlyBefore: onlyBefore,
@@ -80,8 +80,8 @@ func modindexTimed(onlyBefore time.Time, cachedir Abspath, clear bool) (bool, er
 	if err := cfg.buildIndex(); err != nil {
 		return false, err
 	}
-	if len(cfg.newIndex.Entries) == 0 {
-		// no changes, don't write a new index
+	if len(cfg.newIndex.Entries) == 0 && curIndex != nil {
+		// no changes from existing curIndex, don't write a new index
 		return false, nil
 	}
 	if err := cfg.writeIndex(); err != nil {