- Separate language modules into individual ports
- Complete Go module
Details
Details
- Reviewers
osa - Commits
- rP455874: www/unit: separate and complete language modules
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Don't create the py3- port, you will not be allowed to commit it. (And it will be useless soon.)
Comment Actions
Hi Li-Wen,
thanks very much for this job, it looks great!
Since you've created the ports with suffixes with versions -php7[0-2], can we use the same naming convention for unit module for php56, i.e. www/unit-php56?
Also, I'd prefer to see a modules for every version of python we have in our ports tree. Hope we can use the same naming convention for pytho modules, i.e. www/unit-python[27,34-36].
Comment Actions
Hi Li-Wen,
we definitely don't need to create a separate port for every version of python or even php if we can do that with one version of a port.
In this case - how can we install a couple of unit modules to support two different versions of python, i.e. python27 and python36, can we do so with current version of www/py-unit-python?
Comment Actions
If I understand correctly, the new infra will automatically generate packages with 2 different versions of python (default versions of python2 and python3), so we will have py27-unit-python and py36-unit-python packages from one port directory.
Please note that in py-unit-python, I use PYTHON_SUFFIX in the module name so there will be no conflict for two modules co-exist.
However, I also like to have pre-built packages for each supported version of python in the ports tree. I will join the discussion in that differential to express my opinions.
If that differential takes too long to land, I think we can have py3-unit-python (or even py34-, py35-, py36-) added first.
How do you think? Let's have this patch committed first, for getting better Go and PHP support.
Comment Actions
What about two other versions of python? In current version of www/unit we can create the unit modules for all four versions of python. How can we do so with your solution?
However, I also like to have pre-built packages for each supported version of python in the ports tree. I will join the discussion in that differential to express my opinions.
If that differential takes too long to land, I think we can have py3-unit-python (or even py34-, py35-, py36-) added first.
This is what I'd like to see in the ports tree for every language and it's version (at this moment we have 4 versions of php, 4 versions of python as well and go):
- an ability to build a module from ports tree;
- an ability to use/install a pre-built a package as well.
How do you think? Let's have this patch committed first, for getting better Go and PHP support.
So, I don't think I can agree with this approach now. Let's create a complete solution first, then commit it into the ports tree.
Please let me know if you have any questions.
Comment Actions
@osa , I agree with you, I think having py3*-unit-python for now is a good way for improving unit and its modules without regressions (comparing with what we have in ports currently)
@mat, I am happy to work with you for multiple python versions support, and I can take care of modifying py-unit-python and removing py3*-unit-python after D12464 committed.
| www/py-unit-python/Makefile | ||
|---|---|---|
| 5 ↗ | (On Diff #36011) | Side question, does it really need the PKGNAMESUFFIX ? I mean, it already going to be named pyXY-unit, unless there is another port that generates those packages, I think the -python bit should be dropped. |
| 7 ↗ | (On Diff #36011) | Remove ${PYTHON_VER} it is obvious by the pyXY- prefix. |
| 9 ↗ | (On Diff #36011) | s/?=/=/ |
| 10 ↗ | (On Diff #36011) | Unless there are other ports actually using the non default Python versions, I don't think allflavors is needed here. |
| 12 ↗ | (On Diff #36011) | PLIST_FILES= libexec/unit/modules/python${PYTHON_SUFFIX}.unit.so |
| www/py-unit-python/Makefile | ||
|---|---|---|
| 5 ↗ | (On Diff #36011) | I'd like to keep consistency between unit modules, unit-go, unit-php and unit-python. |
| 7 ↗ | (On Diff #36011) | OK, though I thought it's no harm to mention again :) |
| 10 ↗ | (On Diff #36011) | I originally did this, but @osa (www/unit maintainer) would like to keep the old behavior of building all python modules for unit. |
| 12 ↗ | (On Diff #36011) | I checked other PLIST_FILES usage in the ports tree, we use %% for PLIST_SUB. Did I miss anything? |
| www/unit-php56/Makefile | ||
| 6 ↗ | (On Diff #36011) | I guess I should also remove ${PHP_VERSION:R} |
Comment Actions
- Remove unnecessary ${PYTHON_VER} and ${PHP_VERSION:R}
- Remove unnecessary ?= www/py-unit-python was a slave of www/unit but also master of www/py3-unit-python
| www/py-unit-python/Makefile | ||
|---|---|---|
| 5 ↗ | (On Diff #36011) | well, pyXY-unit already says python, no need to repeat it. |
| 10 ↗ | (On Diff #36011) | There is no such behavior. With the current code it only builds the first selected version. |
| 12 ↗ | (On Diff #36011) | Just because a few other did a bad thing does not mean you have to repeat it. |
| www/py-unit-python/Makefile | ||
|---|---|---|
| 5 ↗ | (On Diff #36011) | I thought is pyXX- is the flavor and unit-python is the port name. I prefer this form, honestly. The name I wanted most is unit-pythonXX, though it is not consistent with other ports in the tree. |
| 10 ↗ | (On Diff #36011) | Oh, right, what I remembered is wrong. Just keeping the ability to build modules for other python versions is fine. |
| www/py-unit-python/Makefile | ||
|---|---|---|
| 5 ↗ | (On Diff #36011) | Here, in the end, with what is currently in the review, we have: PORTNAME= unit
PKGNAMEPREFIX= ${PYTHON_PKGNAMEPREFIX}
PKGNAMESUFFIX= -pythonSo the package names will end up being py27-unit-python and py36-unit-python. If you really want to repeat the fact that it is python, then do so, but I find it strange as it is purely artificial and superfluous. |
Comment Actions
Hi @lwhsu,
as far as I see the EMBED option hasn't been defined yet for
lang/php* modules, is it the blocker for new php*-unit modules?
Do we really need '-python' suffix for python-specific modules? I believe 'py*-' prefix is enough in this case.
Comment Actions
Not anymore, I committed the change to make it as default.
In fact, I got the approval from the lang/php* maintainers but thought I can commit it before this patch gets accepted.
Do we really need '-python' suffix for python-specific modules? I believe 'py*-' prefix is enough in this case.
My thought is in the previous, since you and @mat both think it's not necessary, I removed the suffix.
Comment Actions
@osa , Thanks, hope adding these slave ports doesn't introduce too much trouble to you :-)
You can simply select "Accept Revision" in the "Add Action..." list at the bottom and then click "Submit" to explicitly accept the patch in this system. Though this is not necessary, just FYI.
I think @mat is also fine with it so I am going to commit this patch. Thanks for the help from both of you!
Comment Actions
This results in an error on my package builder:
[00:02:57] Warning: (www/unit-php71): [00:02:57] Error: Duplicated origin for unit-php71-0.2_1: www/unit-php71 AND www/unit-php56. Rerun with -v to see which ports are depending on these.
Version settings on another host
$ make -VDEFAULT_VERSIONS apache=2.5 mysql=10.1m php=7.1 ruby=2.3 python=2.7
When building unit_php70
$ make -VPKGNAME unit-php71-0.2_1 $ make build-depends-list /usr/ports/ports-mgmt/pkg /usr/ports/lang/php71
From configure output
Configuration summary: unit bin directory: "/usr/local/bin" unit sbin directory: "/usr/local/sbin" unit modules directory: "/usr/local/libexec/unit/modules" unit state directory: "/usr/local/libexec/unit" unit pid file: "/var/run/unit/unit.pid" unit log file: "/var/log/unit/unit.log" unit control API socket: "unix:/var/run/unit/control.unit.sock" non-privileged user: "nobody" non-privileged group: "" IPv6 support: YES Unix domain sockets support: YES debug logging: NO configuring PHP module checking for PHP ... found + PHP version: 7.1.12 + PHP SAPI: [cli embed fpm phpdbg cgi] checking for PHP embed SAPI ... found + PHP module: php71.unit.so ===> Building for unit-php71-0.2_1
Comment Actions
I guess this is because www/unit-php56 is using default PHP version (5.6) in the ports tree. I should explicitly set it. Sorry for the breakage. I will test the new patch and fix it.