Page MenuHomeFreeBSD

lang/python{27,33,34,35,36}: Make Python curses module work with Unicode
ClosedPublic

Authored by dbaio on Jun 10 2017, 2:41 PM.

Details

Summary
lang/python{27,33,34,35,36}: Make Python curses module work with Unicode

Use readline from ports (USES= readline:port) and patch
setup.py to ignore readline from base. The patch is necessary for
FreeBSD < 1100000, as after this the readline library became an
INTERNALLIB, see base r268461 [1]

Link devel/readline against termcapw instead of termcap is part of
this change, see ports r444463 [2]

Note that this is the **ports** approach for getting Python curses
module working with Unicode. The other way is splitting libncurses
into separate libncurses and libtinfo in base, for which an open
issue exists [3].

Apart from Python language ports, at least www/rtv and
sysutils/py-ranger ports have been tested to work correctly
(display Unicode) after this change.

[1] https://svnweb.freebsd.org/changeset/base/268461
[2] https://svnweb.freebsd.org/changeset/ports/444463
[3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=197317

 PR:	171246, 197317
 Reviewed by:   miwi, koobs
 Reported by:	Vitaly Magerya <vmagerya gmail com>
 Approved by:	garga (mentor), ??? (python, with hat)
 Differential Revision:	D11127
Test Plan

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

dbaio created this revision.Jun 10 2017, 2:41 PM
miwi accepted this revision as: Python.Jun 12 2017, 4:25 AM
koobs requested changes to this revision.Jun 12 2017, 6:48 AM

Please add changes for lang/python33.

While python33 has been DEPRECATED it has not expired. All changes to python ports should be made consistently across all ports unless they are not affected by the fix/issue to both minimise diffs, but also ensure consistent behaviour.

Also, I would:

  • Separate commits (commit for readline (as it affects other ports), then then consumers (lang/python*) separately referncing the original commit revision
This revision now requires changes to proceed.Jun 12 2017, 6:48 AM
koobs added a comment.Jun 12 2017, 6:51 AM

@dbaio And thank you for paying this issue attention :)

koobs edited the summary of this revision. (Show Details)Jun 12 2017, 6:52 AM
mat added a comment.Jun 12 2017, 12:01 PM

You will also probably have to bump PORTREVISION of all devel/readline consumers as you change what it links with.

sunpoet edited edge metadata.Jun 12 2017, 8:19 PM

I'm OK with readline part. But I would like to add it as part of the readline 7.0 update to reduce unnecessary PORTREVISION bump on 250+ ports. I plan to submit readline 7.0 update for exp-run tomorrow.

I've requested exp-run for readline 7.0 update as PR 219947.

dbaio updated this revision to Diff 29529.Jun 13 2017, 2:32 AM
dbaio edited edge metadata.

Thank you all for taking a look on this.
Removing readline and adding python 3.3.

koobs edited the summary of this revision. (Show Details)Jun 13 2017, 2:45 AM
koobs edited the summary of this revision. (Show Details)
koobs retitled this revision from Make Python curses module work with Unicode to lang/python{27,33,34,35,36}: Make Python curses module work with Unicode.Jun 16 2017, 7:48 AM
koobs edited the summary of this revision. (Show Details)
koobs edited the test plan for this revision. (Show Details)

@dbaio I've updated the commit log message to include comments that were added to the TEST PLAN section, and added some QA entries that should be confirmed. Let me know if you have any questions about what each of them are or consist of.

koobs requested changes to this revision.Jun 16 2017, 7:51 AM
koobs edited the test plan for this revision. (Show Details)
This revision now requires changes to proceed.Jun 16 2017, 7:52 AM
dbaio edited the summary of this revision. (Show Details)Jun 25 2017, 2:03 AM
dbaio edited the test plan for this revision. (Show Details)

@koobs, could you take a look at the make test logs?
I get similar results with and without this change... any advices?

Regards.

koobs edited the summary of this revision. (Show Details)Jun 25 2017, 3:31 AM
koobs added a comment.Jun 25 2017, 3:34 AM

@dbaio Satisfied if the test failure/success delta is zero before and after this change. Please confirm that that is the case

Commit log message nit:

Multiple PR: lines should be combined into one (i believe both space or comma separated can be processed by the post-commit hook scripts)

dbaio updated this revision to Diff 30142.Jun 27 2017, 8:53 PM
dbaio edited edge metadata.
dbaio edited the test plan for this revision. (Show Details)

Updating PORTREVISION

dbaio edited the summary of this revision. (Show Details)Jun 27 2017, 8:57 PM
dbaio edited the test plan for this revision. (Show Details)

Thanks @sunpoet for D11172

garga accepted this revision.Jun 27 2017, 11:16 PM

I'm OK with the change if it looks good for @koobs and/or @sunpoet

sunpoet accepted this revision.Jun 28 2017, 12:32 AM

LGTM. Thanks!

This revision was automatically updated to reflect the committed changes.