Handle "optional" dependencies with no extra #176
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
This test checks whether the program properly works around a bug in setuptools<68.2 where required dependencies with constraints get treated as if they listed as optional dependencies under an extra with an empty name. Previously, the only test that would have exposed this issue was the pytest distribution package test. In pre- and post-merge testing, we were only running that test with old versions of setuptools on Python 3.12, and under those particular circumstances the pytest distribution package test would be skipped, so the issue never showed up until we made a release. This test would allow us to catch it normally.
Versions of setuptools prior to 68.2.0 have an issue where they will
misrepresent dependencies that have a constraint but no extra as
optional dependencies with an empty extra string. For example, if
a setup.cfg file includes this:
[options]
install_requires =
importlib-metadata>=0.12;python_version<"3.8"
that dependency should be parsed as a required dependency on all Python
versions less than 3.8, but these old versions of setuptools parse it as
if it were this instead:
[options.extras_require]
=
importlib-metadata>=0.12;python_version<"3.8"
(with an empty key under options.extras_require). This causes test
failures on older versions of Python or anywhere else we have an old
version of setuptools to work with.
To address this, in this commit I'm changing the logic that parses
dependencies and optional dependencies so it adds anything associated
with an empty extra to the set of required dependencies. I also added
some logging statements that were helpful in diagnosing this error.
Previously, in pre- and post-merge testing, we would only test with the oldest allowed versions of our dependencies on Python 3.12. However, the pytest distribution package test gets skipped on Python 3.12, which means we're sacrificing an opportunity to detect issues that arise with pytest when using old versions of setuptools and friends. This commit also enables minimum dependency testing on Python 3.8, so we have a chance to find those errors before making a release.
We've had a long-standing issue on Python 3.12 where running our distribution package test suite on pytest would mess up the whole test session, due to conflicting versions of setuptools-scm being installed in the same environment. Well, it turns out this issue occurs on other versions of Python as well, not only 3.12. So I'm changing the condition in the code that we use to skip the pytest distribution package tests so that it will be true on any version of Python, not only 3.12+. This prevents test failures in CI.
sjlongland
approved these changes
Aug 28, 2024
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Versions of setuptools prior to 68.2 have an issue where they will misrepresent dependencies that have a constraint but no extra as optional dependencies with an empty extra string. For example, if a
setup.cfgfile includes this:that dependency should be parsed as a required dependency on all Python versions less than 3.8, but these old versions of setuptools parse it as if it were this instead:
(with an empty key under options.extras_require). This causes test failures on older versions of Python or anywhere else we have an old version of setuptools to work with.
To address this, in this PR I'm changing the logic that parses dependencies and optional dependencies so it adds anything associated with an empty extra to the set of required dependencies. I also added some logging statements that were helpful in diagnosing this error.
Additionally, I added a test (not a distribution package test) that reproduces the error when run with
setuptools<68.2, and I changed the pre-/post-merge testing workflow to run tests with old versions of our dependencies on Python 3.8, in addition to Python 3.12 which we were already doing. This should give us a better chance at catching issues like this in the future.Closes #175