www/nginx: make easier to read and maintain

[joneum]

Summary

www/nginx: make easier to read and maintain:

• Objectives:
  • • make easier to read and maintain
    • Module config in a single section
    • use OPTIONS framwework where possible
  • Add options groups for mail and http
  • Use options groups to set _IMPLIES instead of large .if HTTP and MAIL blocks
  • Rename all _VERSION vars to align with the OPTION name
  • Add DEVEL_KIT option so we can automate
  • Separate bundled and 3rd party modules
  • Order options alphabetically
  • Pass openssl dir to configure
  • Separate external modules in separate makefile

www/nginx is buildable again with this:

• Fix sub-paths for DSO_EXTMODS introducing ${WRKSRC_mod}
  • Be smarter about the if ${PORT_OPTIONS:MDSO}
  • move post-install-OPT-on to where possible
  • Add some OPT_NGINX_VER if used multiple times

Test Plan

i will add @robak to this review. He is Maintainer from the Salve Ports for nginx.

[mat]

While you are at it, replace all the foo_GH_* with foo_GH_TUPLE, for all the extensions from github, it will replace 4 lines with only 1.

[brnrd]

Thanks for posting it here instead! I started this because every time I needed to look at nginx Makefile it made my head hurt...

In D14773#310715, @mat wrote:

While you are at it, replace all the foo_GH_* with foo_GH_TUPLE, for all the extensions from github, it will replace 4 lines with only 1.

Done in my local repo, will update this review.

Haven't collapsed all as I expect intent behind the separate line with the version numbers (could even push that to an included file). Putting the version in the tuple reduces some options by another 33.3% in size. 2-line modules, MODNAME_GH_TUPLE and MODNAME_CONFIGURE_ION are all that's left then.

Could actually use a bit of help here @mat , I'm struggling with variable expansion here. The DSO_EXTMODS loop in the main Makefile doesn't work as EXTMODS isn't declared yet after bsd.port.options.mk. There's some evidence of using := in the Makefile.extmods file but that didn't work and neither did using bsd.port.pre.mk in stead of bsd.port.options.mk.

[brnrd]

Fold _GH_* params to _GH_TUPLE

Remove _NGINX_VERSION vars too

[joneum]

ping @mat

[mat]

The DSO_EXTMODS does not work because of how opt_VARS does. It interpolates whatever you pass to it, which is bad because at the time when b.options.m.k is evaluated, the GitHub code has not ran so all the WRKSRC_* variables are unset.

What you should do is fill in DSO_EXTMODS with either WRKSRC_x (not the variable, the litteral) or just the x part, and then simply do:

.for modpath in ${DSO_EXTMODS}
.  if ${PORT_OPTIONS:MDSO}
CONF_ARGS+=     --add-dynamic-module=${WRKSRC_${modpath}}
.  else
CONF_ARGS+=     --add-module=${WRKSRC_${modpath}}
.  endif
.endfor

[mat]

Also, do not use := as you don't seem to understand what it does ;-)

www/nginx/Makefile

218

+=

239

+=

[brnrd]

Fix configure args for EXTMODS

• Split DSO_EXTMODS and DSO_EXTDIRS
• Make the ext-mod --add-module stuff work
• Use more target-OPT-on:
• Sort target-OPT-on alphabetically

TODO: Fix objs/Makefile pcre bug

[brnrd]

Fix do-build (this is not autoconf!)

• Remove --with-openssl
• Remove --with-builddir

Now actually builds. Next challenge: pkg-plist

[brnrd]

Fixups / typos

• Fix lingering ${MOD_VERSION} refs in Makefile.extmod
• Convert post-patch-MOD-on: targets to use ${WRKSRC_mod}

For nginx-full, make patch now completes OK

[brnrd]

Move some modules to dynamically loadable

• As per the module's config file

[brnrd]

In D14773#312227, @mat wrote:

Also, do not use := as you don't seem to understand what it does ;-)

I am learning all the way 😄 and have better understanding now.

In D14773#312224, @mat wrote:

The DSO_EXTMODS does not work because of how opt_VARS does. It interpolates whatever you pass to it, which is bad because at the time when b.options.m.k is evaluated, the GitHub code has not ran so all the WRKSRC_* variables are unset.

What you should do is fill in DSO_EXTMODS with either WRKSRC_x (not the variable, the litteral) or just the x part, and then simply do:

.for modpath in ${DSO_EXTMODS}
.  if ${PORT_OPTIONS:MDSO}
CONF_ARGS+=     --add-dynamic-module=${WRKSRC_${modpath}}
.  else
CONF_ARGS+=     --add-module=${WRKSRC_${modpath}}
.  endif
.endfor

Exactly! That's what I've figured out independently... Saw your remarks too late 😢

@mat How do you like the looks of the port now? Better?

Asked on #bsdports as well, should the dynamic plist in post-install not be migrated to static pkg-plist?

%%DSO%%%%STREAM%%%%MODULE%%libexec/nginx/ngx_stream_mod.so

[joneum]

Final Version

[brnrd]

www/nginx is buildable again with this

• Fix sub-paths for DSO_EXTMODS introducing ${WRKSRC_mod}
• Be smarter about the if ${PORT_OPTIONS:MDSO} check
• move post-install-OPT-on to where possible
• Add some OPT_NGINX_VER if used multiple times

Open: Passenger, Clojure, AJP options

[joneum]

after some fixes, here are the finaly version

[brnrd]

www/nginx/Makefile

218

Fixed locally, thanks for spotting!

239

Fixed locally

[brnrd]

Tested building nginx, nginx-lite, nginx-full and nginx-naxsi and these are now all OK.

Reduced the lines of Makefile from ca 1400 to 750...

[joneum]

fix Problem with "pkg-static: Unable to access file"

[mat]

www/nginx/Makefile

192–199

?

201–204

Use an OPTIONS_MULTI for that.

[brnrd]

www/nginx/Makefile

192–199

removed from local version.

Comments need to be removed still. Also see top of Makefile. That is not supposed to be committed.

201–204

That would remove the HTTP option from the HTTPGRP and the MAIL option from MAILGRP. Can be done but that puts the MAIL and HTTP options out of context in make config

All http modules have an //OPT//_IMPLIES= HTTP and all mail modules imply MAIL (lines 97 and 101).
I can't imagine anyone running into this error as it basically means that all modules options (including HTTP and MAIL) have been disabled and the user is requesting an nginx that wouldn't actually do anything. Nginx' configure would throw an error as well.