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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/cmd/ejobs/main.go b/cmd/ejobs/main.go index 18c748c..d3b98df 100644 --- a/cmd/ejobs/main.go +++ b/cmd/ejobs/main.go
@@ -243,11 +243,13 @@ 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 @@ } 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 index cf78c34..8453ee2 100644 --- a/internal/modules/modules.go +++ b/internal/modules/modules.go
@@ -20,8 +20,8 @@ "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 index a11f478..c50651f 100644 --- a/internal/worker/analysis.go +++ b/internal/worker/analysis.go
@@ -255,7 +255,15 @@ } 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 index d61d4eb..fd86dc4 100644 --- a/internal/worker/govulncheck_scan.go +++ b/internal/worker/govulncheck_scan.go
@@ -237,7 +237,15 @@ 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 @@ 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 index f9c8e2d..745ae65 100644 --- a/internal/worker/scan.go +++ b/internal/worker/scan.go
@@ -238,32 +238,38 @@ } } -// 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 index 4233298..2992ab7 100644 --- a/internal/worker/scan_test.go +++ b/internal/worker/scan_test.go
@@ -49,7 +49,15 @@ } { 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) }