405343 - configure.in uses wrong variable to pass -lcurl to AC_TRY_LINK for curl detection for crashreporter on linux

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Mart Raudsepp]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.9) Gecko/20071112 SeaMonkey/1.1.6
Build Identifier: http://releases.mozilla.org/pub/mozilla.org/firefox/releases/3.0b1/source/firefox-3.0b1-source.tar.bz2

Since this commit:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla&command=DIFF_FRAMESET&file=configure.in&rev2=1.1853&rev1=1.1852

LDFLAGS is used to pass -lcurl for testing curl availability with a test program.
However this fails with --as-needed, because it's a completely wrong variable to use for this. As the autoconf manual says:

"Don't use this variable to pass library names (-l) to the linker; use LIBS instead." (source: http://www.gnu.org/software/autoconf/manual/autoconf.html#index-LDFLAGS-94)

So, LIBS MUST be used, not LDFLAGS, or it wrongly fails with LDFLAGS="-Wl,--as-needed" or some other non-common environments, as the gcc command executed will put -lcurl before conftest.c, not after as it must be. Command line argument order does matter.


Reproducible: Always

Steps to Reproduce:
1. configure with LDFLAGS="-Wl,--as-needed" ./configure

Actual Results:  
checking for libcurl... no
configure: error: Couldn't find libcurl, which is required for the crash reporter.  Use --disable-crashreporter to disable the crash reporter.


Expected Results:  
checking for libcurl... yes


Patch to follow

Comment 1

[Mart Raudsepp]

Attachment: Patch against configure.in, so that it works after an autoconf-2.13 run

Would appreciate if Firefox3_final wouldn't have a broken build in default configuration options due to this...

Comment 2

[Mart Raudsepp]

Note that there might be other LDFLAGS abusing in configure.in file(s). I noticed a -lz passing in the same configure.in through LDFLAGS, but the code surrounding it seems to suggest that usually that doesn't matter much, but might still be wrong..

Furthermore, I don't understand why it is not using pkg-config anymore to check libcurl availability. I believe that pkg-config checks should always be preferred, if the .pc files are available. But for some reason the above mentioned commit removed that usage. Instead it could first check through pkg-config, but not die out if it fails (due to maybe some old libcurl version not shipping a .pc file), but fallback to the current code - that way the preferred pkg-config way is preferred..

Comment 3

[(not currently active) Ted Mielczarek]

pkg-config was picking up extra libs (openssl, etc) in the link line for libcurl, which was bogus.  This is a valid bug, but we're going to remove this test entirely when we fix bug 392919 (which will be soon) so I'm marking this WONTFIX.

We'd probably take a patch to fix any other instances of this in configure.in if it breaks certain configs.

Comment 4

[(not currently active) Ted Mielczarek]

On second thought, I didn't get around to this in bug 392919, but I'm still not sure I want the patch here.  We can probably just change this to an AC_CHECK_HEADERS test since we're no longer linking with libcurl.

Comment 5

[(not currently active) Ted Mielczarek]

Fixing this in bug 404855.