-
Notifications
You must be signed in to change notification settings - Fork 574
build: move to PEP 621 project #5379
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
base: main
Are you sure you want to change the base?
Conversation
all project metadata was moved to pyproject.toml
38049df to
fbc4a02
Compare
fbc4a02 to
da38201
Compare
|
Thanks for working on this! I don't have any brain power to give it this week (this is probably the only day i'm going to do any PR review) but hopefully one of the other maintainers can step in. I've at least set the checks to run for now. |
|
I have replaced We also used I understand that this will probably slow down approval of this PR but it's a net positive (for ~1 year until 3.10 goes out of support). |
|
Hm, I'm really not sure why Linux Tests (3.9) failed while building wheel 🤔 @terriko, can you rerun the tests? I think I'm finished with this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Move the project metadata and configuration to PEP 621 in pyproject.toml, remove legacy setup tooling, and update tests, docs, and CI to align with the new structure.
- Adopt PEP 621: add project metadata, dependencies, scripts, and tool configs to pyproject.toml.
- Remove legacy files (setup.py, dev-requirements.txt, bandit.conf, .coveragerc, .mypy.ini) and update CI/docs accordingly.
- Update tests to read dependencies from pyproject.toml and modernize context manager syntax.
Reviewed Changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | New single source of truth for project metadata, dependencies, and tool configs. |
| test/test_requirements.py | Read dependencies from pyproject.toml; refactor helpers; add guards and typing. |
| test/test_package_list_parser.py | Grouped context managers using parentheses for readability. |
| test/test_output_engine.py | Grouped patches with parentheses for readability. |
| test/test_extractor.py | Grouped patches with parentheses for readability. |
| CONTRIBUTING.md | Update install instructions and Bandit usage to reflect pyproject.toml. |
| .pre-commit-config.yaml | Point Bandit to pyproject.toml and ensure bandit[toml] is available. |
| .github/workflows/* | Replace requirements.txt references with pyproject.toml and install extras via .[dev]. |
| doc/how_to_guides/* | Update cache keys from requirements.txt to pyproject.toml. |
| setup.py, dev-requirements.txt, bandit.conf, .coveragerc, .mypy.ini | Removed legacy files no longer needed with PEP 621. |
| MANIFEST.in | Drop requirements.txt from sdist since dependencies are now in pyproject.toml. |
| cve_bin_tool/config.py | Switch to tomllib/tomli consistently and simplify TOML loading. |
Comments suppressed due to low confidence (1)
test/test_requirements.py:118
- String identity comparison is used here; use equality instead. Replace is with == to ensure correct behavior when file is a distinct but equal string.
if file is HTML_DEP_CSV:
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Black config was not changed but we have different result compared to main branch this is because Black determines supported Python versions from pyproject.toml in absence of the `requires-python` section it is determined on the per file basis now that we have full-fledged pyproject.toml Black follows our config more strictly
now that dev-dependencies were moved to pyproject.toml our old script would no longer work dropping this means that there might be some short period of time when pre-commit config will be ahead of pyproject.toml but this shouldn't be a big deal we could rewrite it to update new location but because we now have Dependabot enabled pyproject.toml should stay updated through Dependabot
toml library is abandoned, last release was in 2020 tomli library is the community recognized replacement change required because toml can't parse our pyproject.toml
02ef0eb to
6e88a7b
Compare
|
Rerun unfortunately didn't help. I built locally using 3.9 because I picked it as the lowest currently supported version, worked fine. Also worked fine in my fork (log). The only thing that comes to mind now is caching, I see it's enabled for setup-python action with |
|
Latest hypothesis: This is just an intermittent error with GAD data source. I noticed that all tests run on push to branch and on pull request and on the same commit in PR in my fork test for Python 3.11 failed on push but succeeded for PR (same code in both) which strongly suggests it's an issue with GAD. Rerun fixed on push check as well. Copying relevant part here: This is related to this part of code base I guess: GAD data source. The error is quite obscure to be honest, maybe there's a better way to catch it during build. @terriko, yet another rerun (only failed jobs) should fix it I hope. Initial hypothesis: Cache key is taken from @terriko, can you delete the 3.9 cache and then rerun the test on this PR, please? It can be done from this page. That should be done in quick succession so that some other PR/commit doesn't recreate the 3.9 cache with a non-PEP 517 build. Current cache has key P.S. I also wonder if deleting cache and letting it be recreated from this PR will break it for all other PRs/commits... In this case, we can once again delete the cache or just merge this ASAP |
fixes #1595
TODO list for this PR
[ ] Move doc dependencies to dependency-group:let's make it simple and keep out of the scope of this PR. Simply moving dependencies to the mainproject.tomlmight be a bad idea because they conflict with Python version specified in it. Namely our doc dependencySphinx==8.2.3requires Python>=3.11 but cve-bin-tool itself only requires >=3.9. This results in unresolvable dependencies. In practice docs are completely separate project, we can addpyproject.tomlfor docs only later on.test/test_requirements.py, it expectsrequirements.txtto be present[ ] update how we generateas far as I understand, these are handcrafted, please correct me if I'm wrongrequirements.csv