Page MenuHomeFreeBSD

sysutils/py-mqttwarn - update to Python 3
ClosedPublic

Authored by dvl on May 1 2020, 4:48 PM.

Details

Summary

I can't find the 'binary' to install. I'm confused by the Python
cache directorie

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

dvl requested review of this revision.May 1 2020, 4:48 PM

I think I've solved this, since there is a pypi solution

koobs requested changes to this revision.Aug 8 2020, 1:12 AM
koobs added inline comments.
sysutils/py-mqttwarn/Makefile
14–15 ↗(On Diff #75598)

This can be dropped if it now supports versions other than 2.7

17–24 ↗(On Diff #75598)

Can we collapse these lines into a single RUN_DEPENDS with \ line endings

27 ↗(On Diff #75598)

Why does this need flavors?

sysutils/py-mqttwarn/files/patch-mqttwarn_core.py
8 ↗(On Diff #75598)

Is this an upstream unreleased patch or something we have done only downstream?

Comment in patch header is useful for our future selves about what this is for, and what if anything needs to be done to upstream it (where possible)

This revision now requires changes to proceed.Aug 8 2020, 1:12 AM
dvl marked 3 inline comments as done.

Suggestions from koobs

dvl marked an inline comment as done.

Comment in patch to say why

sysutils/py-mqttwarn/Makefile
27 ↗(On Diff #75598)

I cannot recall now. Removed.

sysutils/py-mqttwarn/files/patch-mqttwarn_core.py
8 ↗(On Diff #75598)

Only downstream. I'll send it upstream.

koobs requested changes to this revision.Aug 8 2020, 9:25 AM
koobs added inline comments.
sysutils/py-mqttwarn/files/patch-setup.py
13–16 ↗(On Diff #75602)

These (all) == need to be replaced with >=. == dependencies are extremely brittle and break every time a port dependency version changes.

https://wiki.freebsd.org/Python/PortsPolicy#Dependencies

This revision now requires changes to proceed.Aug 8 2020, 9:25 AM
dvl marked an inline comment as done.

Updates based on comments

sysutils/py-mqttwarn/files/patch-setup.py
13–16 ↗(On Diff #75602)

When I find: tests_require=extras['test'],

and that is defined as:

'test': [
    'pytest==4.6.7',
    'pytest-cov==2.8.1',
    'lovely.testlayers>=0.7.1',
    'tox==3.14.2',
],

Does this result:

TEST_DEPENDS= ${PYTHON_PKGNAMEPREFIX}pytest>0:devel/py-pytest \

${PYTHON_PKGNAMEPREFIX}pytest-cov>0:devel/py-pytest-cov \
${PYTHON_PKGNAMEPREFIX}tox>0:devel/py-tox

I could not locate lovely.testlayers in our tree.

13–16 ↗(On Diff #75602)

Some of these values are modified from the upstream source (i.e. lower versions) because that is what we have in the ports tree now and I find that they work.

After talking with upstream (https://github.com/jpmens/mqttwarn/issues/441)
I've removed some patches as unrequired.

specify 3.6+ in USES= python
add concurrent to USE_PYTHON - port installs /usr/local/bin/mqttwarn
Add OPTIONS based on extras in setup.py

koobs requested changes to this revision.Aug 9 2020, 3:06 AM
koobs added inline comments.
sysutils/py-mqttwarn/Makefile
25–26 ↗(On Diff #75611)

cov (coverage) and (tox) are upstream development related test dependencies, and not necessary for downstream testing.

  • Instead of depending on tox, depend on the thing that tox runs. In this case, only pytest [1]
  • To remove/override non-compulsory upstream development test dependencies, invoke pytest directly in the do-test target:
do-test:
		@cd ${WRKSRC} && ${PYTHON_CMD} -m pytest -v -rs -o addopts=

-o addopts= overrides test settings/arguments set in pytest.ini or setup.cfg:[pytest] sections (which adds --cov, requiring pytest-cov)

Upstream should move these 'non-compulsory' test dependencies to an extras_require[dev] section, that tox.ini then pulls in. This will leave extras_require[test] as purely compulsory test dependencies (pytest, and optional run time deps)

[1] https://github.com/jpmens/mqttwarn/blob/master/tox.ini#L8

28 ↗(On Diff #75611)

Ports should 'declare' the versions that a package supports and not unnecessarily restrict/limit them.

The package support 2.7. USES=python is appropriate here.

62–76 ↗(On Diff #75611)

All of these extras_require (optional deps) need to be switched from == to >= too.

Note: this does not guarantee that upstream tests with them, or that they will work.

Upstream needs to change their pinned (==) deps to >= (for sdist/setup.py/users) so that they are testing future versions of their dependencies before users get them. This also avoids conflicts between package versions downstream as well.

If they want to continue to pin versions, this can be done in requirements.txt files (unrelated to distribution), but this is not recommended either.

This revision now requires changes to proceed.Aug 9 2020, 3:06 AM
sysutils/py-mqttwarn/files/patch-setup.py
13–16 ↗(On Diff #75602)

I usually just add # <test dependency line> # to be ported until its ported

dvl marked 2 inline comments as done.

Don't be so specific with python
change test deps
change do-test

koobs requested changes to this revision.Aug 10 2020, 2:25 AM
koobs added inline comments.
sysutils/py-mqttwarn/Makefile
14–21 ↗(On Diff #75623)

RUN_DEPENDS <version-specs> should always match those in setup.py as closely as possible:

+    'six>=1.13.0',
+    'paho-mqtt>=1.4.0',
+    'jinja2>=2.10.1',
+    'attrs>=19.3.0',
+    'docopt>=0.6.2',
+    'requests>=2.22.0',
+    'future>=0.18.2',
+    'configparser>=3.5.3',
66 ↗(On Diff #75623)

is this all that's required? There's no Python mysql package required to wrap mysqlclient.so ?

This revision now requires changes to proceed.Aug 10 2020, 2:25 AM
dvl marked an inline comment as done.

Adjust RUN_DEPENDS to match setup.py patches

Add version for rrdtool

This revision is now accepted and ready to land.Aug 12 2020, 1:42 AM
This revision was automatically updated to reflect the committed changes.