Page MenuHomeFreeBSD

www/unit: Separate and complete language modules
ClosedPublic

Authored by lwhsu on Nov 24 2017, 3:32 PM.

Details

Summary
  • Separate language modules into individual ports
  • Complete Go module

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13045
Build 13298: arc lint + arc unit

Event Timeline

Don't create the py3- port, you will not be allowed to commit it. (And it will be useless soon.)

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].

  • Remove py3-unit-python
  • Rename unit-php to unit-php56

unit-php -> unit-php56 renaming is done.
@osa , I think @mat means D12464 , it looks we don't need to create individual port for different python version after it gets committed.

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?

In D13227#275997, @osa wrote:

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?

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.

In D13227#275997, @osa wrote:

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?

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.

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.

  • Add back py3*-unit-python, until we have better support in the ports infra

@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.

  • Use python flavors & allflavors to build modules for all python versions
www/py-unit-python/Makefile
5

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

Remove ${PYTHON_VER} it is obvious by the pyXY- prefix.

9

s/?=/=/

10

Unless there are other ports actually using the non default Python versions, I don't think allflavors is needed here.

12
PLIST_FILES= libexec/unit/modules/python${PYTHON_SUFFIX}.unit.so
www/py-unit-python/Makefile
5

I'd like to keep consistency between unit modules, unit-go, unit-php and unit-python.
How about moving to suffix like: unit-python-py24 or even unit-py24?

7

OK, though I thought it's no harm to mention again :)

10

I originally did this, but @osa (www/unit maintainer) would like to keep the old behavior of building all python modules for unit.

12

I checked other PLIST_FILES usage in the ports tree, we use %% for PLIST_SUB. Did I miss anything?

www/unit-php56/Makefile
6

I guess I should also remove ${PHP_VERSION:R}

  • 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
lwhsu marked 4 inline comments as done.Dec 1 2017, 2:38 PM
www/py-unit-python/Makefile
5

well, pyXY-unit already says python, no need to repeat it.

10

There is no such behavior. With the current code it only builds the first selected version.

12

Just because a few other did a bad thing does not mean you have to repeat it.
The %%FOO%% placeholders are for the pkg-plist file, because it is a static file and variables need to be carried there.
Here, in the Makefile, you can access the variable directly, use it.

www/py-unit-python/Makefile
5

I thought is pyXX- is the flavor and unit-python is the port name. I prefer this form, honestly.
If you insist on removing -python suffix, I will do so.

The name I wanted most is unit-pythonXX, though it is not consistent with other ports in the tree.

10

Oh, right, what I remembered is wrong. Just keeping the ability to build modules for other python versions is fine.

  • Remove unnecessary allflavors
  • Directly use Makefile variables in PLIST_FILES
www/py-unit-python/Makefile
5

Here, in the end, with what is currently in the review, we have:

PORTNAME=    unit
PKGNAMEPREFIX= ${PYTHON_PKGNAMEPREFIX}
PKGNAMESUFFIX= -python

So 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.

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.

Rename www/py-unit-python to www/py-unit

In D13227#279177, @osa wrote:

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?

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.

Hi @lwhsu,

so the final version of the patch looks good and I hope @mat has no more objection in this case as well, so I approve it.

@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!

This revision was automatically updated to reflect the committed changes.

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

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.