Reduce scan startup time in large repositories - #301
Draft
lelia wants to merge 7 commits into
Draft
Conversation
find_files() started a separate recursive rglob traversal for every expanded manifest pattern, so a scan re-walked each root once per pattern and only filtered excluded directories after descending into them. Replace that with one os.walk() per scan root: - Expand and case-fold all active patterns once, then match in memory. - Prune excluded directories, including .git, before descending. - Reject non-manifests on the basename alone (one set lookup plus one compiled glob alternation) before building a relative path or running a path match. - Cache supported manifest patterns per Core instance, but only when the API lookup succeeds, so a transient failure does not pin the run to the smaller local fallback pattern set. - Emit INFO durations for organization setup, pattern retrieval and discovery, with files/directories visited, directories pruned and manifests found. Matching behaviour is unchanged apart from intentionally excluding .git metadata. Adds parity tests against the previous rglob implementation for every built-in ecosystem and pattern, covering case-insensitivity, brace expansion, nested patterns, dot-directories, exclusions, inclusions, symlinks, excluded ecosystems, multiple roots, sorting and deduplication, plus an opt-in benchmark that asserts old/new result equality on a synthetic large-monorepo fixture. Ref: CE-379
Git.__init__() ran `git fetch --all` on every invocation, pulling every remote branch and tag before changed-file detection even began. Resolve commit and branch metadata locally instead, and for pull-request comparisons prefer refs already present in the checkout, fetching a single base or head ref only when it is missing. Also recognises Buildkite's native BUILDKITE_COMMIT, BUILDKITE_BRANCH, BUILDKITE_PULL_REQUEST and BUILDKITE_PULL_REQUEST_BASE_BRANCH so Buildkite jobs can calculate a complete base-to-head changed-file range without mapping their environment onto GitHub Actions variable names. Buildkite is checked before GitHub because some pipelines deliberately export GitHub-compatible variables. Adds INFO durations for Git initialisation, changed-file detection and each fetch, including the ref requested and why. Existing GitHub Actions, GitLab CI, Bitbucket Pipelines and local behaviour is preserved; tests cover local-ref preference, absence of an unconditional fetch, the targeted-fetch fallback, all four CI providers, and non-PR and detached-HEAD execution. Ref: CE-379
`--scm github` read its configuration solely from GITHUB_* variables, so Buildkite users had to shim every one of them to get PR comments. Fall back to Buildkite's own variables when the GITHUB_* equivalents are absent: PR number, commit, branch, checkout path, commit message, build creator, and owner/repository parsed from BUILDKITE_REPO (preferring the pipeline repository over a contributor's fork). Explicit GITHUB_* and PR_NUMBER values still take priority, and GitHub Enterprise remains configurable via GITHUB_API_URL. A running Buildkite PR build maps to the supported `synchronize` comment path, and a non-PR build maps to `push`, so event routing is unchanged. Default-branch detection requires an actual branch name rather than treating two unset variables as a match, which would otherwise mark any build as the default branch and overwrite the repository baseline. Ref: CE-379
The --sub-path routing pre-check walked every selected path to decide whether any manifests existed, then discarded the result so scan creation walked the same paths again. Retain and reuse it. Apply --excluded-ecosystems before the pre-check rather than after, so every find_files() call in a run sees the same ecosystem filter. Add an INFO duration for CLI run registration, and replace the "No Manifest files changed" line with wording that describes the decision being made: no supported manifest was detected in the changed-file set, so a full report is created. Scan-routing semantics are unchanged. Ref: CE-379
Bumped via .hooks/sync_version.py so __init__.py, pyproject.toml and uv.lock stay in sync, and moved the changelog entry under a 2.6.5 heading. Ref: CE-379
lelia
added
publish-preview
|
🚀 CLI preview published: pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.6.5.dev3165619303101TestPyPI's package index can take several minutes to expose a newly uploaded version. |
The publish-docker job downloads the built wheel to ./dist, but the build step omitted `context`, so docker/build-push-action used its default Git context. Buildx then cloned the repository as the build context, where ./dist does not exist, and `COPY dist/socketsecurity-*.whl` failed with "lstat /dist: no such file or directory". Set `context: .` so the build uses the workspace the artifact was downloaded into. This also makes the job's existing trust boundary hold as documented: the context is now the default-branch checkout rather than the pull-request ref, so Dockerfile.preview is read from trusted code and the pull request still enters the image only through the built wheel. Pre-existing; the TestPyPI half of the workflow is unaffected.
lelia
added
publish-docker-preview
|
🐳 Docker preview published: This mutable tag is only created when a Docker preview is explicitly requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scans spent most of their wall-clock time in local setup rather than in the API. Two phases dominated:
rglob()traversal for every expanded manifest pattern, so each scan root was re-walked once per pattern. Excluded directories were filtered only after the walk had already descended into them, and.gitwas never excluded.git fetch --allunconditionally, pulling every remote branch and tag before changed-file detection began.This replaces both with single-pass equivalents, adds native Buildkite support, and adds stage-level timing so the remaining cost of a scan is visible in INFO logs.
Changes
Single-pass manifest discovery. One
os.walk()per scan root instead of one traversal per pattern. Patterns are expanded and case-folded once, excluded directories (now including.git) are pruned before descent, and non-manifests are rejected on their basename before any path work happens.Targeted Git fetching.
git fetch --allis gone. Commit and branch metadata resolve locally, pull-request comparisons prefer refs already in the checkout, and a single base or head ref is fetched only when it's missing.Native Buildkite context. Buildkite's own environment variables are recognised for commit, branch and pull-request range, and
--scm githubfalls back to them for comment context. Buildkite jobs no longer need to shim GitHub Actions variable names.Observability. INFO-level durations for CLI registration, organization setup, Git initialization, each fetch and the ref it requested, changed-file detection, pattern retrieval, and manifest discovery — plus discovery counters (files and directories visited, directories pruned, manifests found).
Behavior
Existing scan routing, output, and CLI behaviour are unchanged across GitHub Actions, GitLab CI, Bitbucket Pipelines, Buildkite and local execution. This does not skip full scans when no manifest changed.
Two intentional differences:
.gitis excluded from manifest discovery. Manifest results are otherwise equivalent to the previous implementation.*.egg-infois in the default exclusion set but the previous exact-name matcher never applied it, so*.egg-infodirectories were being walked and their contents discovered. They are now excluded as intended.One log line changed for accuracy:
No Manifest files changedis nowNo supported manifest detected in the changed-file set; creating a full Socket report. The count-onlyTotal files found: Nline is retained for existing log consumers.Validation
rglobimplementation for every built-in ecosystem and pattern, covering case-insensitivity, brace expansion, nested patterns, dot-directories, exclusions, inclusions, symlinks, excluded ecosystems, multiple roots, sorting and deduplication. Asserts each root is walked exactly once and that excluded directories are pruned before traversal.benchmarks/manifest_discovery.pyis an opt-in developer benchmark (not a timing assertion in the suite) that compares old and new discovery on a synthetic large-monorepo fixture and fails on any result mismatch. It demonstrates one tree walk rather than one walk per pattern; it deliberately makes no promise about a fixed production speedup.Still to do before this leaves draft: validate a prerelease build in an affected pipeline and compare the new stage timings against the reported baseline.
Ref: CE-379