Re: import wip/unit

[Roland Illig <roland.illig%gmx.de@localhost>]

On 05.02.2021 16:38, Juraj Lutter wrote:
> Hi,
>
> I want to import wip/unit to www/unit. pkglint looks OK,
> package has been very carefuly tested on NetBSD-9.1, NetBSD-CURRENT and SmartOS.
>
> If noone has any objection, I will do this over the weekend.

patch-src_nxt__php__sapi is wrong.

-        while(isspace(*value)) {
+        while(isspace((int)(*value))) {

See https://man.netbsd.org/isspace.3 for the details.


In all the patches, I'm missing a remark about the upstream status of
the patches.  The pkgsrc policy is to have as few patches as possible,
therefore each patch should be submitted upstream and the issue URL or
other identifying information should be noted in the comment section of
the patch.

This is something that is not yet implemented in pkglint since I didn't
find time to discuss whether this should be an enforced rule or how the
proper format of the upstream status should look like, so that pkglint
can interpret it reliable.  It is documented in the pkgsrc guide though:

https://www.netbsd.org/docs/pkgsrc/pkgsrc.html#components.patches.caveats


In unit-perl/Makefile, post-configure is run in the plain environment,
without CONFIGURE_ENV.  I wasn't quite sure about this, so I added "env
| sort; exit 1" to the post-configure target and then compared this
output with the output from "bmake show-all-configure".  Some
differences are:

    * The locale is not set to "C"
    * COMPILER_RPATH_FLAG is missing
    * PKG_CONFIG is not set

The latter is probably handled redundantly by the pkgsrc infrastructure.
 Have a look at mk/configure/configure.mk, the target
do-configure-script.  This is the canonical way of running commands in
the configure phase.

This is also not covered in pkglint, it could probably be done, but
again, defining the correct rule set is nontrivial.  This rule set would
need to define which commands are safe (such as echo or printf) and how
a package can access the proper variables, since the code in
do-configure-script uses _CONFIGURE_SCRIPT_ENV, which starts with "_"
and is thus reserved for use by the pkgsrc infrastructure.

For now, just adding ${CONFIGURE_ENV} should be good enough.


In unit/Makefile.common, I would not define a default COMMENT since
every package should define a unique COMMENT anyway, making the COMMENT
in Makefile.common only useful for at most one package.

In unit/Makefile.common, you can replace !empty(PKG_OPTIONS:Mdevkit)
with the shorter ${PKG_OPTIONS:Mdevkit}, which has practically the same
effect.  It would result in a "parse error" if the variable PKG_OPTIONS
were undefined at that point, which is not possible in practice.


You should run "pkglint -Wall" once again in wip/unit, there's one
surprise left. :)


Other than these points, the packages look really solid.  Thanks for the
work you have put into them.


Roland