diff mbox series

dropbear: Do not build static binary

Message ID 20180503114701.9452-2-stefan.sorensen@spectralink.com
State Changes Requested
Headers show
Series dropbear: Do not build static binary | expand

Commit Message

Sørensen, Stefan May 3, 2018, 11:47 a.m. UTC 
Dropbear 2018.76 now uses the --enable-static option to indicate that a static
binary should be built. This will incorrectly pick up the generic buildroot
option intended for building static libraries, causing an unwanted static
binary build with BR2_STATIC_LIBS/BR2_SHARED_STATIC_LIBS.

Fix by appending an --disable-static configure flag, overriding the buildroot
default.
---
 package/dropbear/dropbear.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Baruch Siach May 3, 2018, 6:08 p.m. UTC | #1 
Hi Stefan,

On Thu, May 03, 2018 at 01:47:00PM +0200, Stefan Sørensen wrote:
> Dropbear 2018.76 now uses the --enable-static option to indicate that a static
> binary should be built. This will incorrectly pick up the generic buildroot
> option intended for building static libraries, causing an unwanted static
> binary build with BR2_STATIC_LIBS/BR2_SHARED_STATIC_LIBS.
> 
> Fix by appending an --disable-static configure flag, overriding the buildroot
> default.

Your sign-off is missing here.

> ---
>  package/dropbear/dropbear.mk | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
> index 6bfc05fb2b..f47f97d036 100644
> --- a/package/dropbear/dropbear.mk
> +++ b/package/dropbear/dropbear.mk
> @@ -28,9 +28,10 @@ DROPBEAR_MAKE = \
>  	$(MAKE) MULTI=1 SCPPROGRESS=1 \
>  	PROGRAMS="$(DROPBEAR_PROGRAMS)"
>  
> -ifeq ($(BR2_STATIC_LIBS),y)
> -DROPBEAR_CONF_OPTS += --enable-static
> -endif
> +# The generic --enable-static flags is only intended for use when building
> +# libraries, but dropbear will be built as a static executeable with this
> +# flag, so we overide it here
> +DROPBEAR_CONF_OPTS += --disable-static

You add --disable-static unconditionally, but we do want to build statically 
when BR2_STATIC_LIBS=y. So maybe add --disable-static only for 
BR2_SHARED_STATIC_LIBS=y to counter the effect of --enable-static that 
Buildroot adds automatically. Would that work for you?

>  # Ensure that dropbear doesn't use crypt() when it's not available
>  define DROPBEAR_SVR_PASSWORD_AUTH

baruch
Sørensen, Stefan May 4, 2018, 6:27 a.m. UTC | #2 
On Thu, 2018-05-03 at 21:08 +0300, Baruch Siach wrote:
> > -ifeq ($(BR2_STATIC_LIBS),y)
> > -DROPBEAR_CONF_OPTS += --enable-static
> > -endif
> > +# The generic --enable-static flags is only intended for use when
> > building
> > +# libraries, but dropbear will be built as a static executeable
> > with this
> > +# flag, so we overide it here
> > +DROPBEAR_CONF_OPTS += --disable-static
> 
> You add --disable-static unconditionally, but we do want to build
> statically 
> when BR2_STATIC_LIBS=y. So maybe add --disable-static only for 
> BR2_SHARED_STATIC_LIBS=y to counter the effect of --enable-static
> that Buildroot adds automatically. Would that work for you?

The --disable-static causes dropbear to do nothing, i.e. just do a
normal link, so that will also work with BR2_STATIC_LIBS=y.

Stefan
Baruch Siach May 6, 2018, 6:39 p.m. UTC | #3 
Hi Sørensen,

On Fri, May 04, 2018 at 06:27:02AM +0000, Sørensen, Stefan wrote:
> On Thu, 2018-05-03 at 21:08 +0300, Baruch Siach wrote:
> > > -ifeq ($(BR2_STATIC_LIBS),y)
> > > -DROPBEAR_CONF_OPTS += --enable-static
> > > -endif
> > > +# The generic --enable-static flags is only intended for use when
> > > building
> > > +# libraries, but dropbear will be built as a static executeable
> > > with this
> > > +# flag, so we overide it here
> > > +DROPBEAR_CONF_OPTS += --disable-static
> > 
> > You add --disable-static unconditionally, but we do want to build
> > statically 
> > when BR2_STATIC_LIBS=y. So maybe add --disable-static only for 
> > BR2_SHARED_STATIC_LIBS=y to counter the effect of --enable-static
> > that Buildroot adds automatically. Would that work for you?
> 
> The --disable-static causes dropbear to do nothing, i.e. just do a
> normal link, so that will also work with BR2_STATIC_LIBS=y.

So if --disable-static is a NOP why do we need it at all?

As far as I can see, the only effect of --enable-static is to add -static to 
dropbear binaries link command line via LDFLAGS. This is the expected 
behaviour for BR2_STATIC_LIBS=y. Since BR2_STATIC_LIBS=y is sometimes used 
with external toolchains that provide both shared and static libraries, the 
-static link option is required to produce static binaries. Your patch seems 
to break this use case.

What is this patch meant to fix?

baruch
diff mbox series

Patch

diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
index 6bfc05fb2b..f47f97d036 100644
--- a/package/dropbear/dropbear.mk
+++ b/package/dropbear/dropbear.mk
@@ -28,9 +28,10 @@  DROPBEAR_MAKE = \
 	$(MAKE) MULTI=1 SCPPROGRESS=1 \
 	PROGRAMS="$(DROPBEAR_PROGRAMS)"
 
-ifeq ($(BR2_STATIC_LIBS),y)
-DROPBEAR_CONF_OPTS += --enable-static
-endif
+# The generic --enable-static flags is only intended for use when building
+# libraries, but dropbear will be built as a static executeable with this
+# flag, so we overide it here
+DROPBEAR_CONF_OPTS += --disable-static
 
 # Ensure that dropbear doesn't use crypt() when it's not available
 define DROPBEAR_SVR_PASSWORD_AUTH