Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

self-test: Install sigstore-python in separate virtualenv #181

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

jku
Copy link
Member

@jku jku commented Jan 13, 2025

This prevents sigstore-python dependencies from being accidentally used by the test suite.

Note that Makefile currently still installs everything in the same environment. Fixing this requires some additional work: either the documentation needs to include the use of venv when invoking the test suite manually (making the invocation even more complicated) or the sigstore-python-conformance entrypoint needs to somehow handle the virtual environment...

Fixes #180

@jku jku force-pushed the install-sigstore-in-venv branch 3 times, most recently from 038c264 to ec85a1d Compare January 13, 2025 14:00
This prevents sigstore dependencies from being accidentally
used by the test suite.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku force-pushed the install-sigstore-in-venv branch from ec85a1d to 9cce92f Compare January 13, 2025 14:26
@jku
Copy link
Member Author

jku commented Jan 13, 2025

I think the PR is now correct (if a little ugly -- the way we invoke the wrapper could be a lot more straightforward).

The failure in the selftest is correct: we now reproduce #178. We can merge #179 first, then this should start passing (since we pin protobuf-specs at that point).

@jku jku marked this pull request as ready for review January 13, 2025 14:31
Don't call two different things "sigstore-env"

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku changed the title self-test: Install sigstore in separate virtualenv self-test: Install sigstore-python in separate virtualenv Jan 13, 2025
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM!

Thinking out loud: one thing we could do to speed up our own CI/conformance consumers a bit is use uv throughout. But that's totally unrelated to these fixes themselves 🙂

@di
Copy link
Member

di commented Jan 13, 2025

Trying to make sure I understand the goal here: we want to install sigstore-python, but use the sub-dependencies specified in the requirements.txt here and not it's sub dependencies, correct? Wouldn't pip install sigstore --no-deps do this?

@jku
Copy link
Member Author

jku commented Jan 13, 2025

@di the goal is to install sigstore-python so that it does not poison the virtual environment for the test suite: The practical issue was that e.g. #178 does not appear in our self test but does break every user.

Current situation is:

  • sigstore-python is first installed globally (no venv)
  • the test suite is installed in its own venv but that finds the shared dependencies that have been installed globally

The already merged #179 does prevent this specific poisoning issue but the core problem remains: if sigstore-python and the test suite want to use different dependency versions, they should be able to.

This PR changes things so sigstore-python is installed in a separate venv. The only complication is that it makes the invocation a bit complicated (generated shell script calls the sigstore-python-conformance wrapper that calls "sigstore" binary): I'm pretty sure there is a simpler setup but it will require modifying the wrapper somehow

@jku
Copy link
Member Author

jku commented Jan 13, 2025

I agree generating the script is not pretty but I'll merge as is: I think it still does the right thing and we can improve after the action is working again.

see #185 for improvements

@jku jku merged commit c6d0e7a into sigstore:main Jan 13, 2025
5 checks passed
jku added a commit to jku/sigstore-conformance that referenced this pull request Jan 14, 2025
* self-test: Install sigstore in separate virtualenv

This prevents sigstore dependencies from being accidentally
used by the test suite.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

* selftest: Rename things

Don't call two different things "sigstore-env"

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

---------

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Co-authored-by: William Woodruff <william@trailofbits.com>
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.

self-test should install sigstore-python in a virtual env
3 participants