Openresty downstream bundle maintenance requests

dubstep · September 17, 2011, 10:53am

Hi.

A few things to consider with respect to downstream maintenance support.

The configure script under the “for my $opt (@ARGV) {” loop covers a few
typical items and any others cause the “die “Invalid option $opt\n”;”
line
to be triggered. However, when in a Fedora/Enterprise Linux build
environment, the “%configure” macro in the spec file adds several other
switches (which work fine when building nginx directly).

Can the upstream please consider modifying the configure script to
allow
for these?

A possiblity is:

for my $opt (@ARGV) {
…

} elsif ($opt =~ /^--build=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--host=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--target=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--program-prefix=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--exec-prefix=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--bindir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--sbindir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--sysconfdir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--datadir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--includedir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--libexecdir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--localstatedir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--sharedstatedir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--mandir=(.*)/) {
    push @ngx_opts, $opt;

} elsif ($opt =~ /^--infodir=(.*)/) {
    push @ngx_opts, $opt;

} else {
    die "Invalid option $opt\n";
}

}

May be even merge all the “dir” ones into “} elsif ($opt =~
/^–(.)dir=(.)/) {”

Also, can the upstream consider dropping version numbers from the
folders in
the “bundle” directory and record the version numbers in a README? Would
save having to edit the spec file for every release.

Finally, can the upstream consider allowing the with luajit option to be
selected but not to try to build luajit? I.E. set openresty up to use
luajit
but have the luajit build done separately and not try to replace this by
the
standard lua. I suppose the same applies to the standard lua. Basically,
third party applications should ideally be built separately and we would
look into creating seperate rpms for those. Can foresee some potential
knotty issues to overcome with this but if distributing an rpm, then
there
are issues anyway.

Thanks

Nginx_U · September 17, 2011, 3:58pm

On Sat, Sep 17, 2011 at 4:53 PM, Nginx U. [email protected] wrote:

The configure script under the “for my $opt (@ARGV) {” loop covers a few
typical items and any others cause the “die “Invalid option $opt\n”;” line
to be triggered. However, when in a Fedora/Enterprise Linux build
environment, the “%configure” macro in the spec file adds several other
switches (which work fine when building nginx directly).

Which options do not work for you? The last part in that loop is
actually:

} elsif ($opt =~ /^--\w.*/) {
    push @ngx_opts, $opt;

} else {
    die "Invalid option $opt\n";
}

That is, any options starting with a /\w/ will be matched in the last
2nd condition and passed to nginx’s configure script.

push @ngx_opts, $opt;
} elsif ($opt =~ /^–datadir=(.)/) {
push @ngx_opts, $opt;
} elsif ($opt =~ /^–infodir=(.)/) {
push @ngx_opts, $opt;
} else {
die “Invalid option $opt\n”;
}
}

These options should be happily accepted by the last 2nd option. If
not, please let me know

Also, can the upstream consider dropping version numbers from the folders in
the “bundle” directory and record the version numbers in a README? Would
save having to edit the spec file for every release.

Okay, I’ll remove version numbers in my build script in the next
release of ngx_openresty

Finally, can the upstream consider allowing the with luajit option to be
selected but not to try to build luajit? I.E. set openresty up to use luajit
but have the luajit build done separately and not try to replace this by the
standard lua. I suppose the same applies to the standard lua. Basically,
third party applications should ideally be built separately and we would
look into creating seperate rpms for those.

Will adding new options like --with-luajit=PATH and --with-lua51=PATH
work for you? That is, telling ngx_openresty to use an external luajit
or lua 5.1 build located by PATH?

By default, ngx_openresty will continue try to build its bundled
version of LuaJIT or Lua 5.1 and use it whenever possible and that’s
the whole point of a bundle But I’m still willing to add special
options to help RPM/Debian/etc packaging for sure

Can foresee some potential
knotty issues to overcome with this but ifdistributingan rpm, then there
are issues anyway.

I’m very willing to help and solve any issues along the way

Thanks!
-agentzh

Nginx_U · September 17, 2011, 4:08pm

On Sat, Sep 17, 2011 at 4:53 PM, Nginx U. [email protected] wrote:

Also, can the upstream consider dropping version numbers from the folders in
the “bundle” directory and record the version numbers in a README? Would
save having to edit the spec file for every release.

Just out of curiosity, why will you hard-code the directory names
under bundle/ in your rpm spec file?

Regards,
-agentzh

Nginx_U · September 17, 2011, 4:43pm

On Sat, Sep 17, 2011 at 4:58 PM, agentzh [email protected] wrote:

} elsif ($opt =~ /^–\w.*/) {
push @ngx_opts, $opt;

} else {
die “Invalid option $opt\n”;
}

That is, any options starting with a /\w/ will be matched in the last
2nd condition and passed to nginx’s configure script.

Correct and it matches all the “–with XYZ” and “–without XYZ” flags
for
nginx.

    push @ngx_opts, $opt;
} elsif ($opt =~ /^--datadir=(.*)/) {
    push @ngx_opts, $opt;
} elsif ($opt =~ /^--infodir=(.*)/) {
    push @ngx_opts, $opt;
} else {
    die "Invalid option $opt\n";
}
}
These options should be happily accepted by the last 2nd option. If
not, please let me know

They are matched by the last option of ‘die “Invalid option $opt\n”;’ .

Also, can the upstream consider dropping version numbers from the folders
in
the “bundle” directory and record the version numbers in a README? Would
save having to edit the spec file for every release.

Okay, I’ll remove version numbers in my build script in the next
release of ngx_openresty

No longer required thanks.

Will adding new options like --with-luajit=PATH and --with-lua51=PATH
work for you? That is, telling ngx_openresty to use an external luajit
or lua 5.1 build located by PATH?

This should work. We can then switch piar this with --without luajit. We
obviously want to keep the lua-rds-parser and lua-redis options.

I’m very willing to help and solve any issues along the way

Thanks!
-agentzh

Thanks!

Nginx_U · September 17, 2011, 5:22pm

On Sat, Sep 17, 2011 at 10:43 PM, Nginx U. [email protected]
wrote:

These options should be happily accepted by the last 2nd option. If
not, please let me know

They are matched by the last option of ‘die “Invalid option $opt\n”;’ .

I’ve just confirmed that at least “–includedir=PATH” and
“–datadir=PATH” are accepted by ngx_openresty’s configure script but
rejected by nginx’s configure script:

~/work/nginx-1.0.6 $ ./configure --includedir=/tmp/blah
./configure: error: invalid option "--includedir=/tmp/blah"

~/work/nginx-1.0.6 $ ./configure --datadir=/tmp/blah
./configure: error: invalid option "--datadir=/tmp/blah"

Please note that the ‘error: invalid option “.*”’ messages
different from the one output by ‘die “Invalid option $opt\n”;’.

Regards,
-agentzh

Nginx_U · September 18, 2011, 7:35am

On Sat, Sep 17, 2011 at 10:43 PM, Nginx U. [email protected]
wrote:

On Sat, Sep 17, 2011 at 4:58 PM, agentzh [email protected] wrote:

Will adding new options like --with-luajit=PATH and --with-lua51=PATH
work for you? That is, telling ngx_openresty to use an external luajit
or lua 5.1 build located by PATH?

This should work. We can then switch piar this with --without luajit. We
obviously want to keep the lua-rds-parser and lua-redis options.

I’ve just added the --with-luajit=PATH and --with-lua51=PATH options
to ngx_openresty’s configure script and this update has already been
included in the latest devel release, 1.0.6.7:

http://openresty.org/#Download

Please test it out to see if it fits your requirements All of those
Lua modules bundled by ngx_openresty will still be built by default
when either --with-luajit=PATH or --with-lua51=PATH is specified
unless being explicitly disabled.

Enjoy!
-agentzh

Nginx_U · September 18, 2011, 6:19pm

On Sun, Sep 18, 2011 at 8:34 AM, agentzh [email protected] wrote:

Enjoy!
-agentzh

Hi.

I’ll have a go later.

Just as an update, please ignore the other requests. The only one that
needed attention was this lua related one.

Thanks

Nginx_U · September 18, 2011, 11:08am

On Sat, Sep 17, 2011 at 6:42 PM, Nginx U. [email protected] wrote:

They are matched by the last option of ‘die “Invalid option $opt\n”;’ .
./configure: error: invalid option “–datadir=/tmp/blah”

However things crash with the openresty configure as said. Specifically,
the message returned is for on “–build” which is the first one encountered.
Recreating the build machine at the moment so can’t give specifics right
now. I’ll try changing the text in ‘die “Invalid option $opt\n”;’ when
having another go to verify whether it is actually this or not. Looking at
the code though, it seems clear that these extra options will fail at the
specified point.

Thanks.

Hi.

Just looking at the spec file example I linked to, I can see it is not
using
the rpm %configure macro. So you were on to something there.

I suppose the only remaining valid request from my original post is the
lua
one.

Thanks

Nginx_U · September 18, 2011, 11:31pm

On Sat, Sep 17, 2011 at 11:42 PM, Nginx U. [email protected]
wrote:

+# This combined with Fedora’s FORTIFY_SOURCE=2 option causes it nginx
+# to not compile.
+#CFLAGS=“$CFLAGS -Werror”

debug

CFLAGS=“$CFLAGS -g”

The nginx core bundled by ngx_openresty has this patch as well.

The standard outputs from the rpm %configure macro including all those
elements build fine after this when building nginx by itself. An example of
the typical spec file with this patch
ishttp://www4.atomicorp.com/channels/source/nginx/nginx.spec (they are not
up to date but it does work as is for 1.0.6 and should for the 1.1.x branch
as well.
However things crash with the openresty configure as said. Specifically, the
message returned is for on “–build” which is the first one encountered.

The configure command in the RPM spec file you link to does not crash
on my slackware linux x86_64 for ngx_openresty 1.0.6.5. I wonder what
error messages are you getting?

Recreating the build machine at the moment so can’t give specifics right
now. I’ll try changing the text in’die “Invalid option $opt\n”;’ when
having another go to verify whether it is actually this or not. Looking at
the code though, it seems clear that these extra options will fail at the
specified point.

Why won’t “–build” be matched by /^–\w.*/ in ngx_openresty’s
configure script? I’ve said extra options would be captured by the
last 2nd condition and be passed verbatim to nginx’s configure script
and would not reach the last “die” statement there.

Regards,
-agentzh