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

Slide this branch forward to a place equivalent to the release of 0.10.0 #1

Open
wants to merge 19 commits into
base: python2.6-0.9.3
Choose a base branch
from

Conversation

erikrose
Copy link

Oddly enough, the release looks to have been cut from here, not the 0.10.0 tag. This fixes certbot/certbot#2243.

Remaining to be done:

  • Test with the old le-auto against this

bw2 and others added 19 commits May 13, 2015 00:53
…arse to configargparse module namespace.

Enhancement bw2#9 - added constructor arg to add a 'dump config' option.
Fix for bw2#14 - refactored how unknown config file options are processed. Added 'ignore_unknown_config_file_keys' constructor arg as a better-named version of the 'allow_unknown_config_file_keys' arg which is now deprecated.
Enhancement bw2#15 - added 'auto_env_var_prefix' constructor arg.
The parse_config_file and convert_parsed_args_to_config_file_contents
methods were moved to a generic ConfigFileParser class, which can be
overridden in the ArgumentParser constructor using the
config_file_parser option.

Addresses bw2#21
The very convoluted code for showing examples was greatly simplified,
there is little benefit in showing true args instead of pseudo-args. For
advanced parsers, the full syntax can't be expressed in a simple
description, so we should also refer to the "upstream" documentation for
completeness.
Support for Python2.6 (without argparse tests)
Refactor config file parsing
From https://docs.python.org/dev/library/argparse.html#dest: Any internal - characters will be converted to _ characters to make sure the string is a valid attribute name
Ensure auto_env_var_prefix converts dash to underscore like ArgParse
@erikrose
Copy link
Author

Here's the test I ran. First, reproduce the bug:

% virtualenv le
% source le/bin/activate
% pip install -r py26reqs.txt letsencrypt==0.1.0 letsencrypt-apache==0.1.0
% pip install -r py26reqs.txt -U letsencrypt letsencrypt-apache
% letsencrypt --version
An unexpected error occurred:
VersionConflict: (ConfigArgParse 0.9.3 (/Users/erose/le/lib/python2.7/site-packages), Requirement.parse('ConfigArgParse>=0.10.0'))
Please see the logfile 'letsencrypt.log' for more details.

Then, replace py26reqs.txt in the 2nd pip call with a new file that points to this new branch instead:

% virtualenv le
% source le/bin/activate
% pip install -r py26reqs.txt letsencrypt==0.1.0 letsencrypt-apache==0.1.0
% pip install -r /Users/erose/Checkouts/letsencrypt/py26reqs-new.txt -U letsencrypt letsencrypt-apache
% letsencrypt --version
letsencrypt 0.2.0

The contents of the new file is…

# https://github.com/bw2/ConfigArgParse/issues/17
git+https://github.com/erikrose/[email protected]#egg=ConfigArgParse

That should match the behavior of the old branch once this is merged.

@kuba
Copy link
Owner

kuba commented Jan 22, 2016

$ cd /tmp
$ wget https://pypi.python.org/packages/source/C/ConfigArgParse/ConfigArgParse-0.10.0.tar.gz#md5=408ad7af06cd449420cecc19bee6f0c9
$ tar zxvf ConfigArgParse-0.10.0.tar.gz
$ git clone https://github.com/bw2/ConfigArgParse
$ cd ConfigArgParse
$ git checkout 0.10.0
$ diff -r . /tmp/ConfigArgParse-0.10.0
Only in /tmp/ConfigArgParse-0.10.0: ConfigArgParse.egg-info
Only in .: .git
Only in .: .gitignore
Only in /tmp/ConfigArgParse-0.10.0: PKG-INFO
Only in /tmp/ConfigArgParse-0.10.0: setup.cfg
diff -r ./setup.py /tmp/ConfigArgParse-0.10.0/setup.py
97a98
>         'Programming Language :: Python :: 2.6',
103,104c104,105
<   'Programming Language :: Python :: Implementation :: CPython',
<   'Programming Language :: Python :: Implementation :: PyPy',
---
>         'Programming Language :: Python :: Implementation :: CPython',
>         'Programming Language :: Python :: Implementation :: PyPy',
Only in /tmp/ConfigArgParse-0.10.0/tests: __init__.pyc
Only in /tmp/ConfigArgParse-0.10.0/tests: test_configargparse.pyc
Only in .: tox.ini
Only in .: .travis.yml

Something is not right. Could you verify, @erikrose? Which commit did you pick up? Unfortunately, I don't have time now to investigate further - probably we could get over with this PR, but lets be absolutely sure.

@erikrose
Copy link
Author

Yes, I tried to imply it above, but I wasn't clear enough. As your diff shows, the ConfigArgParse 0.10.0 release was not apparently made from the 0.10.0 tag. Instead, it seems to have been made from 0c29f30, which adds the "Programming Language :: Python :: 2.6" trove specifier and fixes the indentation of the CPython and PyPy ones. Thus, I branched from that, in order to arrive at an exact copy of the released 0.10.0 package.

@kuba
Copy link
Owner

kuba commented Jan 24, 2016

Apologies, I should have compared your fork! It looks OK:

$ git clone https://github.com/erikrose/ConfigArgParse
$ cd ConfigArgParse
$ git checkout call-0.10.0-0.9.3
$ diff -r . /tmp/ConfigArgParse-0.10.0
Only in /tmp/ConfigArgParse-0.10.0: ConfigArgParse.egg-info
Only in .: .git
Only in .: .gitignore
Only in /tmp/ConfigArgParse-0.10.0: PKG-INFO
diff -r ./README.rst /tmp/ConfigArgParse-0.10.0/README.rst
1,6d0
< .. note::
< 
<     Though this is stored under the python2.6-0.9.3 branch, this is now
<     actually a copy of ConfigArgParse 0.10.0. See
<     https://github.com/letsencrypt/letsencrypt/issues/2243 for more explanation.
< 
Only in /tmp/ConfigArgParse-0.10.0: setup.cfg
Only in /tmp/ConfigArgParse-0.10.0/tests: __init__.pyc
Only in /tmp/ConfigArgParse-0.10.0/tests: test_configargparse.pyc
Only in .: tox.ini
Only in .: .travis.yml

@erikrose
Copy link
Author

Since we missed you before you went on vacation, we'll land certbot/certbot#2248 temporarily and then roll it back when you return and get a chance to land this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants