feat(internal/fetch): remove unmatched tarball (#3062)

If a tarball is found by name in the Librarian cache directory but its
SHA does not match the expected SHA, delete the tarball and fall through
to redownload.

Fixes #3043

Author: JoeWang1127

internal/fetch/cache.go

@@ -18,6 +18,7 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"io"
+	"log/slog"
 	"os"
 	"path/filepath"
 )
@@ -66,23 +67,29 @@ func RepoDir(repo, commit, expectedSHA256 string) (string, error) {
 	}
 
 	tgz := tarballPath(cacheDir, repo, commit)
-	outdir := filepath.Join(cacheDir, fmt.Sprintf("%s@%s", repo, commit))
+	outDir := filepath.Join(cacheDir, fmt.Sprintf("%s@%s", repo, commit))
 
 	// Step 1: Check if extracted directory exists and contains files.
 	if cached, err := extractedDir(cacheDir, repo, commit); err == nil {
 		return cached, nil
 	}
 
 	// Step 2: Check if tarball exists. Verify its SHA256 matches expectedSHA256.
-	// If hash doesn't match, fall through to re-download.
+	// If hash doesn't match or any error happens during the extraction, delete
+	// the tarball and fall through to re-download.
 	if _, err := os.Stat(tgz); err == nil {
 		sha, err := computeSHA256(tgz)
-		if err == nil && sha == expectedSHA256 {
-			if err := os.MkdirAll(outdir, 0755); err != nil {
-				return "", fmt.Errorf("failed creating %q: %w", outdir, err)
+		if err == nil {
+			if sha == expectedSHA256 {
+				if err := os.MkdirAll(outDir, 0755); err != nil {
+					return "", fmt.Errorf("failed creating %q: %w", outDir, err)
+				}
+				if err := ExtractTarball(tgz, outDir); err == nil {
+					return outDir, nil
+				}
 			}
-			if err := ExtractTarball(tgz, outdir); err == nil {
-				return outdir, nil
+			if err := os.Remove(tgz); err != nil {
+				slog.Debug("failed to remove tarball", "path", tgz, "err", err)
 			}
 		}
 	}
@@ -92,16 +99,16 @@ func RepoDir(repo, commit, expectedSHA256 string) (string, error) {
 	if err := os.MkdirAll(filepath.Dir(tgz), 0755); err != nil {
 		return "", fmt.Errorf("failed creating %q: %w", filepath.Dir(tgz), err)
 	}
-	if err := os.MkdirAll(outdir, 0755); err != nil {
-		return "", fmt.Errorf("failed creating %q: %w", outdir, err)
+	if err := os.MkdirAll(outDir, 0755); err != nil {
+		return "", fmt.Errorf("failed creating %q: %w", outDir, err)
 	}
 	if err := DownloadTarball(tgz, sourceURL, expectedSHA256); err != nil {
 		return "", err
 	}
-	if err := ExtractTarball(tgz, outdir); err != nil {
+	if err := ExtractTarball(tgz, outDir); err != nil {
 		return "", fmt.Errorf("failed to extract tarball: %w", err)
 	}
-	return outdir, nil
+	return outDir, nil
 }
 
 // cacheDir returns the root cache directory for librarian operations. It

internal/fetch/cache_test.go

@@ -165,6 +165,59 @@ func TestRepoDir_TarballExists(t *testing.T) {
 	}
 }
 
+func TestRepoDir_MismatchTarball(t *testing.T) {
+	cache := t.TempDir()
+	t.Setenv(envLibrarianCache, cache)
+	// Set up a mock web server to fetch a tarball.
+	tarballData := createTestTarball(t, "googleapis-"+testCommit, map[string]string{
+		"README.md":                    "# googleapis",
+		"google/api/annotations.proto": "syntax = \"proto3\";",
+	})
+	expectedSHA := fmt.Sprintf("%x", sha256.Sum256(tarballData))
+
+	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !strings.HasSuffix(r.URL.Path, "/archive/"+testCommit+".tar.gz") {
+			http.NotFound(w, r)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+		w.Write(tarballData)
+	}))
+	defer server.Close()
+
+	defer func(t http.RoundTripper) { http.DefaultTransport = t }(http.DefaultTransport)
+	http.DefaultTransport = server.Client().Transport
+	// Create an empty tarball file in the cache directory.
+	repo := strings.TrimPrefix(server.URL, "https://")
+	downloadDir := filepath.Join(cache, "download")
+	if err := os.MkdirAll(downloadDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+	tarballName := fmt.Sprintf("%s@%s.tar.gz", repo, testCommit)
+	f, err := os.Create(filepath.Join(downloadDir, tarballName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+
+	got, err := RepoDir(repo, testCommit, expectedSHA)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := os.Stat(filepath.Join(got, "README.md")); err != nil {
+		t.Errorf("expected README.md to exist: %v", err)
+	}
+	if _, err := os.Stat(filepath.Join(got, "google/api/annotations.proto")); err != nil {
+		t.Errorf("expected google/api/annotations.proto to exist: %v", err)
+	}
+
+	tarballPath := tarballPath(cache, repo, testCommit)
+	if _, err := os.Stat(tarballPath); err != nil {
+		t.Errorf("expected tarball to be cached at %q: %v", tarballPath, err)
+	}
+}
+
 func TestRepoDir_Download(t *testing.T) {
 	cachedir := t.TempDir()
 	t.Setenv(envLibrarianCache, cachedir)