-
Notifications
You must be signed in to change notification settings - Fork 484
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
Fix dependency conflicts in pyproject.toml #1548
Conversation
This makes three changes to fix dependency resolution - Fixes a bug where the sphinx version is unresolvable, and pins it to 8.0.2 (the minimum version required by the minimum version of sphinx-autodoc-typehints) - Lowers the required version of `pefile` in order to allow us to use the `pyinstaller` version specified in the `dev` dependencies - Bumps the python version to 3.10 in order to support sphinx 8.0.2
I reverted the Python version bumps and found the highest versions of I also looked through the git history to see if there was a reason that we require such a high version of |
This lowers the required versions of sphinx and sphinx-autodoc-typehints to the highest versions that still support Python 3.8.
a1d8e1c
to
055c8db
Compare
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.
Thanks, I think this messes with a few too many things. Many of the requirements were artificially bumped up by the person pushing up to move to pyproject.toml, but I'm not sure there's good cause for blocking the older versions.
I also don't really want to fix on an old version of pefile because pyinstaller (used once and only by devs) has a performance issue for the one time we run it.
@@ -11,7 +11,7 @@ license = { text = "VSL" } | |||
dynamic = ["version"] | |||
|
|||
dependencies = [ | |||
"pefile>=2024.8.26", |
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.
This is the wrong way to fix the issue. I don't think pefile is at fault, I believe it's the way that pyinstaller used it, and pyinstaller is only used by devs to construct an exe (and it's only a performance degradation), so I think we bump pyinstaller down (it used to start at 6.5, but during the toml update it got bump to the latest version at that time).
@@ -48,8 +48,8 @@ test = [ | |||
|
|||
docs = [ | |||
"volatility3[dev]", | |||
"sphinx>=8.0.0,<7", |
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.
I've tested and it appears to builds with version 8 ok, so I'm not sure this needs to drop to 7, I think the higher version should bump to 9.
@@ -48,8 +48,8 @@ test = [ | |||
|
|||
docs = [ | |||
"volatility3[dev]", | |||
"sphinx>=8.0.0,<7", | |||
"sphinx-autodoc-typehints>=2.5.0,<3", |
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.
I assume this change is to work with the version of sphinx available? Again, I believe the lower bound was pulled up artificially when the pyproject.toml
inhereited the values from the requirements.txt
.
Could you please review #1558, and if you've got the ability to easily check the dependencies and outputs, check that everything's ok? I've left my comments for the reasons I took a different route in this PR, but feel free to point out problems in the new one, or reply here if you're not happy with any of the reasoning... Thanks! |
Closes #1546
This makes three changes to fix dependency resolution
8.0.2 (the minimum version required by the minimum version of
sphinx-autodoc-typehints)
pefile
in order to allow us to usethe
pyinstaller
version specified in thedev
dependenciesI know bumping the minimum Python version is not desirable, but I wasn't sure which
sphinx
/sphinx-autodoc-typehints
versions would be acceptable due to my unfamiliarity with our doc requirements. We can lower those if an acceptable version can be found that supports python 3.8. There isn't a way to specify different required Python versions for different sets of dependencies withinpyproject.toml
.