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

Remove pbr runtime dependency in favor of importlib.metadata? #839

Closed
cjolowicz opened this issue Mar 1, 2022 · 6 comments · Fixed by #1016
Closed

Remove pbr runtime dependency in favor of importlib.metadata? #839

cjolowicz opened this issue Mar 1, 2022 · 6 comments · Fixed by #1016
Labels
enhancement New feature or request

Comments

@cjolowicz
Copy link

cjolowicz commented Mar 1, 2022

Is your feature request related to a problem? Please describe.
This project depends on pbr to compute its __version__ attribute at runtime. For Python >= 3.8, this dependency could be dropped using importlib.metadata. For older Pythons, we could use the importlib_metadata backport.

Apart from reducing runtime dependencies and (arguably) following best practices, this switch would also prevent this package from breaking in environments without setuptools. pbr has an undeclared runtime dependency on setuptools, causing bandit to crash unconditionally on startup if setuptools is not installed. This is a blocker for tools like Nox creating virtualenvs without setuptools and wheels, which would be beneficial for the ecosystem at large, see wntrblm/nox#557. The bug report on pbr has not received a response in the last two years.

Describe the solution you'd like

Describe alternatives you've considered

  • Add a runtime dependency on setuptools

Additional context
There is additional context at wntrblm/nox#557 and pbr#1868899

@cjolowicz cjolowicz added the enhancement New feature or request label Mar 1, 2022
@ericwb
Copy link
Member

ericwb commented Mar 1, 2022

Pbr is also used in the docs build. So switching to importlib_metadata might not resolve everything.

@sigmavirus24
Copy link
Member

There's a lot of context on the psf discourse site and this org but the importlib family of libraries have not been stable and are still considered not stable by the maintainer by their own comments. Unfortunately setuptools has also taken in a mission of destabilizing the python community but that's no fault of pbr whose stability has far exceeded the other libraries mentioned here, not to mention the other features it provides.

@cjolowicz
Copy link
Author

cjolowicz commented Mar 4, 2022

Thanks @ericwb for taking a stab at this, and @sigmavirus24 for providing your thoughts.

To be clear, my goal here is to ensure that Bandit will continue to work when installed in an environment without setuptools. Adding setuptools to Bandit's dependencies would also solve this.

I'm aware of the controversy around importlib.metadata (and importlib.resources). Has importlib.metadata.version specifically been affected by incompatible changes in the past? The proposed dependency would be limited to this single str -> str function. As for future changes, this library is no longer marked as provisional as of Python 3.10.

I'll also note that there is now work under way in pbr to replace pkg_resources with importlib.metadata, see https://review.opendev.org/c/openstack/pbr/+/662035.

@cjolowicz
Copy link
Author

cjolowicz commented Mar 4, 2022

Here's the repro for this issue:

❯ virtualenv --no-setuptools venv
❯ . venv/bin/activate
❯ pip install bandit
❯ bandit --help
Traceback (most recent call last):
  File "/Users/cjolowicz/Code/github.com/PyCQA/bandit/venv/bin/bandit", line 5, in <module>
    from bandit.cli.main import main
  File "/Users/cjolowicz/Code/github.com/PyCQA/bandit/venv/lib/python3.10/site-packages/bandit/__init__.py", line 19, in <module>
    __version__ = pbr.version.VersionInfo("bandit").version_string()
  File "/Users/cjolowicz/Code/github.com/PyCQA/bandit/venv/lib/python3.10/site-packages/pbr/version.py", line 467, in version_string
    return self.semantic_version().brief_string()
  File "/Users/cjolowicz/Code/github.com/PyCQA/bandit/venv/lib/python3.10/site-packages/pbr/version.py", line 462, in semantic_version
    self._semantic = self._get_version_from_pkg_resources()
  File "/Users/cjolowicz/Code/github.com/PyCQA/bandit/venv/lib/python3.10/site-packages/pbr/version.py", line 439, in _get_version_from_pkg_resources
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'

Both tox and Nox are considering to create environments without setuptools and wheel by default.

@sigmavirus24
Copy link
Member

As for future changes, this library is no longer marked as provisional as of Python 3.10.

I'm not Google. In discussions of the impact of the very real instability, the maintainer who often merges changes without going through code review said they wouldn't hesitate to break the API in another minor version of Python (I'm paraphrasing) so regardless of what the stability is documented as, it's not the intent of the maintainer to uphold that. The backports are also insufficient as it provides no way to have one tool - which relies on the old API - and one - which relies on the new API - be installed together. The migration path was not considered and is actively being broken for tools that are otherwise stable.

If pbr wants to take on handling that nightmare they're welcome to. I'd rather bandit rely on something that hides that for us. I expect stevedore to hide entry-point loading for us to given the nightmare of that arena as well.

I believe we still support entry-point plugins and so even if we fix this one spot, setuptools not being installed will break other spots eventually unless stevedore already handled pkg_resources not being installed

@cjolowicz
Copy link
Author

I'm not Google.

I don't know what this relates to, but I'm going to bow out of this for now.

Please consider this issue a friendly heads up that Bandit will be affected if/when packaging tools stop provisioning virtualenvs with setuptools. It certainly wasn't my intent to push this project to adopt a dependency it doesn't want.

TomerFi added a commit to TomerFi/aioswitcher that referenced this issue Sep 6, 2022
ericwb added a commit to ericwb/bandit that referenced this issue Apr 7, 2023
The importlib module has a metadata.version to retrieve the package
version of the given module. This can be used in lieu of pbr for
gathering versioning. And since importlib is part of the base
Python package in 3.8 and greater, we can drop another dependency.

Closes PyCQA#839

Signed-off-by: Eric Brown <[email protected]>
Signed-off-by: Eric Brown <[email protected]>
ericwb added a commit that referenced this issue Jun 5, 2023
The importlib module has a metadata.version to retrieve the package
version of the given module. This can be used in lieu of pbr for
gathering versioning. And since importlib is part of the base
Python package in 3.8 and greater, we can drop another dependency.

Closes #839

Signed-off-by: Eric Brown <[email protected]>
Signed-off-by: Eric Brown <[email protected]>
Co-authored-by: Luke Hinds <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants