[oe] [meta-openembedded/meta-webserver][PATCH] nginx: new recipe, updated from aging original in rpi layer (has not been scrubbed for any potential policy issues).

[Stephen Arnold]

Paul:

Okay, I have a cleaner and more compliant recipe and a few questions for
your comments.  See below...


On Wed, Oct 9, 2013 at 2:55 AM, Paul Eggleton <paul.eggleton at linux.intel.com
> wrote:

> Hi Stephen,
>
> Thanks for sending this. A few comments below.
>
> On Tuesday 08 October 2013 22:33:44 stephen.arnold42 wrote:
> > From: "stephen.arnold42" <stephen.arnold42 at gmail.com>
>
> Ideally the part of the subject from "updated..." onwards should go into
> the
> commit message. You should really specify that the "rpi" layer you refer
> to is
> your own.
>

So,  [meta-openembedded/meta-webserver][PATCH] nginx: new recipe] and be
more accurate about lineage.

I only know the IRC nick of the original author of the old recipe
(bencoh).  I wrote the init script and minimal config file, made sure
everything built and ran correctly in the rpi-image, and made an rpi-data
package.  That lead to me making a newer (generic) recipe for 1.4.0 (but
without the rpi-graphics).  How should I describe that?  With bencoh as the
original author and me as "upstream"?


> > +++ b/meta-webserver/recipes-httpd/nginx/files/nginx-cross_1.4.0.diff
> > @@ -0,0 +1,212 @@
> > +diff -uraN nginx-1.0.11.orig/auto/feature nginx-1.0.11/auto/feature
>
> Could you please add a header to this patch mentioning what it does, the
> origin if it wasn't something you wrote, the Upstream-Status and your
> Signed-
> off-by?
>

So you want a bigger commit message mentioning the above in front of the
signed-off-by that's already there?  Origin is described above (bencoh and
then me) but what do you mean by "Upstream-Status"?  Who/what is upstream
in this case?


> > +DESCRIPTION = "HTTP and reverse proxy server"
>
> Please set this as SUMMARY rather than DESCRIPTION since it's one line.
>
> > +HOMEPAGE = "http://nginx.org/"
> > +LICENSE = "BSD"
>
> Can we be specific about which BSD license this is? Looking at it I think
> "BSD-2-Clause" is the correct one.
>
> > +SECTION = "net"
> > +PRIORITY = "optional"
>
> Please drop PRIORITY, we don't use this anymore.
>

Done, done, and done.



> > +DEPENDS = "libpcre gzip openssl"
> > +
> > +PR = "r4"
>
> PR should ideally be dropped since this is a new recipe for public layers.
>

Dropped completely, or set PR = "r0" ?


> > +SRC_URI = " \
> > +     http://nginx.org/download/nginx-${PV}.tar.gz \
> > +     file://nginx-cross_${PV}.diff;name=crosspatch \
> > +     file://nginx.conf \
> > +     file://nginx.init \
> > +"
> > +
> > +S = "${WORKDIR}/nginx-${PV}"
>
> This is the default value for S ("${WORKDIR}/${BP}" where BP is
> "${BPN}-${PV}"
> and BPN is "nginx") so no need to set this.
>
> > +inherit autotools update-rc.d useradd
>
> I could be mistaken, but looking at the source it doesn't appear to be
> using
> autotools, in which case inheriting autotools wouldn't be right. That does
> mean you'll have to define your own do_install (doing
> "oe_runmake 'DESTDIR=${D}' install" before what you currently have in
> do_install_append).
>
> > +SRC_URI[md5sum] = "d496e58864ab10ed56278b7655b0d0b2"
> > +SRC_URI[sha256sum] =
> > "84aeb7a131fccff036dc80283dd98c989d2844eb84359cfe7c4863475de923a9"
> > +SRC_URI[crosspatch.md5sum] = "707c4cdd6bb82719ea2ed50971101c21"
> > +SRC_URI[crosspatch.sha256sum] =
> > "96cc3b087126caaa0951ab3e3f9f26169e9caf283dd2aeb689ed6c435070f052"
>
> We don't normally checksum local files such as "crosspatch".
>
> > +LIC_FILES_CHKSUM =
> "file://LICENSE;md5=917bfdf005ffb6fd025550414ff05a9f" +
> > +CONFFILES_${PN} = "${sysconfdir}/nginx/nginx.conf \
> > +             ${sysconfdir}/nginx/fastcgi.conf\
> > +             ${sysconfdir}/nginx/fastcgi_params \
> > +             ${sysconfdir}/nginx/koi-utf \
> > +             ${sysconfdir}/nginx/koi-win \
> > +             ${sysconfdir}/nginx/mime.types \
> > +             ${sysconfdir}/nginx/scgi_params \
> > +             ${sysconfdir}/nginx/uwsgi_params \
> > +             ${sysconfdir}/nginx/win-utf \
> > +"
> > +
> > +INITSCRIPT_NAME = "nginx"
> > +INITSCRIPT_PARAMS = "defaults 92 20"
> > +
> > +do_configure () {
> > +     PTRSIZE=$(expr ${SITEINFO_BITS} / 8)
> > +
> > +     echo $CFLAGS
> > +     echo $LDFLAGS
> > +
> > +     ./configure \
> > +     --crossbuild=Linux:${TUNE_ARCH} \
> > +     --with-endian=${@base_conditional('SITEINFO_ENDIANNESS', 'le',
> 'little',
> > 'big', d)} \ +        --with-int=4 \
> > +     --with-long=${PTRSIZE} \
> > +     --with-long-long=8 \
> > +     --with-ptr-size=${PTRSIZE} \
> > +     --with-sig-atomic-t=${PTRSIZE} \
> > +     --with-size-t=${PTRSIZE} \
> > +     --with-off-t=${PTRSIZE} \
> > +     --with-time-t=${PTRSIZE} \
> > +     --with-sys-nerr=132 \
> > +     --conf-path=/etc/nginx/nginx.conf \
> > +     --http-log-path=/var/log/nginx/access.log \
> > +     --error-log-path=/var/log/nginx/error.log \
> > +     --pid-path=/var/run/nginx/nginx.pid \
> > +     --prefix=/usr \
> > +     --with-http_ssl_module \
> > +     --with-http_gzip_static_module
> > +}
> > +
> > +do_install_append () {
> > +    install -d ${D}${localstatedir}/www/localhost
> > +    mv ${D}/usr/html ${D}${localstatedir}/www/localhost/
> > +    chown www:www-data -R ${D}${localstatedir}
> > +
> > +    install -d ${D}${sysconfdir}/init.d
> > +    install -d ${D}${sysconfdir}/nginx
> > +    install -m 0755 ${WORKDIR}/nginx.init ${D}${sysconfdir}/init.d/nginx
> > +    install -m 0644 ${WORKDIR}/nginx.conf ${D}${sysconfdir}/nginx/
> > +
> > +    install -d ${D}${sysconfdir}/default/volatiles
> > +    echo "d www www-data 0755 ${localstatedir}/run/nginx none" \
> > +        > ${D}${sysconfdir}/default/volatiles/99_nginx
> > +}
> > +
> > +FILES_${PN} += "${localstatedir}/ /run/"
>
> I don't think we should be adding /run/ here. There is a guide on how to
> deal
> with unshipped file/dir warnings for /run in case you haven't seen it:
>
> http://permalink.gmane.org/gmane.comp.handhelds.openembedded/58530
>

Thanks, and done.



> Cheers,
> Paul
>
> --
>
> Paul Eggleton
> Intel Open Source Technology Centre
>