Page MenuHomeFreeBSD

devel/py-pyface: Update to 6.1.2
Needs ReviewPublic

Authored by kai on Sep 25 2019, 6:23 PM.

Details

Reviewers
Vladimir.Chukharev_gmail.com
koobs
Group Reviewers
Python
Summary
devel/py-pyface: Update to 6.1.2

- Introduce new default option QT5 [1] to let the port make use of the Qt5
  toolkit.  (Support for the wxPython toolkit is also available but was left
  out intentionally because it isn't available for Python 3, yet.)

- Also add a "do-test" target to make future QA easier while I'm here.

Changelog since 6.1.2:

https://github.com/enthought/pyface/blob/6.1.2/CHANGES.txt

PR: 239576
Submitted by: vladimir.chukharev@gmail.com (maintainer)
Approved by:    vladimir.chukharev@gmail.com (maintainer) [1]
Test Plan
  • poudriere -> OK (11.2-, 11.3-, 12.0-RELEASE, 13.0-CURRENT@r352110 amd64 + i386) for each py27 + py36 flavor
  • portlint -> OK (one false positive about " When USES=pyqt:5 is defined, you must also define USE_PYQT=xxxx)
  • make test -> OK (for py27 + py36, requires graphical display)

Everything is fine so far except the special case with the py-qt5-* ports that are used as TEST_DEPENDS. The question is here if the usage of the TEST_DEPENDS is acceptable?

There seems to be no other way to use some of the py-qt5-* ports only for testing purposes, e.g. with USE_PYQT=package_test.

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 28764
Build 26774: arc lint + arc unit

Event Timeline

kai created this revision.Sep 25 2019, 6:23 PM
koobs added a subscriber: koobs.Oct 2 2019, 6:14 AM
koobs added inline comments.
devel/py-pyface/Makefile
36–48

Should all of this be scoped to only particular PYTHON_VER's ?

kai added inline comments.Oct 2 2019, 2:58 PM
devel/py-pyface/Makefile
36–48

No, at the moment it can be used with every PYTHON_VER so far. For Python 2 it would be possible to add an option like "WX" to use the wxWidget toolkit but this would complicate things. For reference, this review has some similarities with D21229.

The patch looks good to me. And I hope this was clear in the the discussion in PR 239576, though that was not stated explicitly there.

IMHO, the comment by @koobs can be marked 'done', as is the reply marked.
I filed PR 240445 for the portlint false positive, with a proposed patch.

Regarding the questions about py-qt5-*, USE_PYQT and TEST_DEPENDS, hardly it can be solved within this review, IMHO.

I accept the revision, in hope that I do not violate some rules by doing so.

This revision is now accepted and ready to land.Nov 10 2019, 8:52 AM
koobs edited the summary of this revision. (Show Details)Nov 11 2019, 11:54 PM
koobs requested changes to this revision.Nov 11 2019, 11:58 PM

Uses/pyqt.mk should grow either standard support for :[build,run,test] args, or if it cant in its current state (it currently supports a <component>[_build|_run] format), then it should support <component>[_test] too, allowing those dependencies to be set only as test dependencies

Given the change to Uses/pyqt.mk to add this support is probably quite small, I'd look to add support with this diff, as a good candidate/use-case/warrant/example for its use

Add kde@ to blocking reviewers for that change

This revision now requires changes to proceed.Nov 11 2019, 11:58 PM

Personally I'd go with changing the pyqt format to the more consistently used in the tree <component>:<target> format but that's a more invasive change requiring a substantial amount of existing port churn. It would be good as a second step evolution of pyqt.mk to bring it in line with the rest of the framework

Uses/pyqt.mk should grow either standard support for :[build,run,test] args, or if it cant in its current state (it currently supports a <component>[_build|_run] format), then it should support <component>[_test] too, allowing those dependencies to be set only as test dependencies
Given the change to Uses/pyqt.mk to add this support is probably quite small, I'd look to add support with this diff, as a good candidate/use-case/warrant/example for its use
Add kde@ to blocking reviewers for that change

There was never a need so far for this, but I don't really see a reason why this could not be added. I'll look into adding support for ${component}_test later today.

@kai could you please test the linked review against your changes here?

lbartoletti_tuxfamily.org added inline comments.
devel/py-pyface/Makefile
36–48

For reference, I'm working on wxPython4: https://reviews.freebsd.org/D21915

kai updated this revision to Diff 66983.Jan 18 2020, 11:18 PM

@tcberner: Thank you for D23241 !

I changed this diff slightly to make use of the updated Mk/Uses/pyqt.mk once it lands:

  • py-qt5-* TEST_DEPENDS were converted into *_test for USE_PYQT
  • Updated comment about the current situation regarding wxPython 4 as pointed out by @lbartoletti

The test results by using make test for the py37 flavor were fine:

Ran 571 tests in 45.598s

OK (SKIP=61)

Using make -V TEST_DEPENDS also yields the expected results:

py37-mock>0:devel/py-mock@py37
py37-nose>0:devel/py-nose@py37
py37-numpy>0:math/py-numpy@py37
py37-pygments>0:textproc/py-pygments@py37
/usr/local/bin/python3.7:lang/python37
py37-qt5-network>=5.13.1:net/py-qt5-network@py37
py37-qt5-opengl>=5.13.1:x11/py-qt5-opengl@py37
py37-qt5-test>=5.13.1:devel/py-qt5-test@py37
py37-qt5-webkit>=5.13.1:www/py-qt5-webkit@py37
py37-qt5-webkitwidgets>=5.13.1:www/py-qt5-webkitwidgets@py37