Page MenuHomeFreeBSD

security/py-{acme,certbot}: update to 0.15.0
ClosedPublic

Authored by cpm on May 5 2017, 1:15 PM.

Details

Summary
security/py-{acme,certbot}: update to 0.15.0

Common:

- Update PORTVERSION and distinfo checksum (0.15.0)
- Update RUN/TEST dependencies.
- Remove unnecessary patches (applied upstream)

Changes:      https://github.com/certbot/certbot/blob/master/CHANGELOG.md#0150---2017-06-08

Approved by:	koobs (maintainer, py-certbot)
Diff Revision:	D10612
Test Plan
  • portlint -ac output looks fine.
  • poudriere testport builds fine on 10.3/i386, 10.3/amd64, 11.0/i386, 11.0/amd64, 12.0/i386 and 12.0/amd64.
  • make test has passed all tests fine.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cpm created this revision.May 5 2017, 1:15 PM
cpm edited the summary of this revision. (Show Details)
cpm edited the summary of this revision. (Show Details)May 16 2017, 8:57 AM
cpm updated this revision to Diff 28411.
  • Add support version 0.12.0 of configargparse
cpm edited the summary of this revision. (Show Details)May 16 2017, 9:00 AM
cpm edited the summary of this revision. (Show Details)May 16 2017, 9:10 AM
cpm updated this revision to Diff 28412.

Revert to previous diff. First I'll commit bug 219306 which includes configargparse fix.

cpm retitled this revision from security/py-{acme,certbot}: update to 0.14.0 to security/py-{acme,certbot}: update to 0.14.1.May 17 2017, 5:17 PM
cpm edited the summary of this revision. (Show Details)
cpm updated this revision to Diff 28476.

Update to 0.14.1

koobs requested changes to this revision.May 20 2017, 2:50 AM

Do tests work (for acme and certbot) without mock/nose installed on the system?

These requirements haven't been removed from setup.py (see below). Separately, they're listed in dev_extras, but they should be in tests_require if they are required (create upstream issue/pr), so that the test command can install the dependencies if they're not already installed.

dev_extras = [
    # Pin astroid==1.3.5, pylint==1.4.2 as a workaround for #289
    'astroid==1.3.5',
    'coverage',
    'ipdb',
    'nose',
    'pylint==1.4.2',  # upstream #248
    'tox',
    'twine',
    'wheel',
]

I see inconsistencies between RUN_DEPENDS and setup.py install_requires, such as (but not limited to):

  • 'cryptography>=0.7', # load_pem_x509_certificate (we say >=0.8)
  • 'parsedatetime>=1.3', # Calendar.parseDT (not in RUN_DEPENDS)
  • 'configobj', (not in RUN_DEPENDS)

Please clarify the details of the RUN_DEPENDS changes (and the differences to setup.py)

This revision now requires changes to proceed.May 20 2017, 2:50 AM
koobs edited the summary of this revision. (Show Details)May 20 2017, 2:51 AM
cpm edited edge metadata.May 20 2017, 7:20 PM
cpm updated this revision to Diff 28620.

security/py-acme:

  • Add devel/py-nose to TEST_DEPENDS
cpm added a comment.May 20 2017, 7:30 PM

Do tests work (for acme and certbot) without mock/nose installed on the system?
These requirements haven't been removed from setup.py (see below). Separately, they're listed in dev_extras, but they should be in tests_require if they are required (create upstream issue/pr), so that the test command can install the dependencies if they're not already installed.

dev_extras = [
    # Pin astroid==1.3.5, pylint==1.4.2 as a workaround for #289
    'astroid==1.3.5',
    'coverage',
    'ipdb',
    'nose',
    'pylint==1.4.2',  # upstream #248
    'tox',
    'twine',
    'wheel',
]

I see inconsistencies between RUN_DEPENDS and setup.py install_requires, such as (but not limited to):

  • 'cryptography>=0.7', # load_pem_x509_certificate (we say >=0.8)
  • 'parsedatetime>=1.3', # Calendar.parseDT (not in RUN_DEPENDS)
  • 'configobj', (not in RUN_DEPENDS)

Please clarify the details of the RUN_DEPENDS changes (and the differences to setup.py)

I checked the setup.py of the current distfile (0.14.1)

'ConfigArgParse>=0.9.3',
'configobj',
'cryptography>=0.7',  # load_pem_x509_certificate
'mock',
'parsedatetime>=1.3',  # Calendar.parseDT
cpm added a comment.May 20 2017, 7:37 PM

Upstream bumped cryptography requirement to >=0.8 for py-acme.

Have you seen other inconsistencies?

cpm updated this revision to Diff 28623.May 20 2017, 10:20 PM

Update diff after r441335 and r441336

cpm edited the summary of this revision. (Show Details)May 20 2017, 10:21 PM
cpm retitled this revision from security/py-{acme,certbot}: update to 0.14.1 to security/py-{acme,certbot}: update to 0.14.2.May 26 2017, 5:32 PM
cpm edited the summary of this revision. (Show Details)
cpm updated this revision to Diff 28875.
cpm retitled this revision from security/py-{acme,certbot}: update to 0.14.2 to security/py-{acme,certbot}: update to 0.15.0.Jun 21 2017, 11:20 AM
cpm edited the summary of this revision. (Show Details)
cpm updated this revision to Diff 29906.
cpm added a comment.Jun 21 2017, 11:22 AM

py-acme

Ran 365 tests in 9.550s (OK)

py-certbot

Ran 687 tests in 18.176s (OK)

koobs requested changes to this revision.Jun 23 2017, 12:00 AM
In D10612#233788, @cpm wrote:

py-acme
Ran 365 tests in 9.550s (OK)
py-certbot
Ran 687 tests in 18.176s (OK)

Can you run these tests (make test in port dir) in poudriere (using testport -i to enter jail) to test whether all dependencies are satisfied at run/test time in a clean environment.

Reason is my original comment on the following dependency inconsistencies did not seem to be addressed:

'cryptography>=0.7', # load_pem_x509_certificate (we say >=0.8)
'parsedatetime>=1.3', # Calendar.parseDT (not in RUN_DEPENDS)
'configobj', (not in RUN_DEPENDS)

Your reply only mentioned acme

This revision now requires changes to proceed.Jun 23 2017, 12:00 AM
cpm edited the summary of this revision. (Show Details)Jun 23 2017, 11:59 AM
cpm added a comment.Jun 23 2017, 12:04 PM
In D10612#233788, @cpm wrote:

py-acme
Ran 365 tests in 9.550s (OK)
py-certbot
Ran 687 tests in 18.176s (OK)

Can you run these tests (make test in port dir) in poudriere (using testport -i to enter jail) to test whether all dependencies are satisfied at run/test time in a clean environment.
Reason is my original comment on the following dependency inconsistencies did not seem to be addressed:

'cryptography>=0.7', # load_pem_x509_certificate (we say >=0.8)
'parsedatetime>=1.3', # Calendar.parseDT (not in RUN_DEPENDS)
'configobj', (not in RUN_DEPENDS)

Your reply only mentioned acme

In D10612#233788, @cpm wrote:

py-acme
Ran 365 tests in 9.550s (OK)
py-certbot
Ran 687 tests in 18.176s (OK)

Can you run these tests (make test in port dir) in poudriere (using testport -i to enter jail) to test whether all dependencies are satisfied at run/test time in a clean environment.
Reason is my original comment on the following dependency inconsistencies did not seem to be addressed:

'cryptography>=0.7', # load_pem_x509_certificate (we say >=0.8)
'parsedatetime>=1.3', # Calendar.parseDT (not in RUN_DEPENDS)
'configobj', (not in RUN_DEPENDS)

Please, check out py-acme/setup.py

All test cases ran successfully using poudriere jails.

Your reply only mentioned acme

cpm added a comment.EditedJun 23 2017, 12:08 PM

devel/py-mock was moved from TEST_DEPENDS to RUN_DEPENDS.
devel/py-nose remains as TEST dependency.

So, all changes were applied according to the corresponding setup.py

koobs added a comment.Jun 24 2017, 7:52 AM

mock has always been a spurious (incorrect) install_requires (RUN_DEPENDS), it should be in tests_require (TEST_DEPENDS) instead. See: https://github.com/certbot/certbot/pull/1735

cpm edited edge metadata.EditedJun 24 2017, 8:15 AM
cpm updated this revision to Diff 30019.
  • Move devel/py-mock dependency from RUN_DEPENDS to TEST_DEPENDS.
cpm added a comment.Jun 24 2017, 8:17 AM

mock has always been a spurious (incorrect) install_requires (RUN_DEPENDS), it should be in tests_require (TEST_DEPENDS) instead. See: https://github.com/certbot/certbot/pull/1735

Good catch!

Thanks Kubilay

cpm edited the summary of this revision. (Show Details)Jun 24 2017, 8:18 AM
cpm edited the summary of this revision. (Show Details)Jun 24 2017, 8:21 AM
koobs accepted this revision.Jun 24 2017, 8:40 AM

LGTM.

Commit log message is missing Reviewed by: <foo>

This revision is now accepted and ready to land.Jun 24 2017, 8:40 AM
This revision was automatically updated to reflect the committed changes.