Bug 333803 – net-analyzer/hydra does not respect LDFLAGS

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Diego Elio Pettenò (RETIRED) 2010-08-21 14:45:45 UTC]

See attached build log.

(Please do not complain about the need for looking at the build log until you can provide an easy way to open bugs with the correct data picked out of a tinderbox log. Thanks.)

[Comment 1 Diego Elio Pettenò (RETIRED) 2010-08-21 14:46:10 UTC]

Created attachment 243865 [details]
Build log

[Comment 2 Kevin Pyle 2010-08-21 18:48:23 UTC]

Created attachment 243927 [details, diff]
Patch to hydra-5.4-r3.ebuild to fix numerous QA issues

This package needed some significant attention:

- Ignored CPPFLAGS (minor)
- Ignored LDFLAGS
- Indirectly ignored failure to link hydra, by way of an ||echo that ate the error
- Explicitly ignored failure to link pw-inspector
- Failed to link with USE=-ssl, since the ebuild only applies -lcrypto if USE=ssl, but hydra-sip.c is built always and uses MD5 functions from openssl.
- Forced CC=gcc, breaking cross-compilation and complicating heterogeneous distcc farms.

This patch addresses the problems in the following ways:
- $CPPFLAGS added to the existing $CFLAGS fix
- $LDFLAGS added where needed.  For convenience, I spliced in the value of LDFLAGS as present in the ebuild environment, rather than adding a reference to the Make variable $(LDFLAGS)
- Deleted the ||echo.  Its advice is of minimal use to Gentoo users.
- Deleted the dash that ignored errors building pw-inspector
- Moved -lcrypto to be always in XLIBS.  Moved dev-libs/openssl outside the ssl? check in $DEPEND.
- Truncated ${S}/Makefile.unix, which contained only CC=gcc and STRIP=strip.  STRIP is set again anyway by configure.  Inherited toolchain-funcs and used tc-export CC so that a CHOST-qualified CC is used.

Also, I added a note next to the invocation of the configure script to warn future maintainers that, despite the extensive use of autotools based names, the top level files are not autotools.  Though oddly, the Gtk frontend is autotools based.

[Comment 3 Jeroen Roovers (RETIRED) 2010-08-22 22:16:56 UTC]

Thanks for the patch.

--- a/hydra-5.4-r3.ebuild	2010-05-21 16:06:40.000000000 +0000
+++ b/hydra-5.4-r3.ebuild	2010-08-21 18:32:02.738992004 +0000
@@ -15,21 +15,30 @@

[...]
 
 src_prepare() {
-	sed -i "s:-O2:${CFLAGS}:" Makefile.am || die "sed failed"
+	sed -i \
+		-e "s:-O2:${CPPFLAGS} ${CFLAGS}:" \
+		-e 's:|| echo.*$::' \
+		-e '/\t-$(CC)/s/-//' \
+		-e "/ -o /s:\$(OPTS):& ${LDFLAGS}:" \
+		Makefile.am || die "sed failed"

That's injecting the *FLAGS into the Makefile - it's better to set those in the environment and let make pick them up using $(*FLAGS).

Other than that, all the changes were OK and have been merged into version 5.7.

[Comment 4 Kevin Pyle 2010-08-22 22:40:12 UTC]

(In reply to comment #3)
> That's injecting the *FLAGS into the Makefile - it's better to set those in the
> environment and let make pick them up using $(*FLAGS).

Existing ebuilds are inconsistent about that.  Some inject it, as was done before my patch (see how the CFLAGS were handled in the 5.4-r3 ebuild), and that style seemed fairly common in the ebuilds I have looked at while preparing patches.  I know that inserting it as $() instead of injecting via ${} makes it easier to send upstream.  Is there any other benefit?