Skip to content

feat: find a charm's Python dependencies from its build plugin for ops.testing - #2880

Open
tonyandrewmeyer wants to merge 13 commits into
canonical:mainfrom
tonyandrewmeyer:saddle/split-13-charm-requirements
Open

tonyandrewmeyer wants to merge 13 commits into
canonical:mainfrom
tonyandrewmeyer:saddle/split-13-charm-requirements

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Adds a private scenario._charm_requirements module that installs a charm's Python dependencies the way the build plugin in its charmcraft.yaml would, so that a charm in a multi-charm test can run in a virtual environment with its own dependencies rather than the test's. Nothing uses it yet (building and caching the environment is a later PR in the series).

  • Only the python and uv plugins are supported. The charm and reactive plugins are deprecated, and poetry is planned to be done when requested. A charm with no charmcraft.yaml is also an error, even if it has a uv.lock, because charmcraft can't build it either. So is a part using a plugin that isn't one of craft-parts' built-in ones, even next to a python or uv part, because it might install Python dependencies too. Every error is a private RuntimeError subclass, which will move onto the public error when there's a caller to raise it from.
  • Rather than reading requirements files itself, the module gives pip and uv the same arguments charmcraft does, from the same directory (the part's source and source-subdir). For the python plugin that's pip install --no-deps with the part's python-constraints, python-requirements and python-packages, in charmcraft's order, so relative paths and nested -r files resolve exactly as they do in a pack.
  • For the python plugin, the charm libraries' PYDEPS aren't included, because charmcraft's python plugin doesn't install them either (it installs with --no-deps and expects the requirements file to be complete). This was incorrect in the spec.
  • For the uv plugin, this runs uv export with the options charmcraft's uv plugin passes to uv sync (--no-dev, --no-editable, and the part's extras and groups), and --no-emit-project to match charmcraft's --no-install-project. It uses --locked, so a stale uv.lock is an error rather than being re-locked as charmcraft pack would.
  • ops, ops-scenario and ops-tracing are uninstalled afterwards, whatever the plugin: the charm's environment will use the test's own copies, so the versions always match. (ops-tracing pins an exact ops version, so a charm's locked copy wouldn't work with the test's ops.) Because of that, installing into the environment the tests are running in is refused.

The install tests build their own wheels, and the uv tests lock a charm with path dependencies, so they don't need an index or the network. They do need uv on PATH, as the test environments have.

tonyandrewmeyer and others added 6 commits November 9, 2026 18:34
Add a private module that reads a charm's dependencies the way
charmcraft pack would, from the python or uv plugin part in its
charmcraft.yaml: requirements files, packages, constraints and the
PYDEPS of its charm libraries, or uv export from its uv.lock with the
part's extras and groups. The result is one flat requirements list
with relative paths made absolute, and without ops or ops-scenario.

Nothing uses it yet: it is the dependency extraction for running a
charm in its own virtual environment in multi-charm tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on plugin does

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… finding charm dependencies

ops-tracing pins an exact ops version, so a charm's locked copy wouldn't work with the test's ops. It's now left out with ops and ops-scenario.

A charm with no charmcraft.yaml is now an error even if it has a uv.lock or poetry.lock, because charmcraft can't build it either.

The uv test charm is now a package with an editable path dependency, so that --no-emit-project and --no-editable are both tested, and the requirements-file test covers -e and a -f URL. The uv export output is parsed directly rather than through a temporary file, and the docstring gives pip's hash rule, as uv accepts a mix of hashed and unhashed requirements.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g charm dependencies

A charm that uses an extension, such as flask-framework, is now an error unless one of its own parts sets the python or uv plugin, because the extension adds parts that aren't in charmcraft.yaml. Previously it was reported as using the deprecated charm plugin.

An editable requirement for ops, ops-scenario or ops-tracing (`-e ...#egg=ops`) is now left out like the other forms. Short options with the value attached (`-rbase.txt`) are read like the spaced form, so their paths are made absolute. List options in the part that are a string are now an error rather than being read one character at a time.

The docstring says that a stale uv.lock is an error, unlike charmcraft, which uses the lockfile as it is. The uv tests no longer skip when uv isn't on PATH.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ments

Rather than flattening requirements files and rewriting relative paths, give pip the arguments that charmcraft's python plugin does (`--constraint=`, `--requirement=` and the `python-packages`), run from the part's build directory: its `source` and `source-subdir`. Hashes, `--require-hashes`, environment markers, nested and missing `-r` files, bare relative paths, paths with spaces and comment lines now behave as they do in charmcraft. `uv export` also runs in the build directory.

The new `install_charm_requirements` installs the requirements into a given environment, using `uv pip` if uv is on PATH and the test's pip otherwise, then uninstalls ops, ops-scenario and ops-tracing by distribution name, so the charm's own copies are removed however they're specified. `uv export` leaves them out with `--no-emit-package`, which also handles ops from a path source.

`charm_requirements` now returns the build directory and the install arguments rather than a flat list of requirements, and `read_requirements` is removed. Nothing uses the module yet.

Every part is checked, so a charm plugin part is an error wherever it is, and more than one python or uv part is an error rather than only the first being used. Any extension is an error, since charmcraft either rejects a customised charm part or adds its own alongside. A part whose source isn't a local directory is an error. The uv list options are checked before uv is looked for.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lass

- Remove spurious parentheses from autoclass directive for Event
  in ops-testing.rst, consistent with all other autoclass entries.
- Change _RequirementsError base from RuntimeError to Exception,
  consistent with other scenario error classes.
.. autoclass:: ops.testing.DeferredEvent
.. autoclass:: ops.testing.ErrorStatus
.. autoclass:: ops.testing.Event()
.. autoclass:: ops.testing.Event

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by.

@tonyandrewmeyer
tonyandrewmeyer marked this pull request as ready for review November 10, 2026 04:48
@dwilding

Copy link
Copy Markdown
Contributor

Before I start reviewing this in detail, I wonder, does this deserve to be a standalone tool?

@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

Before I start reviewing this in detail, I wonder, does this deserve to be a standalone tool?

Do you have other use-cases in mind? (Beyond OP089's one.)

@dwilding

Copy link
Copy Markdown
Contributor

Do you have other use-cases in mind? (Beyond OP089's one.)

No, I was just curious 🙂

@hpidcock

Copy link
Copy Markdown
Member

@chapman-von-monty review this PR.

@chapman-von-monty

Copy link
Copy Markdown

Verdict: Request changes

P2 — Avoid exposing credentials and unbounded subprocess output in dependency-resolution errors. In testing/src/scenario/_charm_requirements.py (pip path around lines 121–124 and uv path around 306–314), exceptions include raw pip arguments and subprocess stderr. Requirements may contain credential-bearing direct URLs, and stderr can include those URLs or arbitrarily large output. This can leak secrets into logs and produce unwieldy diagnostics.

Please sanitize or omit sensitive arguments/output and bound any retained stderr before including it in the raised error. Add targeted tests verifying credential-bearing URLs are not present in errors and long stderr is truncated or otherwise bounded.

The existing PR comment is an unrelated drive-by; this is the substantive review.

tonyandrewmeyer and others added 7 commits November 14, 2026 00:38
A part with a plugin outside the known lists was only mentioned when no
Python part was found, so next to a python or uv part it was silently
ignored, though it might install Python dependencies too. It's now
refused either way, and the list of plugins that don't is craft-parts'
built-in set rather than eight of them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment

Installing removes ops, ops-scenario and ops-tracing afterwards, so
being handed the test's own interpreter would remove the test suite's
ops. Environments are compared by sys.prefix, because a virtual
environment's interpreter is usually a symlink to the base one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
charmcraft's python plugin installs with --no-deps and then runs pip
check, which is what catches a requirements file that's missing a
dependency. It runs before ops is removed, which deliberately leaves
dependencies missing. The uv plugin doesn't check, as charmcraft's
doesn't, and its export leaves out ops, which locked packages may need.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
charmcraft's experimental monorepo mode mounts the charm's whole git
repository, and its documentation has charms set source: ../.. with a
source-subdir back to the charm. That already worked here; it's now
tested. A source outside the repository is never available to
charmcraft, so it's refused rather than having pip run there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pip writes some failures, such as pip check's, to stdout, so without uv
an error could say only that the command failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chapman-von-monty

Copy link
Copy Markdown

Follow-up review at head d07080b70aa81662442183cc80cca2ba8e3ea188: the previously reported P2 remains unresolved—subprocess errors can expose raw pip arguments and unbounded output. I found no additional issues in the seven-commit update. See my earlier review for the requested fix and regression tests: #2880 (comment)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants