From d71c2b4d3122b78532212a5069d2559a9ebf09dc Mon Sep 17 00:00:00 2001 From: fabio-gos-sonarsource Date: Mon, 29 Jun 2026 17:50:18 +0200 Subject: [PATCH 1/3] fix(scan): bound clones with a timeout and use a rolling worker pool An org-wide scan could freeze indefinitely on a single repository. Two root causes, both platform-agnostic (GitHub/GHE, GitLab, Bitbucket Cloud/DC, Azure all share this path): 1. gogit.Getrepos used go-git PlainClone with no deadline, so a stalled connection to the git server hung the worker forever. It also swallowed clone errors (logged, then returned a valid path with nil error), so a failed clone looked like an empty-but-successful repo. 2. AnalyseReposList ran fixed batches with a barrier: the next batch could not start until the slowest repo in the current batch finished, so one slow/hung repo stalled up to Workers-1 idle workers and froze overall progress ("Waiting for workers..."). Changes: - gogit.Getrepos now takes a timeout and uses PlainCloneContext; on failure it removes the partial clone and returns the error (timeouts become an explicit "clone timed out" message). - New CloneTimeout setting (minutes, default 15, 0 = disabled) threaded through RepoParams -> goloc.Params -> gogit. Added to config_sample.json, the webui platform defaults, and the advanced-options form. - AnalyseReposList rewritten as a semaphore-bounded rolling worker pool with a WaitGroup; same max concurrency as before, no batch barrier, so one slow repo can no longer block the others. - Skipped repositories (clone timeout/failure or analysis error) are now recorded per run and persisted to Results/config/analysis_skipped.json, then surfaced in the ResultsAll web page (+ /api/skipped-repositories) and the GlobalReport PDF. - Tests: clone-timeout + no-leak in gogit; getCloneTimeout and the concurrency-safe skipRecorder in golc; save/load roundtrip and PDF section rendering in utils. Co-Authored-By: Claude Opus 4.8 (1M context) --- ResultsAll.go | 70 ++++++++++++- config_sample.json | 18 ++-- golc.go | 203 ++++++++++++++++++++++++++++---------- golc_test.go | 57 +++++++---- pkg/gogit/gogit.go | 42 +++++++- pkg/gogit/gogit_test.go | 34 ++++++- pkg/goloc/goloc.go | 4 +- pkg/utils/globalreport.go | 93 ++++++++++++++++- pkg/utils/skipped.go | 62 ++++++++++++ pkg/utils/skipped_test.go | 95 ++++++++++++++++++ webui.go | 26 +++-- 11 files changed, 609 insertions(+), 95 deletions(-) create mode 100644 pkg/utils/skipped.go create mode 100644 pkg/utils/skipped_test.go diff --git a/ResultsAll.go b/ResultsAll.go index c36479d..4b91e0c 100644 --- a/ResultsAll.go +++ b/ResultsAll.go @@ -189,7 +189,8 @@ type PageData struct { Languages []LanguageData GlobalReport Globalinfo Repositories []RepositoryData - NoteLOCExcluded string // Note that JSON is excluded from total (SonarQube behavior) + SkippedRepos []utils.SkippedRepo // repos the analysis phase could not complete (clone timeout/failure, analysis error) + NoteLOCExcluded string // Note that JSON is excluded from total (SonarQube behavior) Platform string } @@ -911,10 +912,15 @@ func loadApplicationData() (PageData, error) { detectedPlatform, _, _ := detectPlatformAndReadAnalysis() + // Repositories that could not be analyzed (clone timeout/failure or counting + // error). Missing file (older result sets) => empty list, so the page still renders. + skippedRepos := utils.LoadSkippedRepos("Results") + pageData = PageData{ Languages: languages, GlobalReport: globalInfo, Repositories: repositoryData, + SkippedRepos: skippedRepos, NoteLOCExcluded: utils.NoteExcludedFromTotal, Platform: detectedPlatform, } @@ -924,8 +930,11 @@ func loadApplicationData() (PageData, error) { // setupHTTPHandlers configures all HTTP route handlers func setupHTTPHandlers(pageData PageData) { - // Load HTML template - tmpl := template.Must(template.New("index").Parse(htmlTemplate)) + // Load HTML template. The "add" helper renders 1-based row numbers in the + // skipped-repositories table (Go templates have no built-in increment). + tmpl := template.Must(template.New("index").Funcs(template.FuncMap{ + "add": func(a, b int) int { return a + b }, + }).Parse(htmlTemplate)) http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { err := tmpl.Execute(w, pageData) @@ -982,6 +991,17 @@ func setupHTTPHandlers(pageData PageData) { json.NewEncoder(w).Encode(pageData.Repositories) }) + // API Endpoint for repositories that could not be analyzed (clone timeout/failure, + // analysis error). Always returns a JSON array (empty when nothing was skipped). + http.HandleFunc("/api/skipped-repositories", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(contentTypeHeader, applicationJSONType) + skipped := pageData.SkippedRepos + if skipped == nil { + skipped = []utils.SkippedRepo{} + } + json.NewEncoder(w).Encode(skipped) + }) + // Repository Detail Page Handler http.HandleFunc("/repository/", func(w http.ResponseWriter, r *http.Request) { // Parse URL path to extract repository name and branch. @@ -1418,6 +1438,50 @@ const htmlTemplate = ` + + {{if .SkippedRepos}} + +
+
+
+
+
+
+ Skipped Repositories ({{len .SkippedRepos}}) +
+
+

+ These repositories were targeted but could not be analyzed (clone timeout, clone failure, or analysis error) and are not included in the totals above. Review the reason, then retry, raise the clone timeout, or add them to the exclusion list. +

+
+ + + + + + + + + + + {{range $i, $r := .SkippedRepos}} + + + + + + + {{end}} + +
#RepositoryBranchReason
{{add $i 1}}{{if $r.ProjectKey}}{{$r.ProjectKey}} / {{end}}{{$r.RepoSlug}}{{$r.Branch}}{{$r.Reason}}
+
+
+
+
+
+
+
+ {{end}} diff --git a/config_sample.json b/config_sample.json index cacd87c..83a4b18 100644 --- a/config_sample.json +++ b/config_sample.json @@ -25,7 +25,8 @@ "ResultByFile": true, "ResultAll": true, "Org": true, - "WorkDir": "" + "WorkDir": "", + "CloneTimeout": 15 }, "BitBucket": { "Users": "XXXXX", @@ -53,7 +54,8 @@ "ResultByFile": true, "ResultAll": true, "Org": true, - "WorkDir": "" + "WorkDir": "", + "CloneTimeout": 15 }, "Github": { "AccessToken": "XXXXX", @@ -79,7 +81,8 @@ "ResultByFile": true, "ResultAll": true, "Org": true, - "WorkDir": "" + "WorkDir": "", + "CloneTimeout": 15 }, "GithubEnterprise": { "AccessToken": "XXXXX", @@ -105,7 +108,8 @@ "ResultByFile": true, "ResultAll": true, "Org": true, - "WorkDir": "" + "WorkDir": "", + "CloneTimeout": 15 }, "Gitlab": { "AccessToken": "XXXXX", @@ -131,7 +135,8 @@ "ResultByFile": true, "ResultAll": true, "Org": true, - "WorkDir": "" + "WorkDir": "", + "CloneTimeout": 15 }, "Azure": { "AccessToken": "XXXXX", @@ -157,7 +162,8 @@ "ResultByFile": true, "ResultAll": true, "Org": true, - "WorkDir": "" + "WorkDir": "", + "CloneTimeout": 15 }, "File": { "Organization": "XXXXX", diff --git a/golc.go b/golc.go index 3a40008..4eb867f 100644 --- a/golc.go +++ b/golc.go @@ -99,12 +99,13 @@ type LanguageRes struct { } type RepoParams struct { - ProjectKey string - Namespace string - RepoSlug string - MainBranch string - PathToScan string - WorkDir string + ProjectKey string + Namespace string + RepoSlug string + MainBranch string + PathToScan string + WorkDir string + CloneTimeout time.Duration } type logWriter struct { @@ -341,50 +342,64 @@ func AnalyseReposList(DestinationResult string, platformConfig map[string]interf spin := spinner.New(spinner.CharSets[35], 100*time.Millisecond) spin.Color("green", "bold") - messageF := "" - spin.FinalMSG = messageF + spin.FinalMSG = "" - // Create a channel to receive results - results := make(chan int) - count := 1 + repos := repolist.([]interface{}) + total := len(repos) + + // Reset the per-run skip recorder so a re-run does not inherit stale entries. + skipped.reset() + // Effective concurrency: + // - multithreading off -> 1 (strictly sequential) + // - list <= NumberWorkerRepos -> run them all at once (preserves prior behavior) + // - otherwise -> Workers + concurrency := 1 if platformConfig["Multithreading"].(bool) { - if len(repolist.([]interface{})) > int(platformConfig["NumberWorkerRepos"].(float64)) { - // Launch goroutines in batches of X - X := int(platformConfig["Workers"].(float64)) - batches := len(repolist.([]interface{})) / X - remainder := len(repolist.([]interface{})) % X - for i := 0; i < batches; i++ { - for j := i * X; j < (i+1)*X; j++ { - go analyseRepoFunc(repolist.([]interface{})[j], DestinationResult, platformConfig, spin, results, &count) - } - waitForWorkers(X, results) - } - // Launch remaining goroutines - for i := batches * X; i < batches*X+remainder; i++ { - go analyseRepoFunc(repolist.([]interface{})[i], DestinationResult, platformConfig, spin, results, &count) - } - waitForWorkers(remainder, results) + if total <= int(platformConfig["NumberWorkerRepos"].(float64)) { + concurrency = total } else { - // Launch goroutines for each repo - for _, project := range repolist.([]interface{}) { - go analyseRepoFunc(project, DestinationResult, platformConfig, spin, results, &count) - } - waitForWorkers(len(repolist.([]interface{})), results) - } - } else { - // Without multithreading: run one repo at a time. analyseRepoFunc always - // sends to the unbuffered results channel, so it must run in a goroutine - // with a matching receiver — calling it synchronously here would block - // forever on that send (deadlock). Waiting for 1 result per iteration - // keeps the analysis strictly sequential. - for _, project := range repolist.([]interface{}) { - go analyseRepoFunc(project, DestinationResult, platformConfig, spin, results, &count) - waitForWorkers(1, results) + concurrency = int(platformConfig["Workers"].(float64)) } } + if concurrency < 1 { + concurrency = 1 + } + + // Rolling worker pool: a semaphore caps concurrency while every finished repo + // frees its slot for the next one immediately. This replaces the old fixed-batch + // barrier, where a single slow or hung repository stalled its entire batch (up to + // Workers-1 idle workers) and froze overall progress. Combined with the per-repo + // clone timeout in gogit.Getrepos, no single repository can hang the whole scan. + results := make(chan int, total) // buffered so worker sends never block + sem := make(chan struct{}, concurrency) + var wg sync.WaitGroup + count := 1 + + for _, project := range repos { + wg.Add(1) + sem <- struct{}{} // acquire a slot (blocks once concurrency is reached) + go func(p interface{}) { + defer wg.Done() + defer func() { <-sem }() // release the slot + analyseRepoFunc(p, DestinationResult, platformConfig, spin, results, &count) + }(project) + } - return len(repolist.([]interface{})) + wg.Wait() + close(results) + + // Persist the skipped repositories so the ResultsAll web page and the PDF report + // can surface them. DestinationResult is the base Results directory at this point. + skippedList := skipped.snapshot() + if err := utils.SaveSkippedRepos(DestinationResult, skippedList); err != nil { + logger.Errorf("❌ Error saving skipped repositories: %v", err) + } + if len(skippedList) > 0 { + logger.Warnf("⚠️ %d repository(ies) were skipped during analysis (see report for details)", len(skippedList)) + } + + return total } func getExcludePaths(configValue interface{}) []string { @@ -413,6 +428,75 @@ func getWorkDir(platformConfig map[string]interface{}) string { return "" } +// defaultCloneTimeoutMinutes bounds a single repository clone by default so that +// one stalled clone can no longer hang an entire org-wide scan. Operators can raise +// it for very large repos, or set CloneTimeout to 0 to disable the deadline. +const defaultCloneTimeoutMinutes = 15.0 + +// getCloneTimeout reads the optional per-platform "CloneTimeout" setting (in minutes) +// and returns it as a duration. Absent/invalid => default; an explicit 0 (or negative) +// disables the deadline. +func getCloneTimeout(platformConfig map[string]interface{}) time.Duration { + minutes := defaultCloneTimeoutMinutes + if v, ok := platformConfig["CloneTimeout"]; ok && v != nil { + if f, ok := v.(float64); ok { + if f <= 0 { + return 0 + } + minutes = f + } + } + return time.Duration(minutes * float64(time.Minute)) +} + +// skippedRepo describes a repository/branch that the analysis phase could not +// complete (clone timeout, clone failure, or counting error). It maps directly to +// utils.SkippedRepo for persistence. +type skippedRepo struct { + ProjectKey string + RepoSlug string + Branch string + Reason string +} + +// skipRecorder accumulates skipped repositories across the concurrent analysis +// workers. A single golc run analyzes one platform, so a package-level recorder +// (reset at the start of each AnalyseReposList) is sufficient; the mutex makes it +// safe for the worker goroutines to record concurrently. +type skipRecorder struct { + mu sync.Mutex + items []skippedRepo +} + +func (r *skipRecorder) reset() { + r.mu.Lock() + r.items = nil + r.mu.Unlock() +} + +func (r *skipRecorder) add(s skippedRepo) { + r.mu.Lock() + r.items = append(r.items, s) + r.mu.Unlock() +} + +func (r *skipRecorder) snapshot() []utils.SkippedRepo { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]utils.SkippedRepo, len(r.items)) + for i, s := range r.items { + out[i] = utils.SkippedRepo{ + ProjectKey: s.ProjectKey, + RepoSlug: s.RepoSlug, + Branch: s.Branch, + Reason: s.Reason, + } + } + return out +} + +var skipped = &skipRecorder{} + // exitGolc terminates the process after sweeping any temp clones that deferred // cleanup would otherwise miss (os.Exit does not run deferred funcs) — issue #81. // Use this instead of os.Exit anywhere a clone may already exist on disk. @@ -461,6 +545,7 @@ func analyseBitCRepo(project interface{}, DestinationResult string, platformConf MainBranch: p.MainBranch, PathToScan: pathToScan, WorkDir: getWorkDir(platformConfig), + CloneTimeout: getCloneTimeout(platformConfig), } performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) } @@ -482,6 +567,7 @@ func analyseBitSRVRepo(project interface{}, DestinationResult string, platformCo MainBranch: p.MainBranch, PathToScan: fmt.Sprintf("%s://%s:%s@%sscm/%s/%s.git", platformConfig["Protocol"].(string), platformConfig["Users"].(string), platformConfig["AccessToken"].(string), trimmedURL, p.ProjectKey, p.RepoSlug), WorkDir: getWorkDir(platformConfig), + CloneTimeout: getCloneTimeout(platformConfig), } performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) } @@ -504,6 +590,7 @@ func analyseGithubRepo(project interface{}, DestinationResult string, platformCo MainBranch: p.MainBranch, PathToScan: fmt.Sprintf("%s://%s:x-oauth-basic@%s/%s/%s.git", platformConfig["Protocol"].(string), platformConfig["AccessToken"].(string), baseapi, p.Org, p.RepoSlug), WorkDir: getWorkDir(platformConfig), + CloneTimeout: getCloneTimeout(platformConfig), } performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) } @@ -527,6 +614,7 @@ func analyseGitlabRepo(project interface{}, DestinationResult string, platformCo MainBranch: p.MainBranch, PathToScan: fmt.Sprintf("%s://gitlab-ci-token:%s@%s/%s.git", platformConfig["Protocol"].(string), platformConfig["AccessToken"].(string), domain, p.Namespace), WorkDir: getWorkDir(platformConfig), + CloneTimeout: getCloneTimeout(platformConfig), } performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) } @@ -547,6 +635,7 @@ func analyseAzurebRepo(project interface{}, DestinationResult string, platformCo MainBranch: p.MainBranch, PathToScan: fmt.Sprintf("%s://%s@%s/%s/%s/%s/%s", platformConfig["Protocol"].(string), platformConfig["AccessToken"].(string), "dev.azure.com", platformConfig["Organization"].(string), p.ProjectKey, "_git", p.RepoSlug), WorkDir: getWorkDir(platformConfig), + CloneTimeout: getCloneTimeout(platformConfig), } performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) } @@ -582,17 +671,33 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin Cloned: false, Repopath: "", WorkDir: params.WorkDir, + CloneTimeout: params.CloneTimeout, } if ResultAll { golocParams.ByFile = true } + + // recordSkip notes that this repository/branch could not be analyzed so it can be + // surfaced (with a reason) in the web page and PDF report instead of silently + // vanishing from the totals. + recordSkip := func(reason string) { + skipped.add(skippedRepo{ + ProjectKey: params.ProjectKey, + RepoSlug: params.RepoSlug, + Branch: params.MainBranch, + Reason: reason, + }) + } + MessB := fmt.Sprintf(" Extracting files from repo : %s ", params.RepoSlug) spin.Suffix = MessB spin.Start() gc, err := goloc.NewGCloc(golocParams, assets.Languages) if err != nil { + spin.Stop() logger.Errorf(errorMessageRepo+"%v", err) + recordSkip(err.Error()) *count++ results <- 1 return @@ -620,6 +725,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin if err := gc.Run(); err != nil { fmt.Print("\n") logger.Errorf("❌ Error during analysis with ByAll = true: %v", err) + recordSkip(fmt.Sprintf("analysis error: %v", err)) *count++ results <- 1 return @@ -635,6 +741,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin if err != nil { fmt.Print("\n") logger.Errorf("❌ Error initializing GCloc for ByFile = false: %v", err) + recordSkip(fmt.Sprintf("analysis error: %v", err)) *count++ results <- 1 return @@ -643,6 +750,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin if err := gc.Run(); err != nil { fmt.Print("\n") logger.Errorf("❌ Error during analysis with ByFile = false: %v", err) + recordSkip(fmt.Sprintf("analysis error: %v", err)) *count++ results <- 1 return @@ -652,6 +760,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin if err := gc.Run(); err != nil { fmt.Print("\n") logger.Errorf("❌ Error during analysis: %v", err) + recordSkip(fmt.Sprintf("analysis error: %v", err)) *count++ results <- 1 return @@ -668,14 +777,6 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin } } -// Wait for all goroutines to complete -func waitForWorkers(numWorkers int, results chan int) { - for i := 0; i < numWorkers; i++ { - fmt.Printf("\r Waiting for workers...\n") - <-results - } -} - // Specific analysis functions calling the generic one // Analysis function call for BitBucket Cloud diff --git a/golc_test.go b/golc_test.go index 132fd67..1ddef29 100644 --- a/golc_test.go +++ b/golc_test.go @@ -8,7 +8,9 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" + "time" "github.com/SonarSource-Demos/sonar-golc/pkg/devops/getazure" getbibucket "github.com/SonarSource-Demos/sonar-golc/pkg/devops/getbitbucket" @@ -482,24 +484,45 @@ func TestAnalysisFunctions(t *testing.T) { } }) - t.Run("waitForWorkers function", func(t *testing.T) { - // Create a test channel - results := make(chan int, 3) - - // Send some results - results <- 1 - results <- 1 - results <- 1 + t.Run("getCloneTimeout function", func(t *testing.T) { + // Absent key -> default. + if got := getCloneTimeout(map[string]interface{}{}); got != time.Duration(defaultCloneTimeoutMinutes*float64(time.Minute)) { + t.Errorf("absent CloneTimeout: got %v, want default %v minutes", got, defaultCloneTimeoutMinutes) + } + // Explicit value (minutes) -> that duration. + if got := getCloneTimeout(map[string]interface{}{"CloneTimeout": float64(5)}); got != 5*time.Minute { + t.Errorf("CloneTimeout=5: got %v, want 5m", got) + } + // Zero -> disabled (0 duration, no deadline). + if got := getCloneTimeout(map[string]interface{}{"CloneTimeout": float64(0)}); got != 0 { + t.Errorf("CloneTimeout=0: got %v, want 0 (disabled)", got) + } + // Negative -> disabled. + if got := getCloneTimeout(map[string]interface{}{"CloneTimeout": float64(-3)}); got != 0 { + t.Errorf("CloneTimeout<0: got %v, want 0 (disabled)", got) + } + }) - // Should not hang or panic - func() { - defer func() { - if r := recover(); r != nil { - t.Errorf("waitForWorkers panicked: %v", r) - } - }() - waitForWorkers(3, results) - }() + t.Run("skipRecorder is concurrency-safe", func(t *testing.T) { + r := &skipRecorder{} + r.reset() + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + r.add(skippedRepo{RepoSlug: "repo", Branch: "main", Reason: "clone timed out"}) + }(i) + } + wg.Wait() + if got := len(r.snapshot()); got != 50 { + t.Errorf("skipRecorder recorded %d entries, want 50", got) + } + // reset clears the slate for the next run. + r.reset() + if got := len(r.snapshot()); got != 0 { + t.Errorf("after reset, got %d entries, want 0", got) + } }) } diff --git a/pkg/gogit/gogit.go b/pkg/gogit/gogit.go index 3fd50f9..7346c88 100644 --- a/pkg/gogit/gogit.go +++ b/pkg/gogit/gogit.go @@ -1,13 +1,16 @@ package gogit import ( + "context" "crypto/rand" "encoding/hex" + "errors" "fmt" "log" "os" "path/filepath" "regexp" + "time" "github.com/SonarSource-Demos/sonar-golc/pkg/utils" @@ -18,7 +21,19 @@ import ( //"github.com/go-git/go-git/v5/plumbing/transport/http" ) -func Getrepos(src, branch, token, workDir string) (string, error) { +// Getrepos shallow-clones a single branch of src into a fresh temp directory under +// workDir and returns the clone path. +// +// A non-zero timeout bounds the clone via context: go-git's PlainClone has no +// internal deadline, so a half-open TCP connection to the git server (VPN drop, +// proxy, server under load) would otherwise hang the cloning goroutine forever and +// stall the whole analysis. A timeout of 0 disables the deadline (legacy behavior). +// +// Clone failures are returned to the caller (and the partial clone is removed) +// instead of being swallowed: the previous code logged the error but still returned +// a valid path with a nil error, so callers treated a failed clone as an +// empty-but-successful repository. +func Getrepos(src, branch, token, workDir string, timeout time.Duration) (string, error) { loggers := utils.SharedLogger() suffix, err := randomSuffix() @@ -41,7 +56,15 @@ func Getrepos(src, branch, token, workDir string) (string, error) { capability.ThinPack, } - _, err = git.PlainClone(dst, false, &git.CloneOptions{ + // Bound the clone with a context deadline when a timeout is configured. + ctx := context.Background() + if timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } + + _, err = git.PlainCloneContext(ctx, dst, false, &git.CloneOptions{ URL: src, ReferenceName: plumbing.NewBranchReferenceName(branch), @@ -54,9 +77,20 @@ func Getrepos(src, branch, token, workDir string) (string, error) { if err != nil { re := regexp.MustCompile(`(https?:\/\/)[^@]+(@)`) maskedSrc := re.ReplaceAllString(src, "${1}*****${2}") - //fmt.Printf("\n--❌ Stack: gogit.Getrepos Git Branch %s - %s-- Source: %s -", plumbing.Main, err, maskedSrc) - loggers.Errorf("\r\t\t\t\t❌ Stack: gogit.Getrepos Git Branch %s - %s-- Source: %s -", plumbing.Main, err, maskedSrc) + // A partial clone may exist on disk; remove it and stop tracking it so a + // failed clone does not leak into the work directory (issue #81). + _ = os.RemoveAll(dst) + utils.UnregisterTempClone(dst) + + // A deadline becomes an explicit timeout error so operators know to tune + // CloneTimeout or exclude the offending repository. + if errors.Is(err, context.DeadlineExceeded) { + loggers.Errorf("\r\t\t\t\t❌ gogit.Getrepos: clone of branch %s timed out after %s -- Source: %s", branch, timeout, maskedSrc) + return "", fmt.Errorf("clone timed out after %s", timeout) + } + loggers.Errorf("\r\t\t\t\t❌ Stack: gogit.Getrepos Git Branch %s - %s-- Source: %s -", branch, err, maskedSrc) + return "", fmt.Errorf("clone failed: %w", err) } symLink, err := isSymLink(dst) diff --git a/pkg/gogit/gogit_test.go b/pkg/gogit/gogit_test.go index 8919982..f8b524f 100644 --- a/pkg/gogit/gogit_test.go +++ b/pkg/gogit/gogit_test.go @@ -4,7 +4,9 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" + "time" ) // makeLocalRepo creates a throwaway local git repository with one commit on @@ -41,7 +43,7 @@ func TestGetreposLocalClone(t *testing.T) { repo := makeLocalRepo(t) work := t.TempDir() - dst, err := Getrepos(repo, "master", "", work) + dst, err := Getrepos(repo, "master", "", work, 30*time.Second) if err != nil { t.Fatalf("Getrepos: %v", err) } @@ -61,7 +63,35 @@ func TestGetreposBadWorkDir(t *testing.T) { if err := os.WriteFile(f, []byte("x"), 0644); err != nil { t.Fatalf("setup: %v", err) } - if _, err := Getrepos("https://example.invalid/x.git", "master", "", filepath.Join(f, "sub")); err == nil { + if _, err := Getrepos("https://example.invalid/x.git", "master", "", filepath.Join(f, "sub"), 30*time.Second); err == nil { t.Error("expected error for unusable work dir, got nil") } } + +// TestGetreposCloneTimeout verifies that a clone which cannot complete in time +// returns a timeout error (rather than hanging) and leaves no temp clone behind. +// It points at a non-routable address so the connection stalls until the deadline. +func TestGetreposCloneTimeout(t *testing.T) { + work := t.TempDir() + + // 198.51.100.0/24 (TEST-NET-2) is reserved and non-routable, so the TCP + // connect blocks until our context deadline fires. + _, err := Getrepos("https://198.51.100.1/some/repo.git", "master", "", work, 1*time.Second) + if err == nil { + t.Fatal("expected a timeout/clone error, got nil") + } + if !strings.Contains(err.Error(), "timed out") && !strings.Contains(err.Error(), "clone failed") { + t.Errorf("expected a timeout or clone-failed error, got: %v", err) + } + + // The partial/failed clone must not be left behind in the work dir. + entries, rerr := os.ReadDir(work) + if rerr != nil { + t.Fatalf("read work dir: %v", rerr) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), "gcloc-extract-") { + t.Errorf("failed clone left temp dir behind: %s", e.Name()) + } + } +} diff --git a/pkg/goloc/goloc.go b/pkg/goloc/goloc.go index 0409a57..ed76b38 100644 --- a/pkg/goloc/goloc.go +++ b/pkg/goloc/goloc.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "time" "github.com/SonarSource-Demos/sonar-golc/pkg/analyzer" "github.com/SonarSource-Demos/sonar-golc/pkg/filesystem" @@ -45,6 +46,7 @@ type Params struct { Repopath string RepopathDisposable bool // if true, Repopath is a temp clone safe to delete; if false, it is the user's directory and must not be removed WorkDir string // base directory for temp clones/extractions; empty => GOLC_WORKDIR env or os.TempDir() + CloneTimeout time.Duration // per-repo clone deadline; 0 disables (bounds hung clones, see gogit.Getrepos) } type GCloc struct { @@ -190,7 +192,7 @@ func getRepoPath(params Params) (path string, disposable bool, err error) { } if len(params.Branch) != 0 { - p, e := gogit.Getrepos(params.Path, params.Branch, params.Token, params.WorkDir) + p, e := gogit.Getrepos(params.Path, params.Branch, params.Token, params.WorkDir, params.CloneTimeout) return p, true, e } // Use local path directly when it is an existing directory (Directory / file analysis). diff --git a/pkg/utils/globalreport.go b/pkg/utils/globalreport.go index 120e910..d55138e 100644 --- a/pkg/utils/globalreport.go +++ b/pkg/utils/globalreport.go @@ -92,8 +92,13 @@ func CreateGlobalReport(directory string) error { return err } + // Repositories the analysis phase could not complete (clone timeout/failure or + // counting error). Surfaced in the PDF so a large scan that skips a few problem + // repos does not silently undercount. Missing file => empty list. + skippedRepos := LoadSkippedRepos(directory) + // Create a PDF - if err := renderGlobalPDF(languages, ginfo); err != nil { + if err := renderGlobalPDF(languages, ginfo, skippedRepos); err != nil { return err } @@ -313,8 +318,88 @@ func renderLanguageRow(pdf *gofpdf.Fpdf, lang LanguageData, i, maxLOC int, barCo pdf.SetXY(marginL, rowY+rowH) } +// renderSkippedReposSection appends a "Skipped Repositories" section to the global +// PDF. When no repositories were skipped it renders a short confirmation line; when +// some were, it lists each with its branch and the reason it was skipped (clone +// timeout, clone failure, or analysis error). +func renderSkippedReposSection(pdf *gofpdf.Fpdf, skippedRepos []SkippedRepo, marginL, contentW float64) { + pdf.Ln(8) + + // Section header bar (amber to read as a warning, distinct from the blue + // "Language Breakdown" header). + pdf.SetFillColor(214, 137, 16) + pdf.Rect(marginL, pdf.GetY(), contentW, 8, "F") + pdf.SetFont("Helvetica", "B", 10) + pdf.SetTextColor(255, 255, 255) + pdf.SetX(marginL + 2) + pdf.CellFormat(contentW-2, 8, fmt.Sprintf("Skipped Repositories (%d)", len(skippedRepos)), "", 1, "L", false, 0, "") + pdf.Ln(2) + + if len(skippedRepos) == 0 { + pdf.SetFont("Helvetica", "I", 8) + pdf.SetTextColor(110, 110, 120) + pdf.SetX(marginL) + pdf.CellFormat(contentW, 6, "None — all targeted repositories were analyzed.", "", 1, "L", false, 0, "") + pdf.SetTextColor(0, 0, 0) + return + } + + const ( + colNum = 10.0 + colRepo = 55.0 + colBranch = 38.0 + ) + colReason := contentW - colNum - colRepo - colBranch + + drawHeaders := func() { + pdf.SetFillColor(245, 226, 196) + pdf.SetFont("Helvetica", "B", 8) + pdf.SetTextColor(60, 45, 20) + pdf.SetX(marginL) + pdf.CellFormat(colNum, 6, "#", "0", 0, "C", true, 0, "") + pdf.CellFormat(colRepo, 6, "REPOSITORY", "0", 0, "L", true, 0, "") + pdf.CellFormat(colBranch, 6, "BRANCH", "0", 0, "L", true, 0, "") + pdf.CellFormat(colReason, 6, "REASON", "0", 1, "L", true, 0, "") + } + drawHeaders() + + // Truncate a cell value so it fits its column width (gofpdf has no native + // ellipsis); width is estimated from the current font's string width. + fit := func(s string, w float64) string { + if pdf.GetStringWidth(s) <= w-2 { + return s + } + for len(s) > 1 && pdf.GetStringWidth(s+"...") > w-2 { + s = s[:len(s)-1] + } + return s + "..." + } + + pdf.SetFont("Helvetica", "", 8) + pdf.SetTextColor(40, 40, 50) + for i, r := range skippedRepos { + // Repeat the header after an auto page break so long lists stay readable. + if pdf.GetY() > 270 { + pdf.AddPage() + drawHeaders() + pdf.SetFont("Helvetica", "", 8) + pdf.SetTextColor(40, 40, 50) + } + repo := r.RepoSlug + if r.ProjectKey != "" { + repo = r.ProjectKey + "/" + r.RepoSlug + } + pdf.SetX(marginL) + pdf.CellFormat(colNum, 6, fmt.Sprintf("%d", i+1), "0", 0, "C", false, 0, "") + pdf.CellFormat(colRepo, 6, fit(repo, colRepo), "0", 0, "L", false, 0, "") + pdf.CellFormat(colBranch, 6, fit(r.Branch, colBranch), "0", 0, "L", false, 0, "") + pdf.CellFormat(colReason, 6, fit(r.Reason, colReason), "0", 1, "L", false, 0, "") + } + pdf.SetTextColor(0, 0, 0) +} + // renderGlobalPDF generates the GlobalReport.pdf from languages and global info. -func renderGlobalPDF(languages []LanguageData, ginfo Globalinfo) error { +func renderGlobalPDF(languages []LanguageData, ginfo Globalinfo, skippedRepos []SkippedRepo) error { loggers := NewLogger() languages, maxLOC := prepareLanguagesForPDF(languages) @@ -462,6 +547,10 @@ func renderGlobalPDF(languages []LanguageData, ginfo Globalinfo) error { rowH := 7.0 renderLanguageRows(pdf, languages, maxLOC, drawColHeaders, marginL, pageH, marginB, colNum, colLang, colLOC, colPct, colBar, rowH, barColors) + + // ── Skipped repositories section ───────────────────────────────── + renderSkippedReposSection(pdf, skippedRepos, marginL, contentW) + if err := pdf.OutputFileAndClose("Results/GlobalReport.pdf"); err != nil { loggers.Errorf("Error saving PDF file: %v", err) return err diff --git a/pkg/utils/skipped.go b/pkg/utils/skipped.go new file mode 100644 index 0000000..57e6bce --- /dev/null +++ b/pkg/utils/skipped.go @@ -0,0 +1,62 @@ +package utils + +import ( + "encoding/json" + "os" + "path/filepath" +) + +// SkippedRepo records a repository (branch) that could not be analyzed during the +// analysis phase — e.g. its clone timed out, the clone failed, or line counting +// errored. These are surfaced in the ResultsAll web page and the PDF report so a +// large org-wide scan that skips a few problem repos does not silently undercount. +type SkippedRepo struct { + ProjectKey string `json:"ProjectKey"` + RepoSlug string `json:"RepoSlug"` + Branch string `json:"Branch"` + Reason string `json:"Reason"` +} + +// SkippedReport is the on-disk envelope persisted to Results/config/analysis_skipped.json. +type SkippedReport struct { + SkippedRepositories []SkippedRepo `json:"SkippedRepositories"` +} + +// SkippedReposPath returns the canonical location of the skipped-repos file for a +// given base Results directory. +func SkippedReposPath(baseResultsDir string) string { + return filepath.Join(baseResultsDir, "config", "analysis_skipped.json") +} + +// SaveSkippedRepos writes the skipped-repos list under /config. The +// file is always (re)written — including with an empty list — so a clean re-run +// clears any stale entries from a previous analysis. +func SaveSkippedRepos(baseResultsDir string, repos []SkippedRepo) error { + if repos == nil { + repos = []SkippedRepo{} + } + path := SkippedReposPath(baseResultsDir) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + data, err := json.MarshalIndent(SkippedReport{SkippedRepositories: repos}, "", " ") + if err != nil { + return err + } + return os.WriteFile(path, data, 0644) +} + +// LoadSkippedRepos reads the skipped-repos list for a base Results directory. A +// missing or unreadable file yields an empty slice (not an error): reports should +// render fine on older result sets that predate this feature. +func LoadSkippedRepos(baseResultsDir string) []SkippedRepo { + data, err := os.ReadFile(SkippedReposPath(baseResultsDir)) + if err != nil { + return nil + } + var rep SkippedReport + if err := json.Unmarshal(data, &rep); err != nil { + return nil + } + return rep.SkippedRepositories +} diff --git a/pkg/utils/skipped_test.go b/pkg/utils/skipped_test.go new file mode 100644 index 0000000..b2e35b5 --- /dev/null +++ b/pkg/utils/skipped_test.go @@ -0,0 +1,95 @@ +package utils + +import ( + "os" + "path/filepath" + "testing" + + "github.com/jung-kurt/gofpdf" +) + +// TestRenderSkippedReposSection exercises the PDF section renderer for both the +// empty and populated cases, including a list long enough to trigger a page break +// and a very long reason that must be truncated — it must not panic. +func TestRenderSkippedReposSection(t *testing.T) { + cases := map[string][]SkippedRepo{ + "empty": nil, + "one": {{ProjectKey: "org", RepoSlug: "repo", Branch: "main", Reason: "clone timed out after 15m0s"}}, + } + // A long list with a long reason to cover truncation + page-break paths. + var many []SkippedRepo + for i := 0; i < 60; i++ { + many = append(many, SkippedRepo{ + RepoSlug: "some-really-quite-long-repository-name-number", + Branch: "feature/a-fairly-long-branch-name", + Reason: "clone failed: error: RPC failed; curl 56 recv failure: connection reset by peer the remote end hung up unexpectedly", + }) + } + cases["many"] = many + + for name, repos := range cases { + t.Run(name, func(t *testing.T) { + pdf := gofpdf.New("P", "mm", "A4", "") + pdf.SetFont("Helvetica", "", 8) + pdf.AddPage() + renderSkippedReposSection(pdf, repos, 15.0, 180.0) + if err := pdf.OutputFileAndClose(filepath.Join(t.TempDir(), "out.pdf")); err != nil { + t.Fatalf("render/output failed: %v", err) + } + }) + } +} + +// TestSkippedReposRoundTrip verifies that the skipped-repos list persists and +// reloads intact from /config/analysis_skipped.json. +func TestSkippedReposRoundTrip(t *testing.T) { + base := t.TempDir() + + want := []SkippedRepo{ + {ProjectKey: "acme", RepoSlug: "bi_publishing", Branch: "main", Reason: "clone timed out after 15m0s"}, + {ProjectKey: "", RepoSlug: "lonely-repo", Branch: "develop", Reason: "analysis error: boom"}, + } + + if err := SaveSkippedRepos(base, want); err != nil { + t.Fatalf("SaveSkippedRepos: %v", err) + } + + // File must land at the canonical path. + if _, err := os.Stat(filepath.Join(base, "config", "analysis_skipped.json")); err != nil { + t.Fatalf("expected skipped file at canonical path: %v", err) + } + + got := LoadSkippedRepos(base) + if len(got) != len(want) { + t.Fatalf("loaded %d entries, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("entry %d: got %+v, want %+v", i, got[i], want[i]) + } + } +} + +// TestSaveSkippedReposEmptyClearsStale verifies that saving an empty list writes a +// valid (empty) file so a clean re-run clears entries from a previous analysis. +func TestSaveSkippedReposEmptyClearsStale(t *testing.T) { + base := t.TempDir() + + if err := SaveSkippedRepos(base, []SkippedRepo{{RepoSlug: "x", Branch: "main", Reason: "stale"}}); err != nil { + t.Fatalf("seed save: %v", err) + } + if err := SaveSkippedRepos(base, nil); err != nil { + t.Fatalf("empty save: %v", err) + } + if got := LoadSkippedRepos(base); len(got) != 0 { + t.Errorf("after empty save, loaded %d entries, want 0", len(got)) + } +} + +// TestLoadSkippedReposMissing verifies a missing file yields an empty slice (not an +// error) so reports render fine on result sets that predate this feature. +func TestLoadSkippedReposMissing(t *testing.T) { + if got := LoadSkippedRepos(t.TempDir()); len(got) != 0 { + t.Errorf("missing file: got %d entries, want 0", len(got)) + } +} diff --git a/webui.go b/webui.go index ed66390..12a9259 100644 --- a/webui.go +++ b/webui.go @@ -144,7 +144,7 @@ var platformDefaults = map[string]map[string]interface{}{ "ResultAll": true, "Org": true, "Period": float64(-1), "Factor": float64(33), "DefaultBranch": true, "Stats": false, "ResultByFile": true, "FolderKeywords": []interface{}{}, "FileNamePatterns": []interface{}{}, - "WorkDir": "", "Repos": "", "Project": "", "Branch": "", + "WorkDir": "", "Repos": "", "Project": "", "Branch": "", "CloneTimeout": float64(15), }, "GithubEnterprise": { "DevOps": "github", "Apiver": "2022-11-28", "FileExclusion": "", @@ -152,7 +152,7 @@ var platformDefaults = map[string]map[string]interface{}{ "ResultAll": true, "Org": true, "Period": float64(-1), "Factor": float64(33), "DefaultBranch": true, "Stats": false, "ResultByFile": true, "FolderKeywords": []interface{}{}, "FileNamePatterns": []interface{}{}, - "WorkDir": "", "Repos": "", "Project": "", "Branch": "", + "WorkDir": "", "Repos": "", "Project": "", "Branch": "", "CloneTimeout": float64(15), }, "Gitlab": { "DevOps": "gitlab", "Url": "https://gitlab.com/", "Apiver": "v4", @@ -161,7 +161,7 @@ var platformDefaults = map[string]map[string]interface{}{ "ResultAll": true, "Org": true, "Period": float64(-1), "Factor": float64(33), "DefaultBranch": true, "Stats": false, "ResultByFile": true, "FolderKeywords": []interface{}{}, "FileNamePatterns": []interface{}{}, - "WorkDir": "", "Repos": "", "Project": "", "Branch": "", + "WorkDir": "", "Repos": "", "Project": "", "Branch": "", "CloneTimeout": float64(15), }, "BitBucket": { "DevOps": "bitbucket", "Url": "https://api.bitbucket.org/", "Apiver": "2.0", @@ -170,7 +170,7 @@ var platformDefaults = map[string]map[string]interface{}{ "ResultAll": true, "Org": true, "Period": float64(-1), "Factor": float64(33), "DefaultBranch": true, "Stats": false, "ResultByFile": true, "FolderKeywords": []interface{}{}, "FileNamePatterns": []interface{}{}, - "WorkDir": "", "Repos": "", "Project": "", "Branch": "", + "WorkDir": "", "Repos": "", "Project": "", "Branch": "", "CloneTimeout": float64(15), }, "BitBucketSRV": { "DevOps": "bitbucket_dc", "Apiver": "1.0", "Baseapi": "rest/api/", @@ -179,7 +179,7 @@ var platformDefaults = map[string]map[string]interface{}{ "ResultAll": true, "Org": true, "Period": float64(-5), "Factor": float64(33), "DefaultBranch": true, "Stats": false, "ResultByFile": true, "FolderKeywords": []interface{}{}, "FileNamePatterns": []interface{}{}, - "WorkDir": "", "Repos": "", "Project": "", "Branch": "", + "WorkDir": "", "Repos": "", "Project": "", "Branch": "", "CloneTimeout": float64(15), }, "Azure": { "DevOps": "azure", "Url": "https://dev.azure.com/", "Apiver": "7.1", @@ -188,7 +188,7 @@ var platformDefaults = map[string]map[string]interface{}{ "ResultAll": true, "Org": true, "Period": float64(-1), "Factor": float64(33), "DefaultBranch": true, "Stats": false, "ResultByFile": true, "FolderKeywords": []interface{}{}, "FileNamePatterns": []interface{}{}, - "WorkDir": "", "Repos": "", "Project": "", "Branch": "", + "WorkDir": "", "Repos": "", "Project": "", "Branch": "", "CloneTimeout": float64(15), }, "File": { "DevOps": "file", "FileExclusion": "", "FileLoad": ".cloc_file_load", @@ -1254,16 +1254,20 @@ const htmlTemplate = `
-
+
-
+
+
+ + +
@@ -1729,7 +1733,7 @@ function syncPresetKeywords() { } function populateAdvanced(key, saved) { - const defaults = {DefaultBranch:true,Branch:'',Multithreading:true,Workers:10,WorkDir:'', + const defaults = {DefaultBranch:true,Branch:'',Multithreading:true,Workers:10,WorkDir:'',CloneTimeout:15, FileExclusion:'',ExtExclusion:[],ExcludeTests:true,ExcludeVendor:true,ExcludeBuild:true, FolderKeywords:[],FileNamePatterns:DEFAULT_FILE_PATTERNS,Repos:'',Project:'',ResultByFile:true,Org:true}; const cfg = Object.assign({}, defaults, saved); @@ -1738,6 +1742,8 @@ function populateAdvanced(key, saved) { document.getElementById('adv-branch').value = cfg.Branch || ''; document.getElementById('adv-multithreading').checked = cfg.Multithreading !== false; document.getElementById('adv-workers').value = cfg.Workers || 10; + const ctEl = document.getElementById('adv-cloneTimeout'); + if (ctEl) ctEl.value = (cfg.CloneTimeout === undefined || cfg.CloneTimeout === null) ? 15 : cfg.CloneTimeout; document.getElementById('adv-workDir').value = cfg.WorkDir || ''; document.getElementById('adv-extExclusion').value = Array.isArray(cfg.ExtExclusion) ? cfg.ExtExclusion.filter(Boolean).join(',') : (cfg.ExtExclusion||''); @@ -1854,6 +1860,8 @@ function gatherConfig() { cfg.Multithreading = document.getElementById('adv-multithreading').checked; cfg.Workers = parseInt(document.getElementById('adv-workers').value) || 10; cfg.NumberWorkerRepos = cfg.Workers; + const ctRaw = document.getElementById('adv-cloneTimeout'); + cfg.CloneTimeout = ctRaw ? (parseInt(ctRaw.value, 10) || 0) : 15; cfg.WorkDir = document.getElementById('adv-workDir').value.trim(); cfg.FileExclusion = ''; const ext = document.getElementById('adv-extExclusion').value.trim(); From 5ccf8979a17b7ea584741a2f3f0e4dc42a7dce48 Mon Sep 17 00:00:00 2001 From: fabio-gos-sonarsource Date: Tue, 30 Jun 2026 09:53:32 +0200 Subject: [PATCH 2/3] fix(review): atomic progress counter + safer CloneTimeout default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two Gitar review findings on #91: 1. Data race on the shared progress counter. AnalyseReposList passed &count (plain int) into every worker goroutine; the rolling pool runs them concurrently and each does count++/reads count. Convert the counter to atomic.Int64 across analyseRepoFunc / the five analyse*Repo funcs / performRepoAnalysis / analyseDirectory, and increment via count.Add(1). This also fixes a latent bug where the success log only read count (never incremented it), so every repo printed "1" — each completed repo now gets a unique sequence number. The same shared pattern in AnalyseReposListFile/analyseDirectory is fixed too. Adds TestAnalyseReposListConcurrentCountNoRace as a -race regression guard over the multithreaded path. 2. Clearing the webui "Clone timeout" field silently disabled the deadline: parseInt('')||0 -> 0, which getCloneTimeout reads as "no timeout" — re-introducing the exact hang this feature prevents. An empty/blank field now falls back to the default (15); an explicit "0" still disables the deadline. Verified: go test -race passes for ./pkg/... and -tags golc (incl. the new concurrency test); webui and resultsall binaries build. Co-Authored-By: Claude Opus 4.8 (1M context) --- golc.go | 43 +++++++++++++++++++++++-------------------- golc_workdir_test.go | 44 ++++++++++++++++++++++++++++++++++++++++---- webui.go | 6 +++++- 3 files changed, 68 insertions(+), 25 deletions(-) diff --git a/golc.go b/golc.go index 4eb867f..8c28192 100644 --- a/golc.go +++ b/golc.go @@ -13,6 +13,7 @@ import ( "path/filepath" "strings" "sync" + "sync/atomic" "time" "github.com/sirupsen/logrus" @@ -336,7 +337,7 @@ func addFileToZip(filePath, relPath string, fileInfo os.FileInfo, zipWriter *zip } // Generic function to analyze repositories -func AnalyseReposList(DestinationResult string, platformConfig map[string]interface{}, repolist interface{}, analyseRepoFunc func(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *int)) (cpt int) { +func AnalyseReposList(DestinationResult string, platformConfig map[string]interface{}, repolist interface{}, analyseRepoFunc func(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64)) (cpt int) { //fmt.Print("\n🔎 Analysis of Repos ...\n") logger.Infof("🔎 Analysis of Repos ...\n") @@ -374,7 +375,9 @@ func AnalyseReposList(DestinationResult string, platformConfig map[string]interf results := make(chan int, total) // buffered so worker sends never block sem := make(chan struct{}, concurrency) var wg sync.WaitGroup - count := 1 + // Progress counter shared across the worker goroutines. It is read and bumped + // concurrently by performRepoAnalysis, so it must be atomic, not a plain int. + var count atomic.Int64 for _, project := range repos { wg.Add(1) @@ -508,7 +511,7 @@ func exitGolc(code int) { // Analysis functions for different repository types // Analysis functions for Bitbucket Cloud -func analyseBitCRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *int) { +func analyseBitCRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64) { p := project.(getbibucket.ProjectBranch) var excludeExtensions []string @@ -551,7 +554,7 @@ func analyseBitCRepo(project interface{}, DestinationResult string, platformConf } // Analysis functions for Bitbucket DC -func analyseBitSRVRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, trimmedURL string, spin *spinner.Spinner, results chan int, count *int) { +func analyseBitSRVRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, trimmedURL string, spin *spinner.Spinner, results chan int, count *atomic.Int64) { p := project.(getbibucketdc.ProjectBranch) var excludeExtensions []string @@ -573,7 +576,7 @@ func analyseBitSRVRepo(project interface{}, DestinationResult string, platformCo } // Analysis functions for GitHub -func analyseGithubRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *int) { +func analyseGithubRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64) { p := project.(getgithub.ProjectBranch) var excludeExtensions []string @@ -596,7 +599,7 @@ func analyseGithubRepo(project interface{}, DestinationResult string, platformCo } // Analysis functions for GitLab -func analyseGitlabRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *int) { +func analyseGitlabRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64) { p := project.(getgitlab.ProjectBranch) var excludeExtensions []string @@ -619,7 +622,7 @@ func analyseGitlabRepo(project interface{}, DestinationResult string, platformCo performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) } -func analyseAzurebRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *int) { +func analyseAzurebRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64) { p := project.(getazure.ProjectBranch) var excludeExtensions []string @@ -641,7 +644,7 @@ func analyseAzurebRepo(project interface{}, DestinationResult string, platformCo } // Perform repository analysis (common logic) -func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spinner.Spinner, results chan int, count *int, excludeExtension []string, excludePaths []string, folderKeywords []string, fileNamePatterns []string, ResultByFile bool, ResultAll bool) { +func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spinner.Spinner, results chan int, count *atomic.Int64, excludeExtension []string, excludePaths []string, folderKeywords []string, fileNamePatterns []string, ResultByFile bool, ResultAll bool) { // Always use a consistent filename pattern so downstream parsing works across platforms. // Format: Result_____ // The double-underscore field separator keeps `_` free to appear inside any component @@ -698,7 +701,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin spin.Stop() logger.Errorf(errorMessageRepo+"%v", err) recordSkip(err.Error()) - *count++ + count.Add(1) results <- 1 return } else { @@ -718,7 +721,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin }() //gc.Run() - //*count++ + //count.Add(1) if ResultAll { @@ -726,7 +729,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin fmt.Print("\n") logger.Errorf("❌ Error during analysis with ByAll = true: %v", err) recordSkip(fmt.Sprintf("analysis error: %v", err)) - *count++ + count.Add(1) results <- 1 return } @@ -742,7 +745,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin fmt.Print("\n") logger.Errorf("❌ Error initializing GCloc for ByFile = false: %v", err) recordSkip(fmt.Sprintf("analysis error: %v", err)) - *count++ + count.Add(1) results <- 1 return } @@ -751,7 +754,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin fmt.Print("\n") logger.Errorf("❌ Error during analysis with ByFile = false: %v", err) recordSkip(fmt.Sprintf("analysis error: %v", err)) - *count++ + count.Add(1) results <- 1 return } @@ -761,7 +764,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin fmt.Print("\n") logger.Errorf("❌ Error during analysis: %v", err) recordSkip(fmt.Sprintf("analysis error: %v", err)) - *count++ + count.Add(1) results <- 1 return } @@ -771,7 +774,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin // so it runs on success and on every early-return error path alike. golocParams.Cloned = false spin.Stop() - logger.Infof("\r\t\t\t\t✅ %d The repository <%s> has been analyzed\n", *count, params.RepoSlug) + logger.Infof("\r\t\t\t\t✅ %d The repository <%s> has been analyzed\n", count.Add(1), params.RepoSlug) // Send result through channel results <- 1 } @@ -796,7 +799,7 @@ func AnalyseReposListBitSRV(DestinationResult string, platformConfig map[string] for i, v := range repolist { repoInterfaces[i] = v } - return AnalyseReposList(DestinationResult, platformConfig, repoInterfaces, func(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *int) { + return AnalyseReposList(DestinationResult, platformConfig, repoInterfaces, func(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64) { analyseBitSRVRepo(project, DestinationResult, platformConfig, trimmedURL, spin, results, count) }) } @@ -869,7 +872,7 @@ func saveFileAnalysisResult(destDir, org string, dirs []string) error { /* ---------------- Analyse Directory ---------------- */ // analyseDirectory runs the goloc analysis for a single directory entry. -func analyseDirectory(dir string, ResultByFile, ResultAll bool, fileexclusionEX, extexclusion, folderKeywords, fileNamePatterns []string, destDir string, count *int) { +func analyseDirectory(dir string, ResultByFile, ResultAll bool, fileexclusionEX, extexclusion, folderKeywords, fileNamePatterns []string, destDir string, count *atomic.Int64) { params := goloc.Params{ Path: dir, ByFile: ResultByFile, @@ -920,8 +923,7 @@ func analyseDirectory(dir string, ResultByFile, ResultAll bool, fileexclusionEX, return } - logger.Infof("\t✅ %d The directory <%s> has been analyzed\n", *count, dir) - *count++ + logger.Infof("\t✅ %d The directory <%s> has been analyzed\n", count.Add(1), dir) } // runGlocPasses executes either a dual-pass (ResultAll) or single-pass analysis. @@ -967,7 +969,8 @@ func AnalyseReposListFile(Listdirectorie, fileexclusionEX []string, extexclusion var wg sync.WaitGroup wg.Add(len(Listdirectorie)) - count := 1 + // Shared across the per-directory goroutines below; atomic to avoid a data race. + var count atomic.Int64 for _, Listdirectories := range Listdirectorie { go func(dir string) { diff --git a/golc_workdir_test.go b/golc_workdir_test.go index 30c31b1..5b5979b 100644 --- a/golc_workdir_test.go +++ b/golc_workdir_test.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "sync/atomic" "testing" "time" @@ -59,7 +60,7 @@ func TestPerformRepoAnalysisCloneCleanup(t *testing.T) { } spin := spinner.New(spinner.CharSets[35], 100*time.Millisecond) results := make(chan int, 1) - count := 0 + var count atomic.Int64 performRepoAnalysis(params, dest, spin, results, &count, nil, nil, nil, nil, false, false) <-results @@ -120,7 +121,7 @@ func TestPerformRepoAnalysisLocalDir(t *testing.T) { } spin := spinner.New(spinner.CharSets[35], 100*time.Millisecond) results := make(chan int, 1) - count := 0 + var count atomic.Int64 performRepoAnalysis(params, dest, spin, results, &count, nil, nil, nil, nil, false, false) @@ -137,7 +138,7 @@ func TestPerformRepoAnalysisLocalDir(t *testing.T) { func TestAnalyseDirectoryLocal(t *testing.T) { src := writeSourceDir(t) dest := t.TempDir() - count := 1 + var count atomic.Int64 // Should analyse the directory in place without panicking or deleting it. analyseDirectory(src, false, false, nil, nil, nil, nil, dest, &count) @@ -160,7 +161,7 @@ func TestAnalyseReposListSingleThreaded(t *testing.T) { repolist := []interface{}{"repo-a", "repo-b"} var processed int - stub := func(_ interface{}, _ string, _ map[string]interface{}, _ *spinner.Spinner, results chan int, count *int) { + stub := func(_ interface{}, _ string, _ map[string]interface{}, _ *spinner.Spinner, results chan int, count *atomic.Int64) { processed++ results <- 1 } @@ -182,3 +183,38 @@ func TestAnalyseReposListSingleThreaded(t *testing.T) { t.Errorf("stub processed %d repos, want %d", processed, len(repolist)) } } + +// TestAnalyseReposListConcurrentCountNoRace exercises the multithreaded dispatch +// path with many repos so several worker goroutines run concurrently and all touch +// the shared progress counter. Under `go test -race` this fails if that counter is +// not synchronized — it is the regression guard for the count data race fixed by +// switching to atomic.Int64. +func TestAnalyseReposListConcurrentCountNoRace(t *testing.T) { + dest := t.TempDir() + cfg := map[string]interface{}{ + "Multithreading": true, + "NumberWorkerRepos": float64(4), // < len(repolist) => concurrency == Workers + "Workers": float64(8), + } + repolist := make([]interface{}, 50) + for i := range repolist { + repolist[i] = "repo" + } + + var processed atomic.Int64 + // The stub stands in for performRepoAnalysis: it both bumps and reads the shared + // counter concurrently, the exact access pattern that used to race on a plain int. + stub := func(_ interface{}, _ string, _ map[string]interface{}, _ *spinner.Spinner, results chan int, count *atomic.Int64) { + _ = count.Add(1) + processed.Add(1) + results <- 1 + } + + n := AnalyseReposList(dest, cfg, repolist, stub) + if n != len(repolist) { + t.Errorf("AnalyseReposList returned %d, want %d", n, len(repolist)) + } + if got := processed.Load(); got != int64(len(repolist)) { + t.Errorf("stub processed %d repos, want %d", got, len(repolist)) + } +} diff --git a/webui.go b/webui.go index 12a9259..c0511a2 100644 --- a/webui.go +++ b/webui.go @@ -1861,7 +1861,11 @@ function gatherConfig() { cfg.Workers = parseInt(document.getElementById('adv-workers').value) || 10; cfg.NumberWorkerRepos = cfg.Workers; const ctRaw = document.getElementById('adv-cloneTimeout'); - cfg.CloneTimeout = ctRaw ? (parseInt(ctRaw.value, 10) || 0) : 15; + // Empty/blank field falls back to the default (15); an explicit "0" still disables + // the deadline. Treating a cleared field as 0 would silently turn off the very + // hang-protection this setting exists for. + const ctVal = ctRaw ? ctRaw.value.trim() : ''; + cfg.CloneTimeout = ctVal === '' ? 15 : (parseInt(ctVal, 10) || 0); cfg.WorkDir = document.getElementById('adv-workDir').value.trim(); cfg.FileExclusion = ''; const ext = document.getElementById('adv-extExclusion').value.trim(); From 3337a456556c6bda59a107bb57e5319b514e8363 Mon Sep 17 00:00:00 2001 From: fabio-gos-sonarsource Date: Tue, 30 Jun 2026 10:09:08 +0200 Subject: [PATCH 3/3] refactor(golc): bundle analysis options into a struct (fix S107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SonarCloud flagged two MAJOR "too many parameters" (go:S107) smells on the PR's new code: performRepoAnalysis (11 params) and analyseDirectory (9 params). The parameter counts were unchanged from main — they only entered the PR's new-code scope because the previous commit retouched those signature lines for the atomic counter — but the gate blocks the merge, and they are legitimate readability smells, so fix them properly. Introduce an analysisOptions struct bundling the exclusions, keyword/ name filters, and report-mode flags. performRepoAnalysis now takes (params, dest, spin, results, count, opts) = 6 params; analyseDirectory takes (dir, opts, destDir, count) = 4. The five analyse*Repo callers and AnalyseReposListFile build the struct; behavior is unchanged. AnalyseReposListFile itself (8 params) is intentionally left untouched: it is pre-existing and not in the PR's new-code scope, so editing its signature would needlessly pull a new S107 into scope. Verified: go build (webui, resultsall), go vet, and go test -race -tags golc all pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- golc.go | 105 +++++++++++++++++++++++++++++++++---------- golc_workdir_test.go | 6 +-- 2 files changed, 84 insertions(+), 27 deletions(-) diff --git a/golc.go b/golc.go index 8c28192..7bab057 100644 --- a/golc.go +++ b/golc.go @@ -550,7 +550,14 @@ func analyseBitCRepo(project interface{}, DestinationResult string, platformConf WorkDir: getWorkDir(platformConfig), CloneTimeout: getCloneTimeout(platformConfig), } - performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) + performRepoAnalysis(params, DestinationResult, spin, results, count, analysisOptions{ + ExcludeExtensions: excludeExtensions, + ExcludePaths: excludePath, + FolderKeywords: folderKeywords, + FileNamePatterns: fileNamePatterns, + ResultByFile: platformConfig["ResultByFile"].(bool), + ResultAll: platformConfig["ResultAll"].(bool), + }) } // Analysis functions for Bitbucket DC @@ -572,7 +579,14 @@ func analyseBitSRVRepo(project interface{}, DestinationResult string, platformCo WorkDir: getWorkDir(platformConfig), CloneTimeout: getCloneTimeout(platformConfig), } - performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) + performRepoAnalysis(params, DestinationResult, spin, results, count, analysisOptions{ + ExcludeExtensions: excludeExtensions, + ExcludePaths: excludePath, + FolderKeywords: folderKeywords, + FileNamePatterns: fileNamePatterns, + ResultByFile: platformConfig["ResultByFile"].(bool), + ResultAll: platformConfig["ResultAll"].(bool), + }) } // Analysis functions for GitHub @@ -595,7 +609,14 @@ func analyseGithubRepo(project interface{}, DestinationResult string, platformCo WorkDir: getWorkDir(platformConfig), CloneTimeout: getCloneTimeout(platformConfig), } - performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) + performRepoAnalysis(params, DestinationResult, spin, results, count, analysisOptions{ + ExcludeExtensions: excludeExtensions, + ExcludePaths: excludePath, + FolderKeywords: folderKeywords, + FileNamePatterns: fileNamePatterns, + ResultByFile: platformConfig["ResultByFile"].(bool), + ResultAll: platformConfig["ResultAll"].(bool), + }) } // Analysis functions for GitLab @@ -619,7 +640,14 @@ func analyseGitlabRepo(project interface{}, DestinationResult string, platformCo WorkDir: getWorkDir(platformConfig), CloneTimeout: getCloneTimeout(platformConfig), } - performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) + performRepoAnalysis(params, DestinationResult, spin, results, count, analysisOptions{ + ExcludeExtensions: excludeExtensions, + ExcludePaths: excludePath, + FolderKeywords: folderKeywords, + FileNamePatterns: fileNamePatterns, + ResultByFile: platformConfig["ResultByFile"].(bool), + ResultAll: platformConfig["ResultAll"].(bool), + }) } func analyseAzurebRepo(project interface{}, DestinationResult string, platformConfig map[string]interface{}, spin *spinner.Spinner, results chan int, count *atomic.Int64) { @@ -640,11 +668,30 @@ func analyseAzurebRepo(project interface{}, DestinationResult string, platformCo WorkDir: getWorkDir(platformConfig), CloneTimeout: getCloneTimeout(platformConfig), } - performRepoAnalysis(params, DestinationResult, spin, results, count, excludeExtensions, excludePath, folderKeywords, fileNamePatterns, platformConfig["ResultByFile"].(bool), platformConfig["ResultAll"].(bool)) + performRepoAnalysis(params, DestinationResult, spin, results, count, analysisOptions{ + ExcludeExtensions: excludeExtensions, + ExcludePaths: excludePath, + FolderKeywords: folderKeywords, + FileNamePatterns: fileNamePatterns, + ResultByFile: platformConfig["ResultByFile"].(bool), + ResultAll: platformConfig["ResultAll"].(bool), + }) +} + +// analysisOptions bundles the per-repo analysis knobs (exclusions, keyword/name +// filters, and the report-mode flags) so the worker functions can pass them as a +// single value instead of a long parameter list. +type analysisOptions struct { + ExcludeExtensions []string + ExcludePaths []string + FolderKeywords []string + FileNamePatterns []string + ResultByFile bool + ResultAll bool } // Perform repository analysis (common logic) -func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spinner.Spinner, results chan int, count *atomic.Int64, excludeExtension []string, excludePaths []string, folderKeywords []string, fileNamePatterns []string, ResultByFile bool, ResultAll bool) { +func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spinner.Spinner, results chan int, count *atomic.Int64, opts analysisOptions) { // Always use a consistent filename pattern so downstream parsing works across platforms. // Format: Result_____ // The double-underscore field separator keeps `_` free to appear inside any component @@ -653,13 +700,13 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin outputFileName := fmt.Sprintf("Result_%s__%s__%s", params.ProjectKey, params.RepoSlug, params.MainBranch) golocParams := goloc.Params{ Path: params.PathToScan, - ByFile: ResultByFile, - ByAll: ResultAll, - ExcludePaths: excludePaths, - ExcludeExtensions: excludeExtension, + ByFile: opts.ResultByFile, + ByAll: opts.ResultAll, + ExcludePaths: opts.ExcludePaths, + ExcludeExtensions: opts.ExcludeExtensions, IncludeExtensions: []string{}, - FolderKeywords: folderKeywords, - FileNamePatterns: fileNamePatterns, + FolderKeywords: opts.FolderKeywords, + FileNamePatterns: opts.FileNamePatterns, OrderByLang: false, OrderByFile: false, OrderByCode: false, @@ -676,7 +723,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin WorkDir: params.WorkDir, CloneTimeout: params.CloneTimeout, } - if ResultAll { + if opts.ResultAll { golocParams.ByFile = true } @@ -723,7 +770,7 @@ func performRepoAnalysis(params RepoParams, DestinationResult string, spin *spin //gc.Run() //count.Add(1) - if ResultAll { + if opts.ResultAll { if err := gc.Run(); err != nil { fmt.Print("\n") @@ -872,16 +919,18 @@ func saveFileAnalysisResult(destDir, org string, dirs []string) error { /* ---------------- Analyse Directory ---------------- */ // analyseDirectory runs the goloc analysis for a single directory entry. -func analyseDirectory(dir string, ResultByFile, ResultAll bool, fileexclusionEX, extexclusion, folderKeywords, fileNamePatterns []string, destDir string, count *atomic.Int64) { +// opts carries the exclusions/keyword filters and report-mode flags (ExcludePaths +// holds the file-exclusion list for the File platform). +func analyseDirectory(dir string, opts analysisOptions, destDir string, count *atomic.Int64) { params := goloc.Params{ Path: dir, - ByFile: ResultByFile, - ByAll: ResultAll, - ExcludePaths: fileexclusionEX, - ExcludeExtensions: extexclusion, + ByFile: opts.ResultByFile, + ByAll: opts.ResultAll, + ExcludePaths: opts.ExcludePaths, + ExcludeExtensions: opts.ExcludeExtensions, IncludeExtensions: []string{}, - FolderKeywords: folderKeywords, - FileNamePatterns: fileNamePatterns, + FolderKeywords: opts.FolderKeywords, + FileNamePatterns: opts.FileNamePatterns, OrderByLang: false, OrderByFile: false, OrderByCode: false, @@ -897,7 +946,7 @@ func analyseDirectory(dir string, ResultByFile, ResultAll bool, fileexclusionEX, Cloned: false, Repopath: "", } - if ResultAll { + if opts.ResultAll { params.ByFile = true } @@ -919,7 +968,7 @@ func analyseDirectory(dir string, ResultByFile, ResultAll bool, fileexclusionEX, }(gc.Repopath) } - if err := runGlocPasses(gc, params, ResultAll); err != nil { + if err := runGlocPasses(gc, params, opts.ResultAll); err != nil { return } @@ -972,10 +1021,18 @@ func AnalyseReposListFile(Listdirectorie, fileexclusionEX []string, extexclusion // Shared across the per-directory goroutines below; atomic to avoid a data race. var count atomic.Int64 + opts := analysisOptions{ + ExcludeExtensions: extexclusion, + ExcludePaths: fileexclusionEX, + FolderKeywords: folderKeywords, + FileNamePatterns: fileNamePatterns, + ResultByFile: ResultByFile, + ResultAll: ResultAll, + } for _, Listdirectories := range Listdirectorie { go func(dir string) { defer wg.Done() - analyseDirectory(dir, ResultByFile, ResultAll, fileexclusionEX, extexclusion, folderKeywords, fileNamePatterns, destDir, &count) + analyseDirectory(dir, opts, destDir, &count) }(Listdirectories) } diff --git a/golc_workdir_test.go b/golc_workdir_test.go index 5b5979b..21b4ef7 100644 --- a/golc_workdir_test.go +++ b/golc_workdir_test.go @@ -62,7 +62,7 @@ func TestPerformRepoAnalysisCloneCleanup(t *testing.T) { results := make(chan int, 1) var count atomic.Int64 - performRepoAnalysis(params, dest, spin, results, &count, nil, nil, nil, nil, false, false) + performRepoAnalysis(params, dest, spin, results, &count, analysisOptions{}) <-results entries, _ := os.ReadDir(work) @@ -123,7 +123,7 @@ func TestPerformRepoAnalysisLocalDir(t *testing.T) { results := make(chan int, 1) var count atomic.Int64 - performRepoAnalysis(params, dest, spin, results, &count, nil, nil, nil, nil, false, false) + performRepoAnalysis(params, dest, spin, results, &count, analysisOptions{}) if got := <-results; got != 1 { t.Errorf("expected 1 result signal, got %d", got) @@ -141,7 +141,7 @@ func TestAnalyseDirectoryLocal(t *testing.T) { var count atomic.Int64 // Should analyse the directory in place without panicking or deleting it. - analyseDirectory(src, false, false, nil, nil, nil, nil, dest, &count) + analyseDirectory(src, analysisOptions{}, dest, &count) if _, err := os.Stat(src); err != nil { t.Errorf("source dir should be preserved, stat err=%v", err)