bring in python devshell with Nix #94
Merged
Conversation
This file contains 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
flokli
force-pushed
the
python-devshell
branch
3 times, most recently
from
c4ac079 to
b7ae62a
Compare
This seems unused Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
Make sure there's a `python` with the needed dependencies in the Nix devshell, and, removing the need to pip install whenever we enter the shell. Signed-off-by: Florian Klink <[email protected]>
These need to be added to the devshell too. Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
This allows pytest to be able to import the packages in this repo. Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
pytest is invoked inside the dev-env, we can rely on the python there to have the right dependencies. The processes now fail to import the "sbomnix" python module itself, because they don't have "." in PYTHONPATH. Signed-off-by: Florian Klink <[email protected]>
flokli
force-pushed
the
python-devshell
branch
2 times, most recently
from
0f220d7 to
dec4e6c
Compare
These invocations all call a command and ensure there's a nonzero exit code, which is already done by setting `check=True`. Signed-off-by: Florian Klink <[email protected]>
These scripts import `sbombix` on their own, so it makes sense for their PYTHONPATH to include REPOROOT. Ideally, we should be importing functions from the entrypoint itself, rather than spawning a new process, but that's left for a followup. Signed-off-by: Florian Klink <[email protected]>
This seems unused so far. Signed-off-by: Florian Klink <[email protected]>
`vulnxscan --triage` shells out to repology_cli again, which doesn't work - both of them should be independent entrypoints, rather than calling each other. We should probably move all the repology-related code into a library directory, and call it from both vulnxscan --triage and repology_cli. Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
`nix flake check` already runs that check. Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
We still keep the Makefile target (but rename it to format), as it interactively reformats the source code. Signed-off-by: Florian Klink <[email protected]>
flokli
force-pushed
the
python-devshell
branch
from
dec4e6c to
aeb5e45
Compare
These are moved to flake checks. Keep reuse, because its "annotate" command is used from the dev env. Move black out of the python.withPackages. We don't need to import black as a python module, but only want to execute the binary. Signed-off-by: Florian Klink <[email protected]>
pylint actually relies on being able to import the same python libraries as the code its linting, so we need to expose the python with all runtime deps of sbomnix, and also provide it inside that check. Signed-off-by: Florian Klink <[email protected]>
Not using the pip install mechanism exposed another problem: sbomdb.py actually imports a private module from reuse. Reuse however is not packaged as an element of the python package set, but as an application, causing it to be not in PYTHONPATH of the python composed with python3.withPackages. Fix this by packaging it as a python package, not an application. This fixes the pylint unknown import error, or broken execution inside the devshell for this package. Signed-off-by: Florian Klink <[email protected]>
Signed-off-by: Florian Klink <[email protected]>
When executing from inside the devshell, there's no sbomnix in $PATH, but the entrypoints can be called directly. Signed-off-by: Florian Klink <[email protected]>
flokli
force-pushed
the
python-devshell
branch
from
aeb5e45 to
7f6fa45
Compare
flokli
commented
Nov 15, 2023
Signed-off-by: Brian McGee <[email protected]>
flokli
force-pushed
the
python-devshell
branch
from
5b282d0 to
fa376d9
Compare
henrirosten
reviewed
Nov 15, 2023
henrirosten
approved these changes
Nov 15, 2023
This was referenced Nov 28, 2023
Merged
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.
This drops a
python.withPackages (…)with the devshell, and reconfigures pytest to add.toPYTHONPATH.This removes the additional
pip installrun whenever the shell is entered, so all targets and scripts related to managing that can be removed.pytestnow uses thatpythonfor all third-party python dependencies - which required exposingpyrate-limiterandrequests-limiter.Some of the tests were previously "integration-testing the flake outputs", introducing additional complexity and another nix eval - we run the tests inside the dev environment, so there's no need to do that.
If we really want to test the flake outputs still work, it could be a separate "smoketest" (maybe just inside GH actions).
pytestshould not execute Nix (at least where we can avoid it, which is bringing in the environment we already have).Changing to this approach exposed some smell - the entrypoints are not really thin entrypoints, rely on and execute other binaries in
$PATH, too.Some of this code should probably be moved to some library code, most pressingly,
vulnxscanshould not invokerepology_cli(which is another entrypoint), but instead repology-related code should be moved into a lib that can be used by both consumers.For now, I disabled these three (now failing) pytest tests. In nix-based builds, this doesn't matter and will still work, as we ship all entrypoints in a single
$out/bin.--
Long-term, we might want to decouple the "CLI parsing from process args and preparing result for presentation" part from the actually code that run. (Ideally by moving the "main logic" into lib code too).
Rather than starting another python interpreter and passing CLI args, we could then really test the individual primitives, which should allow more fine-grained testing than just "integration-testing" the whole CLI, which is what we do now.