internal/worker: move prepareModule args to struct

There are a lot of args, and I'm about to add one more, so it will be clearer if they are in a struct. Also, print total job time in `ejobs wait`. Change-Id: I7101d93b32beb7c74083d6f34835400af8c46dfd Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/642875 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Zvonimir Pavlinovic <[email protected]>
golang · Jan 15, 2025 · 339a4c8 · 339a4c8
1 parent 87a8402
commit 339a4c8
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 21 deletions.
diff --git a/cmd/ejobs/main.go b/cmd/ejobs/main.go
@@ -243,11 +243,13 @@ func doWait(ctx context.Context, args []string) error {
 		return err
 	}
 	start := time.Now()
+	var jobStartedAt time.Time
 	for {
 		job, err := requestJSON[jobs.Job](ctx, "jobs/describe?jobid="+jobID, ts)
 		if err != nil {
 			return err
 		}
+		jobStartedAt = job.StartedAt
 		done := job.NumFinished()
 		if done >= job.NumEnqueued {
 			break
@@ -258,7 +260,7 @@ func doWait(ctx context.Context, args []string) error {
 		}
 		time.Sleep(sleepInterval)
 	}
-	fmt.Printf("Job %s finished.\n", jobID)
+	fmt.Printf("Job %s finished in %s.\n", jobID, time.Since(jobStartedAt))
 	return nil
 }
 

diff --git a/internal/modules/modules.go b/internal/modules/modules.go
@@ -20,8 +20,8 @@ import (
 	"golang.org/x/pkgsite-metrics/internal/proxy"
 )
 
-// Download fetches module at version via proxyClient and writes the modules
-// down to disk at dir.
+// Download fetches module at version via proxyClient and unzips the module
+// into dir.
 func Download(ctx context.Context, module, version, dir string, proxyClient *proxy.Client) error {
 	zipr, err := proxyClient.Zip(ctx, module, version)
 	if err != nil {

diff --git a/internal/worker/analysis.go b/internal/worker/analysis.go
@@ -255,7 +255,15 @@ func (s *analysisServer) scan(ctx context.Context, req *analysis.ScanRequest, lo
 }
 
 func (s *analysisServer) scanInternal(ctx context.Context, req *analysis.ScanRequest, binaryPath, moduleDir string) (jt analysis.JSONTree, err error) {
-	if err := prepareModule(ctx, req.Module, req.Version, moduleDir, s.proxyClient, req.Insecure, !req.SkipInit); err != nil {
+	args := prepareModuleArgs{
+		modulePath:  req.Module,
+		version:     req.Version,
+		dir:         moduleDir,
+		proxyClient: s.proxyClient,
+		insecure:    req.Insecure,
+		init:        !req.SkipInit,
+	}
+	if err := prepareModule(ctx, args); err != nil {
 		return nil, err
 	}
 	var sbox *sandbox.Sandbox

diff --git a/internal/worker/govulncheck_scan.go b/internal/worker/govulncheck_scan.go
@@ -237,7 +237,15 @@ func (s *scanner) CompareModule(ctx context.Context, w http.ResponseWriter, sreq
 		inputPath := moduleDir(baseRow.ModulePath, baseRow.Version)
 		defer derrors.Cleanup(&err, func() error { return os.RemoveAll(inputPath) })
 		const init = true
-		if err := prepareModule(ctx, baseRow.ModulePath, baseRow.Version, inputPath, s.proxyClient, s.insecure, init); err != nil {
+		args := prepareModuleArgs{
+			modulePath:  baseRow.ModulePath,
+			version:     baseRow.Version,
+			dir:         inputPath,
+			proxyClient: s.proxyClient,
+			insecure:    s.insecure,
+			init:        init,
+		}
+		if err := prepareModule(ctx, args); err != nil {
 			log.Errorf(ctx, err, "error trying to prepare module %s", baseRow.ModulePath)
 			return nil
 		}
@@ -462,7 +470,15 @@ func (s *scanner) runScanModule(ctx context.Context, modulePath, version, mode s
 		inputPath := moduleDir(modulePath, version)
 		defer derrors.Cleanup(&err, func() error { return os.RemoveAll(inputPath) })
 		const init = true
-		if err := prepareModule(ctx, modulePath, version, inputPath, s.proxyClient, s.insecure, init); err != nil {
+		args := prepareModuleArgs{
+			modulePath:  modulePath,
+			version:     version,
+			dir:         inputPath,
+			proxyClient: s.proxyClient,
+			insecure:    s.insecure,
+			init:        init,
+		}
+		if err := prepareModule(ctx, args); err != nil {
 			return err
 		}
 

diff --git a/internal/worker/scan.go b/internal/worker/scan.go
@@ -238,32 +238,38 @@ func gcsOpenFileFunc(ctx context.Context, bucket *storage.BucketHandle) openFile
 	}
 }
 
-// prepareModule prepares a module for scanning. It downloads the module to the given
-// directory and takes other actions that increase the chance that package loading will succeed.
-// If init is true, those other actions include calling `go mod init` and `go mod tidy` on modules
-// that don't have go.mod files.
-func prepareModule(ctx context.Context, modulePath, version, dir string, proxyClient *proxy.Client, insecure, init bool) error {
-	log.Debugf(ctx, "downloading %s@%s to %s", modulePath, version, dir)
-	if err := modules.Download(ctx, modulePath, version, dir, proxyClient); err != nil {
+type prepareModuleArgs struct {
+	modulePath, version string
+	dir                 string        // where to download the module
+	proxyClient         *proxy.Client // for downloading
+	insecure            bool          // run without sandbox
+	init                bool          // run `go mod init` and `go mod tidy` on modules with no go.mod file
+}
+
+// prepareModule prepares a module for scanning, and takes other actions that increase
+// the chance that package loading will succeed.
+func prepareModule(ctx context.Context, args prepareModuleArgs) error {
+	log.Debugf(ctx, "downloading %s@%s to %s", args.modulePath, args.version, args.dir)
+	if err := modules.Download(ctx, args.modulePath, args.version, args.dir, args.proxyClient); err != nil {
 		log.Debugf(ctx, "download error: %v (%[1]T)", err)
 		return err
 	}
 
-	hasGoMod := fileExists(filepath.Join(dir, "go.mod"))
-	if !init || hasGoMod {
+	hasGoMod := fileExists(filepath.Join(args.dir, "go.mod"))
+	if !args.init || hasGoMod {
 		// Download all dependencies, using the given directory for the Go module cache
 		// if it is non-empty.
 		opts := &goCommandOptions{
-			dir:      dir,
-			insecure: insecure,
+			dir:      args.dir,
+			insecure: args.insecure,
 		}
-		return runGoCommand(ctx, modulePath, version, opts, "mod", "download")
+		return runGoCommand(ctx, args.modulePath, args.version, opts, "mod", "download")
 	}
 	// Run `go mod init` and `go mod tidy`.
-	if err := goModInit(ctx, modulePath, version, dir, modulePath, insecure); err != nil {
+	if err := goModInit(ctx, args.modulePath, args.version, args.dir, args.modulePath, args.insecure); err != nil {
 		return err
 	}
-	return goModTidy(ctx, modulePath, version, dir, insecure)
+	return goModTidy(ctx, args.modulePath, args.version, args.dir, args.insecure)
 }
 
 // moduleDir returns a the path of a directory where the module can be downloaded.

diff --git a/internal/worker/scan_test.go b/internal/worker/scan_test.go
@@ -49,7 +49,15 @@ func TestPrepareModule(t *testing.T) {
 	} {
 		t.Run(fmt.Sprintf("%s@%s,%t", test.modulePath, test.version, test.init), func(t *testing.T) {
 			dir := t.TempDir()
-			err := prepareModule(ctx, test.modulePath, test.version, dir, proxyClient, insecure, test.init)
+			args := prepareModuleArgs{
+				modulePath:  test.modulePath,
+				version:     test.version,
+				dir:         dir,
+				proxyClient: proxyClient,
+				insecure:    insecure,
+				init:        test.init,
+			}
+			err := prepareModule(ctx, args)
 			if !errors.Is(err, test.want) {
 				t.Errorf("got %v, want %v", err, test.want)
 			}