[1/1] package/tini: compile executable static

tini is intended to be used as the init process within containers and
therefore needs to be independent of the C runtime library.

Signed-off-by: Simon Rowe <simon.rowe@citrix.com>
---
 package/tini/tini.mk | 1 +
 1 file changed, 1 insertion(+)

Simon, All,

On 2019-11-20 13:09 +0000, Simon Rowe spake thusly:
> tini is intended to be used as the init process within containers and
> therefore needs to be independent of the C runtime library.
> 
> Signed-off-by: Simon Rowe <simon.rowe@citrix.com>
> ---
>  package/tini/tini.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/tini/tini.mk b/package/tini/tini.mk
> index 5dd9877..986e612 100644
> --- a/package/tini/tini.mk
> +++ b/package/tini/tini.mk
> @@ -10,6 +10,7 @@ TINI_LICENSE = MIT
>  TINI_LICENSE_FILES = LICENSE
>  
>  TINI_CFLAGS = $(TARGET_CFLAGS) \
> +	-static \

That can't work as-is, because this would break with BR2_SHARED_LIBS=y,
in which case only shared libraries are available.

Furthermore, the commit message introducing tini explicitly states "it
is not necessary to compile Tini statically for many non-docker
container environments". So, what has changed since then?

Besides, I would expect that the init process of a container would be
provided inside the container, not come from the host system. As such,
the init system of the container can be dynamically linked with the
runtime of the container itself.

In any case, this patch is incorrect in the shared-only setup, so I
marked it as rejected for now. If you can provide a beter way to do what
you need, then we can revisit it later with a different implementation
(and more explanations on how this is used in the wild).

Regards,
Yann E. MORIN.

>  	-DTINI_VERSION=\"$(TINI_VERSION)\" \
>  	-DTINI_GIT=\"\"
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

On 23/11/2019 21:15, Yann E. MORIN wrote:

> That can't work as-is, because this would break with BR2_SHARED_LIBS=y,
> in which case only shared libraries are available.

Thanks for pointing this out.

> Furthermore, the commit message introducing tini explicitly states "it
> is not necessary to compile Tini statically for many non-docker
> container environments". So, what has changed since then?
> 
> Besides, I would expect that the init process of a container would be
> provided inside the container, not come from the host system. As such,
> the init system of the container can be dynamically linked with the
> runtime of the container itself.

Passing --init to "docker run" takes whatever the executable docker-init 
in the OS filesystem is and uses it as the process for pid 1 in the 
container. If the OS is buildroot and the container uses glibc that 
doesn't work.

> In any case, this patch is incorrect in the shared-only setup, so I
> marked it as rejected for now. If you can provide a beter way to do what
> you need, then we can revisit it later with a different implementation
> (and more explanations on how this is used in the wild).

I can add the static linking as an option. I would have done so but it 
was suggested here

http://lists.busybox.net/pipermail/buildroot/2017-October/204974.html

that an init should always be statically linked.

In summary I can make this optional and mutually exclusive with 
BR2_SHARED_LIBS, would that be acceptable?

Simon

On 25/11/2019 09:23, Simon Rowe wrote:
> If the OS is buildroot and the container uses glibc that 
> doesn't work.

I meant to say "If buildroot uses musl and the container uses glibc ..."

Simon

On 25/11/2019 10:23, Simon Rowe wrote:
> On 23/11/2019 21:15, Yann E. MORIN wrote:
> 
>> That can't work as-is, because this would break with BR2_SHARED_LIBS=y,
>> in which case only shared libraries are available.
> 
> Thanks for pointing this out.

 Well, actually, we really should keep at least libc.a around even if
BR2_SHARED_LIBS=y, because some packages (with no dependencies, and that don't
use NSS - like tini) may still want to link statically.

 In fact, I think we already keep libc.a around in all possible cases even if
BR2_SHARED_LIBS=y, so I think this patch will work as is...


>> Furthermore, the commit message introducing tini explicitly states "it
>> is not necessary to compile Tini statically for many non-docker
>> container environments". So, what has changed since then?

 Maybe Christian can explain? I vaguely remember that discussion from a year
ago. IIRC, Christian initially did exactly this: link tini statically, always.
But then it turned out not to be needed.


>> Besides, I would expect that the init process of a container would be
>> provided inside the container, not come from the host system. 

 No, the point of tini is exactly to avoid using the init inside the container -
either because it's doing too much (e.g. full systemd) or because it doesn't
exist (container image with just the application).

>> As such,
>> the init system of the container can be dynamically linked with the
>> runtime of the container itself.
> 
> Passing --init to "docker run" takes whatever the executable docker-init in the
> OS filesystem is and uses it as the process for pid 1 in the container. If the
> OS is buildroot and the container uses glibc that doesn't work.
> 
>> In any case, this patch is incorrect in the shared-only setup, so I
>> marked it as rejected for now. If you can provide a beter way to do what
>> you need, then we can revisit it later with a different implementation
>> (and more explanations on how this is used in the wild).
> 
> I can add the static linking as an option. I would have done so but it was
> suggested here
> 
> http://lists.busybox.net/pipermail/buildroot/2017-October/204974.html
> 
> that an init should always be statically linked.
> 
> In summary I can make this optional and mutually exclusive with BR2_SHARED_LIBS,
> would that be acceptable?

 Could you check with BR2_SHARED_LIBS=y and the current patch if it's possible
to build tini for all cases? I.e., external toolchain, internal with musl,
glibc, uClibc. If not, then you can make it conditional on !shared like this:

	$(if $(BR2_SHARED_LIBS),,-static)

 And of course, we still need an explanation why it is needed after all while
before we thought it wasn't.

 Regards,
 Arnout

On 01/12/2019 14:41, Arnout Vandecappelle wrote:

>   Could you check with BR2_SHARED_LIBS=y and the current patch if it's possible
> to build tini for all cases? I.e., external toolchain, internal with musl,
> glibc, uClibc. If not, then you can make it conditional on !shared like this:
> 
> 	$(if $(BR2_SHARED_LIBS),,-static)

I've run the current patch with the following toolchains

* internal, only shared libraries, musl
* internal, only shared libraries, uClibc
* internal, only shared libraries, glibc
* external, only shared libraries, musl

and tini is built correctly.

>   And of course, we still need an explanation why it is needed after all while
> before we thought it wasn't.

Does this need an expansion of the commit message or something elsewhere?

Simon

On 03/12/2019 14:14, Simon Rowe wrote:
> On 01/12/2019 14:41, Arnout Vandecappelle wrote:
> 
>>   Could you check with BR2_SHARED_LIBS=y and the current patch if it's possible
>> to build tini for all cases? I.e., external toolchain, internal with musl,
>> glibc, uClibc. If not, then you can make it conditional on !shared like this:
>>
>>     $(if $(BR2_SHARED_LIBS),,-static)
> 
> I've run the current patch with the following toolchains
> 
> * internal, only shared libraries, musl
> * internal, only shared libraries, uClibc
> * internal, only shared libraries, glibc
> * external, only shared libraries, musl
> 
> and tini is built correctly.

 Excellent!

 So the patch was OK, just the commit message should be updated.

> 
>>   And of course, we still need an explanation why it is needed after all while
>> before we thought it wasn't.
> 
> Does this need an expansion of the commit message or something elsewhere?

 Commit message.

 Thanks!

 Regards,
 Arnout