Page MenuHomeFreeBSD
Differential D13448

Add FLAVORS to devel/eric6
ClosedPublic
Actions

Authored by dbn on Dec 11 2017, 4:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Sep 15, 11:56 PM
Unknown Object (File)
Tue, Sep 15, 2:49 PM
Unknown Object (File)
Mon, Sep 14, 11:11 PM
Unknown Object (File)
Tue, Sep 8, 1:09 PM
Unknown Object (File)
Sun, Sep 6, 11:50 AM
Unknown Object (File)
Sun, Sep 6, 2:50 AM
Unknown Object (File)
Thu, Sep 3, 7:36 AM
Unknown Object (File)
Thu, Sep 3, 7:29 AM
View All Files
Subscribers
None

Details

Reviewers
mat
Group Reviewers
Python
portmgr
Commits
rP458671: devel/eric6: update to 17.11.1, add FLAVORS
Summary

Add the following flavor permutations to devel/eric6:

  • Python 2 and Python3
  • Qt 4 and Qt 5

My motivation for switching the port to flavors is:

  1. we are supporting dual major versions of Python and Qt;
  2. given the different versions having user/compile testing is important; and
  3. to allow a package user to not have to use the default version of Python/Qt
Test Plan

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

dbn created this revision.Dec 11 2017, 4:02 PM
mat added a comment.Dec 12 2017, 8:27 AM

It would probably be easier to have the flavors named qtX_pyY, so you don't conflict with the flavors from USES=python.

As a side note, is it possible for this port, as it seems to be an IDE, to support both Python versions at the same time ? A bit like editors/vim does.

devel/eric6/Makefile.inc
26–35

This should probably be in the MASTERPORT as it is only used there.

35
​PLIST_FILES= ${DATADIR}/i18n/${PORTNAME}_${I18N_LANG}.qm
40–46

It could be written in an easier way as:

PY_FLAVORS=        py${PYTHON_DEFAULT:S/.//} py${PYTHON2_DEFAULT:S/.//} py${PYTHON3_DEFAULT:S/.//}
​.for flavor in ${PY_FLAVORS}
​.  if empty(FLAVORS:M${flavor}_qt4)
​FLAVORS:=        ${FLAVORS} ${flavor}_qt4 ${flavor}_qt5
​.  endif
​.endfor
49–51

That is not needed.

49–51

I am not sure what this is doing, it is never used. Also, if it is only used in the MASTERPORT, it should probably be in the MASTERPORT.

dbn marked 5 inline comments as done.Dec 12 2017, 6:48 PM
In D13448#281293, @mat wrote:

It would probably be easier to have the flavors named qtX_pyY, so you don't conflict with the flavors from USES=python.

I have switched to the other way around. FYI, I think this is inconsistent with (although a symmetric translation of) the PKGNAMEPREFIX convention.

I suggest python.mk get hardened to avoid this conflict.

As a side note, is it possible for this port, as it seems to be an IDE, to support both Python versions at the same time ? A bit like editors/vim does.

Correct, the IDE does support both python2 and python3 (and thus the flavor is not strictly required) - however I think it will be useful for a package user should they wish to go python3 only.

devel/eric6/Makefile.inc
49–51

It was a carry over from options. It should read RUN_DEPENDS+=...

dbn updated this revision to Diff 36511.Dec 12 2017, 6:50 PM
dbn marked an inline comment as done.
dbn edited the test plan for this revision. (Show Details)

Update patch per comments.

dbn updated this revision to Diff 36512.Dec 12 2017, 6:51 PM
dbn updated this revision to Diff 36513.Dec 12 2017, 6:54 PM
dbn marked an inline comment as done.
dbn added inline comments.
devel/eric6/Makefile.inc
49–51

In this case, PYQRVER is only defined after the includes and so cannot be in MASTERPORT.

mat added a comment.Dec 12 2017, 8:06 PM
In D13448#281425, @dbn wrote:

I suggest python.mk get hardened to avoid this conflict.

Already done.

devel/eric6/Makefile.inc
39–47

USES=python already does that, remove.

49–51

It is defined line 20 of the Makefile.

dbn updated this revision to Diff 36566.Dec 13 2017, 7:04 PM
dbn marked 3 inline comments as done.
dbn edited the test plan for this revision. (Show Details)

Thanks, I has misunderstanding when FLAVOR gets defined (I thought only after including bsd.port.options.mk, or co). Fixed that and removed redundant code.

mat added inline comments.Dec 14 2017, 2:00 PM
devel/eric6/Makefile
20

FLAVOR can be undefined here, and you are using PYQTVER before it gets defined, so it should be guarded with :U. Also, a default value should probably be set.

devel/eric6/Makefile.inc
47

Is this needed ?

dbn updated this revision to Diff 36671.Dec 16 2017, 6:22 PM
  • Add a default FLAVOR, after defining FLAVORS.
  • Remove redundant ".include <bsd.port.pre.mk>"
dbn marked 2 inline comments as done.Dec 16 2017, 6:22 PM
dbn added a comment.Dec 29 2017, 12:41 PM

Is there any other change suggested/required? Can this change be committed (with flavors)?

mat added a comment.Dec 29 2017, 1:12 PM

Let me build test all this, I wonder if there is not some edge case I have not seen but is broken.

mat added a comment.Dec 29 2017, 5:16 PM

I do not think the language files should be flavored. They install the exact same content at the exact same place. The dependencies should probably be reversed, devel/eric6 should depend on them, or they should be pointed out in pkg-message and the dependencies should be removed.

dbn updated this revision to Diff 37583.Jan 6 2018, 9:44 AM
dbn edited the summary of this revision. (Show Details)
dbn edited the test plan for this revision. (Show Details)

I've opted for your second suggest: break dependency of the language ports on eric6 and instead mention them in a pkg-message.

dbn updated this revision to Diff 37587.Jan 6 2018, 2:08 PM
dbn edited the test plan for this revision. (Show Details)

Fix error with i18n ports identified by exp-run. I think this is ready to ship :-)

mat accepted this revision as: portmgr.Jan 8 2018, 2:33 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 10 2018, 6:39 PM
Closed by commit rP458671: devel/eric6: update to 17.11.1, add FLAVORS (authored by dbn). · Explain Why
This revision was automatically updated to reflect the committed changes.
dbn added a comment.Jan 10 2018, 6:57 PM

@mat thank you for your effort in reviewing this port, and improving both the FLAVORS implementation within eric6 and eric6 in general.

Revision Contents
Changeset List

PathSize
devel/
eric6/
21 lines
43 lines
6 lines

Diff 36566

View Options

devel/eric6/Makefile

Loading...
View Options

devel/eric6/Makefile.inc

Loading...
View Options

devel/eric6/distinfo

Loading...