[v2] monkey: new package

From: Morgan Delestre <m.delestre@sinters.fr>

Monkey is a small, fast and lightweight open source Web Server for GNU/Linux. 
It has been designed with focus in embedded devices, therefore its scalable by 
nature having a low memory and CPU consumption and an excellent performance.

Signed-off-by: Julien Corjon <corjon.j@ecagroup.com>
---
Changes v1 -> v2:
  - Fix typo (suggested by Alexandre)
  - Change autotools for generic-package (suggested by Thomas)
  - Add hash file
  - Add licence
  - Add uClib & musl support
  - Add shared library option

 package/Config.in          |   1 +
 package/monkey/Config.in   |  19 ++++++++
 package/monkey/monkey.hash |   3 ++
 package/monkey/monkey.mk   | 109 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)
 create mode 100644 package/monkey/Config.in
 create mode 100644 package/monkey/monkey.hash
 create mode 100644 package/monkey/monkey.mk

Dear Julien CORJON,

On Mon, 29 Jun 2015 09:02:43 +0000, Julien CORJON wrote:
> From: Morgan Delestre <m.delestre@sinters.fr>
> 
> Monkey is a small, fast and lightweight open source Web Server for GNU/Linux. 
> It has been designed with focus in embedded devices, therefore its scalable by 
> nature having a low memory and CPU consumption and an excellent performance.
> 
> Signed-off-by: Julien Corjon <corjon.j@ecagroup.com>
> ---
> Changes v1 -> v2:
>   - Fix typo (suggested by Alexandre)
>   - Change autotools for generic-package (suggested by Thomas)
>   - Add hash file
>   - Add licence
>   - Add uClib & musl support
>   - Add shared library option

I've applied the patch, but really there was still a lot of things
broken. The most astonishing problem was that with your monkey.mk,
Monkey was built for the host and not for the target. So I really don't
see how you could have tested this, unless you're working on x86/x86-64
embedded systems (but I know you are not!).

So, I've fixed up a whole bunch of issues before applying:

    [Thomas:
      - Add missing dependency on !BR2_STATIC_LIBS (the source code uses
        dlopen) and BR2_USE_MMU (the source code uses fork)
      - Slightly adjust/reword the description of the
        BR2_PACKAGE_MONKEY_SHARED option.
      - Remove all the complicated installation logic for the target, and
        just use "make install" instead.
      - Pass --no-backtrace when uClibc is used, otherwise the build fails
        because <execinfo.h> is not available in uClibc.
      - Pass $(TARGET_CONFIGURE_OPTS) in the environment of the configure
        script., otherwise monkey gets built for the host and not for the
        target.
      - Add a post install target hook to remove a broken symlink
        libmonkey.so installed by Monkey's Makefile when the shared
        library is not enabled.
      - Use TARGET_MAKE_ENV when calling make, just because we should.
      - Pass --malloc-libc so that the libc malloc() is used instead of
        the builtin jemalloc allocator, which requires more work to
        cross-compile properly.
      - Add missing empty line after the .mk header and before the first
        variable definition.]

Things like dependencies on BR2_STATIC_LIBS/BR2_USE_MMU, I understand
that they could have slipped through. But the fact that the binaries
got built for the host indicates that you haven't even built+tested the
package.

Also, if you could post a follow-up patch adding an init script that
starts Monkey at boot time, it would be great.

Thanks!

Thomas

Dear Thomas,

> ...
> I've applied the patch, but really there was still a lot of things
> broken. The most astonishing problem was that with your monkey.mk,
> Monkey was built for the host and not for the target. So I really don't
> see how you could have tested this, unless you're working on x86/x86-64
> embedded systems (but I know you are not!).

You're right, with the autotool -> generic-packages rework I just 
checked the compilation and didn't notice that x86 GCC was used. It 
won't happen again, next time I will fully test patches after each rework.

> So, I've fixed up a whole bunch of issues before applying:
>
>      [Thomas:
>        - Add missing dependency on !BR2_STATIC_LIBS (the source code uses
>          dlopen) and BR2_USE_MMU (the source code uses fork)

Do you have a documentation on "BR dependencies versus functions" to 
learn about that?

>        - Slightly adjust/reword the description of the
>          BR2_PACKAGE_MONKEY_SHARED option.
>        - Remove all the complicated installation logic for the target, and
>          just use "make install" instead.

I didn't know that BR remove all headers from the target image at the 
end of the compilation. Is it the same for the man files?

>        - Pass --no-backtrace when uClibc is used, otherwise the build fails
>          because <execinfo.h> is not available in uClibc.

As I daily use a linaro toolchain, I did not test the uClib and Musl 
configure options. In this case (for future patches), should I include a 
case I cannot test or disable it with depend?

>        - Pass $(TARGET_CONFIGURE_OPTS) in the environment of the configure
>          script., otherwise monkey gets built for the host and not for the
>          target.

Slap me for this one!

>        - Add a post install target hook to remove a broken symlink
>          libmonkey.so installed by Monkey's Makefile when the shared
>          library is not enabled.
>        - Use TARGET_MAKE_ENV when calling make, just because we should.
>        - Pass --malloc-libc so that the libc malloc() is used instead of
>          the builtin jemalloc allocator, which requires more work to
>          cross-compile properly.
>        - Add missing empty line after the .mk header and before the first
>          variable definition.]

> ...

> Also, if you could post a follow-up patch adding an init script that
> starts Monkey at boot time, it would be great.

I notice there is an systemd start script debian/monkey.init but my 
knowledge of init daemons is so limited that I didn't try to 
include/modify this file in BR package declaration.

>
> Thanks!
>
> Thomas

Thanks for the review/fix/commit. I'll try to be more meticulous on the 
next one.

Julien

Hello,

On Wed, 1 Jul 2015 07:27:40 +0000, Julien CORJON wrote:

> You're right, with the autotool -> generic-packages rework I just 
> checked the compilation and didn't notice that x86 GCC was used. It 
> won't happen again, next time I will fully test patches after each rework.

Ah, ok, I understand. Indeed when using the autotools-package infra,
TARGET_CONFIGURE_OPTS is automatically passed in the environment of
configure. But as discussed previously, this package is not using
autoconf/automake, so it doesn't make a lot of sense to use the
autotools-package infrastructure for it.

> >      [Thomas:
> >        - Add missing dependency on !BR2_STATIC_LIBS (the source code uses
> >          dlopen) and BR2_USE_MMU (the source code uses fork)
> 
> Do you have a documentation on "BR dependencies versus functions" to 
> learn about that?

No, not really. And it's not necessarily a 1:1 mapping. For example,
usage of dlopen() is an indication that you should depend
on !BR2_STATIC_LIBS. But there are also other situations where you need
to depend on !BR2_STATIC_LIBS: when the package tries to build a shared
library unconditionally.

Seeing fork() being called is a good indication that you need a depends
on BR2_USE_MMU.

For the other dependencies like BR2_TOOLCHAIN_HAS_THREADS, you can grep
for pthread functions.

For wide-char/locales, it's a bit more complicated.

But it's not too dramatic if you don't get these correct, as the
autobuilders will catch such problems.

> >        - Slightly adjust/reword the description of the
> >          BR2_PACKAGE_MONKEY_SHARED option.
> >        - Remove all the complicated installation logic for the target, and
> >          just use "make install" instead.
> 
> I didn't know that BR remove all headers from the target image at the 
> end of the compilation. Is it the same for the man files?

Yes. Look at what the target-finalize target is doing at
http://git.buildroot.net/buildroot/tree/Makefile#n547. This is executed
at the end of the build: after all packages have been built/installed,
but before the root filesystem images are generated.

> >        - Pass --no-backtrace when uClibc is used, otherwise the build fails
> >          because <execinfo.h> is not available in uClibc.
> 
> As I daily use a linaro toolchain, I did not test the uClib and Musl 
> configure options. In this case (for future patches), should I include a 
> case I cannot test or disable it with depend?

No, we really prefer when the uClibc situation has been at least built
tested: uClibc is still our default C library, so it's a bit weird to
make a package depends on !BR2_TOOLCHAIN_USES_UCLIBC, if it really can
be used with uClibc.

Especially since your package had some specific conditionals for uClibc
and Musl handling.

But to be honest, I did not built the musl case. The autobuilders will
take care of that.

Remember that you have pre-built uClibc toolchains available at
http://autobuild.buildroot.org/toolchains/tarballs/, which allows you
do quickly build test a package with uClibc.

Here I have a script called br-init-conf, which does:

#!/bin/sh
wget -O .config http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config
cat /home/thomas/projets/br/minimal.defconfig >> .config

With minimal.defconfig being:

BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
# BR2_TARGET_ROOTFS_TAR is not set

This allows to initialize a minimal Buildroot configuration that uses a
pre-built uClibc toolchain. Excellent to do a quick test build.

> > Also, if you could post a follow-up patch adding an init script that
> > starts Monkey at boot time, it would be great.
> 
> I notice there is an systemd start script debian/monkey.init but my 
> knowledge of init daemons is so limited that I didn't try to 
> include/modify this file in BR package declaration.

Ok, no problem.

Thanks,

Thomas