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 updated this revision to Diff 28411.May 16 2017, 8:57 AM
cpm edited the summary of this revision. (Show Details)
  • Add support version 0.12.0 of configargparse
cpm edited the summary of this revision. (Show Details)May 16 2017, 9:00 AM
cpm updated this revision to Diff 28412.May 16 2017, 9:10 AM
cpm edited the summary of this revision. (Show Details)

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

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

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 updated this revision to Diff 28620.May 20 2017, 7:20 PM
cpm edited edge metadata.

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 updated this revision to Diff 28875.May 26 2017, 5:32 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.
cpm edited the summary of this revision. (Show Details)
cpm updated this revision to Diff 29906.Jun 21 2017, 11:20 AM
cpm retitled this revision from security/py-{acme,certbot}: update to 0.14.2 to security/py-{acme,certbot}: update to 0.15.0.
cpm edited the summary of this revision. (Show Details)
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 updated this revision to Diff 30019.EditedJun 24 2017, 8:15 AM
cpm edited edge metadata.
  • 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.