Drop wheel, setuptools and pytest from runtime dependencies - #320
Open
fwittreverce wants to merge 1 commit into
Open
Drop wheel, setuptools and pytest from runtime dependencies#320fwittreverce wants to merge 1 commit into
fwittreverce wants to merge 1 commit into
Conversation
None of the three is imported anywhere under `src/`. `c2pa.py` and `lib.py` import only the standard library; `build.py` — the `download-artifacts` console script — imports `requests` and, lazily, `toml`. Those two stay. They are also already classified correctly elsewhere in the repo: * `[build-system] requires` already lists `setuptools>=68.0.0` and `wheel`, so the build has what it needs and the runtime entries are duplicates. * `requirements-dev.txt` lists `wheel` and `setuptools` under "# Build dependencies" and `pytest` under "# Testing dependencies". * `.github/workflows/build.yml` installs pytest explicitly (`pip install pytest`, lines 285 and 377), so CI does not rely on the runtime declaration either. Removing them is therefore a no-op for this repo's own build and test paths, and it keeps three packages out of every consumer's production environment.
Collaborator
|
For further review and consideration, please make sure to sign the Adobe CLA. You will likely need to close and reopen the PR for the job to pass. |
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
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.
wheel,setuptoolsandpytestare declared in[project.dependencies], soevery consumer installs three build/test packages into production environments.
None of the three is imported anywhere under
src/.Evidence
Every import in the installed package, on
main(0.37.9):src/c2pa/c2pa.pysrc/c2pa/lib.pysrc/c2pa/__init__.pysrc/c2pa/build.pyrequests(line 16),toml(line 97, lazy)So
requestsandtomlare genuine runtime dependencies —build.pyis theinstalled
download-artifactsconsole script — and this PR leaves both alone.wheel,setuptoolsandpytestare imported by nothing.This repo already classifies all three correctly in three other places:
[build-system] requiresalready listssetuptools>=68.0.0andwheel,so the build has what it needs; the runtime entries are duplicates.
requirements-dev.txtlistswheelandsetuptoolsunder# Build dependenciesandpytestunder# Testing dependencies..github/workflows/build.ymlinstalls pytest explicitly(
pip install pytest, lines 285 and 377), so CI does not rely on the runtimedeclaration either.
The change is therefore a no-op for this repo's own build and test paths.
Why it is worth doing
They reach production images. In our worker,
uv export --frozen --no-dev—the resolver's production set — lists all three.
setuptoolsin particular hasa CVE history, so a security scan has to triage findings for packages the
application never imports.
They mask missing test dependencies downstream, silently. This is the one
that cost us time. Our own test extra was never actually installed —
uv syncdoes not install extras — and nobody noticed for seven weeks, because
pytestarrived through this dependency chain anyway. Our suite ran on a package no
manifest of ours declared, at a version nobody chose, and a fix we shipped in
that window was inert the whole time. A dependency audit is what eventually
flagged it.
One question, deliberately not in this diff
cryptographyis also unimported undersrc/— it appears only inexamples/and
tests/, and your ownrequirements.txtsays# only used in the training example. It looked like your call rather than mine: dropping it would stoppip install c2pa-pythonfrom giving a reader everything the signing examplesneed. Happy to extend the PR if you would rather it went too.