diff mbox

dhcpdump: fix static build

Message ID a2d055301fd7b62deb7599263ad3ff0ee94c1870.1398283199.git.baruch@tkos.co.il
State Accepted
Commit 429f4415cd153c6809394a8b3245d4d15bba3ec3
Headers show

Commit Message

Baruch Siach April 23, 2014, 7:59 p.m. UTC
Use pcap-config to list optional libpcap dependencies that we need to list
when building statically.

Fixes:
http://autobuild.buildroot.net/results/110/1107c21cdf656763bf7048c6c5c7899369724f5f/

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 package/dhcpdump/dhcpdump.mk | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni April 23, 2014, 8:52 p.m. UTC | #1
Dear Baruch Siach,

On Wed, 23 Apr 2014 22:59:59 +0300, Baruch Siach wrote:

> +DHCPDUMP_LIBS = -lpcap
> +ifeq ($(BR2_PREFER_STATIC_LIB),y)
> +DHCPDUMP_LIBS += $(shell $(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs)
> +endif

This is not at all against your patch specifically, but I'm a bit
worried about all the static linking related kludges we add all over
the place. Is this normal? Shouldn't we fix the packages themselves and
submit patches upstream?

Thomas
Baruch Siach April 24, 2014, 4:29 a.m. UTC | #2
Hi Thomas,

On Wed, Apr 23, 2014 at 10:52:43PM +0200, Thomas Petazzoni wrote:
> On Wed, 23 Apr 2014 22:59:59 +0300, Baruch Siach wrote:
> > +DHCPDUMP_LIBS = -lpcap
> > +ifeq ($(BR2_PREFER_STATIC_LIB),y)
> > +DHCPDUMP_LIBS += $(shell $(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs)
> > +endif
> 
> This is not at all against your patch specifically, but I'm a bit
> worried about all the static linking related kludges we add all over
> the place. Is this normal? Shouldn't we fix the packages themselves and
> submit patches upstream?

Upstream submission is of course preferable. In this case upstream does not 
appear to be very active. The current release is from 2008, while development 
started at 2007.

Do you prefer a Makefile patch that could theoretically be upstreamed?

baruch
Thomas Petazzoni April 24, 2014, 7:16 a.m. UTC | #3
Dear Baruch Siach,

On Thu, 24 Apr 2014 07:29:11 +0300, Baruch Siach wrote:

> > This is not at all against your patch specifically, but I'm a bit
> > worried about all the static linking related kludges we add all over
> > the place. Is this normal? Shouldn't we fix the packages themselves and
> > submit patches upstream?
> 
> Upstream submission is of course preferable. In this case upstream does not 
> appear to be very active. The current release is from 2008, while development 
> started at 2007.
> 
> Do you prefer a Makefile patch that could theoretically be upstreamed?

I must say I don't know. In fact, I was not necessarily speaking
specifically of this particular change, but more generally about all
the LIBS='-lfoo' we're adding all over the place to fix static linking
problems. This seems a bit fragile to me.

And the point of raising this question was specifically to get a
discussion started, and see the opinion of others (Gustavo, Peter,
Arnout, Yann, etc.).

Thomas
Peter Korsgaard April 24, 2014, 3:23 p.m. UTC | #4
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Dear Baruch Siach,
 > On Wed, 23 Apr 2014 22:59:59 +0300, Baruch Siach wrote:

 >> +DHCPDUMP_LIBS = -lpcap
 >> +ifeq ($(BR2_PREFER_STATIC_LIB),y)
 >> +DHCPDUMP_LIBS += $(shell $(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs)
 >> +endif

 > This is not at all against your patch specifically, but I'm a bit
 > worried about all the static linking related kludges we add all over
 > the place. Is this normal? Shouldn't we fix the packages themselves and
 > submit patches upstream?

It would certainly be nice to get these things handled upstream, but
it's similar to the special-archs/nommu/uClibc stuff, E.G. not something
"normal" people gets affected by (and something we have decided to
support), so I don't think there's much else we can do.
Arnout Vandecappelle April 24, 2014, 10:01 p.m. UTC | #5
On 24/04/14 17:23, Peter Korsgaard wrote:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Dear Baruch Siach,
>  > On Wed, 23 Apr 2014 22:59:59 +0300, Baruch Siach wrote:
> 
>  >> +DHCPDUMP_LIBS = -lpcap
>  >> +ifeq ($(BR2_PREFER_STATIC_LIB),y)
>  >> +DHCPDUMP_LIBS += $(shell $(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs)
>  >> +endif
> 
>  > This is not at all against your patch specifically, but I'm a bit
>  > worried about all the static linking related kludges we add all over
>  > the place. Is this normal? Shouldn't we fix the packages themselves and
>  > submit patches upstream?
> 
> It would certainly be nice to get these things handled upstream, but
> it's similar to the special-archs/nommu/uClibc stuff, E.G. not something
> "normal" people gets affected by (and something we have decided to
> support), so I don't think there's much else we can do.

 Still, I expect that upstreams are more likely to be willing to accept
patches for linking against a static library than patches for exotic
systems. Especially since the former is usually only a config patch,
while the special-archs/nommu/uClibc stuff is more likely to affect the
code itself.

 Regards,
 Arnout
Peter Korsgaard May 7, 2014, 8:48 p.m. UTC | #6
On Wed, Apr 23, 2014 at 9:59 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Use pcap-config to list optional libpcap dependencies that we need to list
> when building statically.
>
> Fixes:
> http://autobuild.buildroot.net/results/110/1107c21cdf656763bf7048c6c5c7899369724f5f/
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Committed, thanks.
diff mbox

Patch

diff --git a/package/dhcpdump/dhcpdump.mk b/package/dhcpdump/dhcpdump.mk
index cb2daa973177..ded7c295c3ae 100644
--- a/package/dhcpdump/dhcpdump.mk
+++ b/package/dhcpdump/dhcpdump.mk
@@ -10,8 +10,13 @@  DHCPDUMP_DEPENDENCIES = libpcap
 DHCPDUMP_LICENSE = BSD-2c
 DHCPDUMP_LICENSE_FILES = LICENSE
 
+DHCPDUMP_LIBS = -lpcap
+ifeq ($(BR2_PREFER_STATIC_LIB),y)
+DHCPDUMP_LIBS += $(shell $(STAGING_DIR)/usr/bin/pcap-config --static --additional-libs)
+endif
+
 define DHCPDUMP_BUILD_CMDS
-	$(MAKE) -C $(@D) CC="$(TARGET_CC) $(TARGET_CFLAGS)"
+	$(MAKE) -C $(@D) CC="$(TARGET_CC) $(TARGET_CFLAGS)" LIBS="$(DHCPDUMP_LIBS)"
 endef
 
 define DHCPDUMP_INSTALL_TARGET_CMDS