Skip to content

Commit

Permalink
Make argparse dependency unconditional. (#2249)
Browse files Browse the repository at this point in the history
The primary motivation is to avoid a branch, giving bugs one fewer place to hide. But, as a bonus, more people get a more bugfixed version of argparse. (To use the example from the argparse docs, people stuck on Python 3.2.3 can get bugfixes that made it into the stdlib only in 3.2.4.)
  • Loading branch information
erikrose authored and bmw committed Mar 9, 2017
1 parent 6669b95 commit 8f10103
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
3 changes: 1 addition & 2 deletions acme/setup.py
Expand Up @@ -8,6 +8,7 @@

# Please update tox.ini when modifying dependency version requirements
install_requires = [
'argparse',
# load_pem_private/public_key (>=0.6)
# rsa_recover_prime_factors (>=0.8)
'cryptography>=0.8',
Expand All @@ -30,8 +31,6 @@
# Keep in sync with conditional_requirements.py.
if sys.version_info < (2, 7):
install_requires.extend([
# only some distros recognize stdlib argparse as already satisfying
'argparse',
'mock<1.1.0',
])
else:
Expand Down
3 changes: 1 addition & 2 deletions setup.py
Expand Up @@ -36,6 +36,7 @@ def read_file(filename, encoding='utf8'):
# https://github.com/pypa/pip/issues/988 for more info.
install_requires = [
'acme=={0}'.format(version),
'argparse',
# We technically need ConfigArgParse 0.10.0 for Python 2.6 support, but
# saying so here causes a runtime error against our temporary fork of 0.9.3
# in which we added 2.6 support (see #2243), so we relax the requirement.
Expand All @@ -58,8 +59,6 @@ def read_file(filename, encoding='utf8'):
# Keep in sync with conditional_requirements.py.
if sys.version_info < (2, 7):
install_requires.extend([
# only some distros recognize stdlib argparse as already satisfying
'argparse',
'mock<1.1.0',
])
else:
Expand Down

1 comment on commit 8f10103

@pva
Copy link

@pva pva commented on 8f10103 Apr 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit and analogous in acmes breaks certbot. I receive error:

Traceback (most recent call last):
File "/usr/lib/python-exec/python2.7/certbot", line 6, in
from pkg_resources import load_entry_point
File "/usr/lib64/python2.7/site-packages/pkg_resources/init.py", line 3015, in
@_call_aside
File "/usr/lib64/python2.7/site-packages/pkg_resources/init.py", line 2999, in _call_aside
f(*args, **kwargs)
File "/usr/lib64/python2.7/site-packages/pkg_resources/init.py", line 3028, in _initialize_master_working_set
working_set = WorkingSet._build_master()
File "/usr/lib64/python2.7/site-packages/pkg_resources/init.py", line 655, in _build_master
ws.require(requires)
File "/usr/lib64/python2.7/site-packages/pkg_resources/init.py", line 963, in require
needed = self.resolve(parse_requirements(requirements))
File "/usr/lib64/python2.7/site-packages/pkg_resources/init.py", line 849, in resolve
raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'argparse' distribution was not found and is required by certbot

We should not depend on things that exist in standard library. Please, set correct sys.version_info around this dependency.

Similar issues in other projects: Gallopsled/pwntools#598
https://bugs.launchpad.net/keystone/+bug/1441083

Please sign in to comment.