Skip to content

Commit 961692b

Browse files
bomly-guyclaude
andauthored
fix(pip): scope direct dependencies, name project roots, diagnose old pip (#338)
* fix(pip): scope direct dependencies, name project roots, diagnose old pip Three pip-detector fixes reported against 0.18.0. Every installed distribution was reported as a direct dependency (#273). `pip inspect` reports a flat installed set with no `requested_by` field, so the `len(RequestedBy) == 0` clause was always true and hung every package off the root. Edges now come from each distribution's `requires_dist`, and the direct set from the names the project's requirements files declare plus the installer's REQUESTED marker. Packages left without a parent are re-parented onto the root so the graph keeps a single root. requirements.txt projects rendered as the literal `root` in the manifest tree (#272). Roots are now named after `pyproject.toml`'s project name, the subproject directory, the scanned repository, or the project directory — in that order. Bomly's own `bomly-git-*` clone directories are skipped so remote scans stay deterministic. `pip inspect` failed opaquely on machines whose ambient pip predates 22.2 (#274), which is where the command was added; the venv inherits that pip from `python -m venv`. Bomly now upgrades pip inside its own isolated temp venv, and when that cannot run, fails with the pip version and the minimum named instead of `exit status 1`. Documented the requirement in the pip and pipenv prerequisites, and corrected the network-behavior row that claimed `pip inspect` never reaches the network. Regenerated the three affected smoke goldens (root rename only). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(pip): scope declarations to manifests and diagnose old pip without upgrading Review follow-ups. [P1] The direct-dependency set came from declaredPythonDependencies, which also reads poetry.lock, uv.lock, and Pipfile.lock. Since the pip-inspect path is shared by Poetry, uv, and Pipenv, their fallback would have marked every locked transitive as direct — #273 all over again for those detectors. Direct declarations now come from a dedicated collector that reads only hand-authored files: requirements.txt / -dev / .in, the dependency tables of pyproject.toml (PEP 621, PEP 735, Poetry, uv), and the Pipfile. requirements.lock is excluded for the same reason as the other lockfiles. declaredPythonDependencies keeps its looser semantics for the tool-package filter, which asks a different question. [P1] Dropped the pip self-upgrade. Installing or upgrading a package manager violates the repository's non-negotiable contract, and routing it through base.install also appended the project's --install-arg values to the bootstrap. The detector now only diagnoses: it reads the venv's pip version before installing anything and fails with the version and the minimum named. The fallback still produces a graph, so the user sees an actionable notice instead of "exit status 1". Also from review: reattach orphans by root-reachability rather than parent count, so a requires_dist cycle cannot strand a component; wrap the declaration-collection error with context; log the pip --version subprocess at DEBUG with binary, args, and working dir; and correct the documented root-naming chain, which omitted the subproject and repository fallbacks. Goldens re-verified against the pinned smoke repos: unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 10dc47e commit 961692b

23 files changed

Lines changed: 914 additions & 149 deletions

dev-docs/ARCHITECTURE.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,14 @@ Python build-tool inspection can accidentally read the wrong environment: `pip i
377377

378378
The smoke/benchmark Python targets rely on the fast-paths for determinism: `scan-python-poetry` uses the committed `poetry.lock` fast-path, and `scan-python-pip` commits a `requirements.lock`. The venv isolation remains the correctness backstop for real-world pip projects scanned without a committed lock.
379379

380+
Two consequences of inspecting an environment rather than reading a manifest:
381+
382+
- **Shape is reconstructed, not reported.** `pip inspect` returns a flat installed set. Edges come from each distribution's `requires_dist`; the direct set comes from what the project declares by name, plus the installer's `REQUESTED` marker for what those declarations cannot name (`-r` includes, environments populated by another front-end). Anything the root cannot reach is re-parented onto it, so a `requires_dist` cycle cannot strand a component. Treating every installed distribution as direct — the pre-fix behavior — reported pure transitives as top-level dependencies.
383+
- **Declarations are hand-authored files only.** `directPythonDeclarations` reads requirements files, the dependency tables of `pyproject.toml`, and the `Pipfile`. Lockfiles — including `requirements.lock` — are excluded on purpose: they record the resolved closure, where a transitive package appears exactly like a direct one. Since the inspect path is shared by pip, Poetry, uv, and Pipenv, admitting `poetry.lock` or `uv.lock` here would recreate the all-direct bug for those detectors. This is a deliberately narrower question than `declaredPythonDependencies`, which asks only whether a package belongs to the project at all (used to keep declared tool packages).
384+
- **`pip inspect` needs pip ≥ 22.2.** `python -m venv` seeds the virtualenv from the ambient interpreter, so an old system Python yields a venv that cannot inspect itself. Bomly diagnoses this before the install and fails with the pip version and the requirement named, which surfaces through the fallback notice instead of a bare `exit status 1`. It does not upgrade pip: installing package managers is out of scope (see Non-Negotiables), and mutating the environment before resolution would add an unpinned network write to every scan.
385+
386+
Python roots are named, not labeled `root`: `pyproject.toml`'s project name, else the subproject directory, else the scanned repository, else the project directory. Bomly's own `bomly-git-*` clone directories are never used — they are random per run, which would make remote-target output non-deterministic.
387+
380388
### Decision: detector fallbacks are loud, annotated degradations
381389

382390
When a build-tool-primary detector (Maven, Gradle, Go, …) cannot produce a graph and its `sdk.FallbackDetector` succeeds instead, the scan silently loses transitive resolution — the exact capability the primary exists for. That degradation is now first-class provenance rather than a Debug-only log line:

docs/DETECTORS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ resolution.
7373
| --- | --- | --- |
7474
| Lockfile parser | `npm-detector`, `pnpm-detector`, `bundler-detector`, `composer-detector`, `nuget-detector`, `github-actions-detector`, SBOM ingest | None — pure file parse |
7575
| Lockfile-first hybrid | `cargo-detector`, `poetry-detector`, `uv-detector` | None when the lockfile is present; the build-tool fallback uses `--locked` / `--no-sync` to stay offline |
76-
| `pip inspect` | `pip-detector`, `pipenv-detector` | None — reads the local Python environment |
76+
| `pip inspect` | `pip-detector`, `pipenv-detector` | None when a lockfile is present; otherwise **may download** while populating the isolated environment it inspects |
7777
| Build-tool primary | `go-detector`, `maven-detector`, `gradle-detector`, `sbt-native-detector` | **May download** uncached artifacts during normal resolution |
7878

7979
The build-tool-primary detectors invoke commands you would already run locally (`go list`, `mvn dependency:tree`, `gradle dependencies`, `sbt dependencyTree`). Whether they hit the network is a property of those tools and your local cache state, not a Bomly choice. To keep these scans fully offline, pre-warm the local cache (`go mod download`, `mvn dependency:go-offline`, etc.) or commit a lockfile when the ecosystem supports one.

docs/detectors/ecosystems/python/pip.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,20 @@ Bomly uses this chain when it finds `pip` evidence.
3131

3232
The isolated virtualenv lives under the OS temp directory and is keyed by the project path. Bomly recreates it before installing so stale packages cannot leak into the scan, then inspects that venv directly. Ambient site-packages are never accepted as the project graph.
3333

34+
Direct vs. transitive comes from what your project declares by name — requirements files, the dependency tables in `pyproject.toml`, the `Pipfile` — plus the installer's `REQUESTED` marker. Lockfiles never count as declarations: they record the whole resolved closure, so reading them here would make every transitive package look direct. Everything else in the environment is wired under its parents using each distribution's `Requires-Dist` metadata.
35+
36+
A `requirements.txt` project carries no name of its own, so the graph's root node is named from the first of: `pyproject.toml`'s declared project name, the subproject directory (in a recursive monorepo scan), the scanned repository name (for a `--url` target), or the project directory. Bomly's own `bomly-git-*` clone directories are never used — they are random per run.
37+
3438
## Network behavior
3539

3640
`requirements.lock` parsing is offline and does not execute Python.
3741

38-
⚠️ Without `requirements.lock`, Bomly installs into its isolated temp virtualenv so the graph is accurate. That can download packages from PyPI or the indexes declared by your install args.
42+
⚠️ Without `requirements.lock`, Bomly installs into its isolated temp virtualenv so the graph is accurate. That can download packages from PyPI or the indexes declared by your install args. The only commands Bomly runs there are `python -m venv`, `pip --version`, your requirements install, and `pip inspect` — it never installs or upgrades pip itself.
3943

4044
## Prerequisites
4145

4246
- `python` on `PATH` with `pip` installed (`python -m pip --version` must work).
47+
- **pip 22.2 or newer**, which is the first release with `pip inspect`. `python -m venv` seeds the virtualenv with the ambient interpreter's pip, so an old system Python (macOS ships 3.9 with pip 21.x) produces a venv that cannot inspect itself. Bomly checks this before installing and fails with the pip version and this requirement named, rather than a bare exit status; the detector chain then falls back, so the scan still completes with a reduced graph. Bomly never installs or upgrades pip for you — fix it with `python -m pip install --upgrade pip` for that interpreter, or put a newer Python on `PATH`.
4348
- A `requirements.txt`, `requirements-dev.txt`, `requirements.in`, `requirements.lock`, or any `*requirements*.txt` file in the scan path acts as the evidence pattern that triggers `pip-detector`.
4449

4550
## `--install-first`

docs/detectors/ecosystems/python/pipenv.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ The inspected graph is accepted only when declared packages are present in the v
4444
- A valid Pipenv-managed virtualenv, **or**
4545
- A committed `Pipfile.lock`.
4646
- For `--install-first`: `pipenv` on `PATH`.
47+
- Inspecting a Pipenv virtualenv needs **pip 22.2 or newer** inside it (`pip inspect` does not exist before that). Older environments fall back to parsing `Pipfile.lock`.
4748

4849
## `--install-first`
4950

0 commit comments

Comments
 (0)