Re: [PATCH] crtd linking issues

On 06/08/2012 05:22 AM, Amos Jeffries wrote:
> On 8/06/2012 11:01 p.m., Alexander Komyagin wrote:
>> On Thu, 2012-06-07 at 17:13 -0600, Alex Rousskov wrote:
>>> On 06/07/2012 10:11 AM, Alexander Komyagin wrote:
>>>> When compiling squid 3.2 with crtd enabled, I experienced link errors
>>>> due to non-present libsslutil.
>>>>
>>>> I've seen some similar messages, but none had the exact patch. So I
>>>> made
>>>> the one myself (could it be pushed to upstream ?) :
>>>>
>>>> --- a/src/ssl/Makefile.am 2012-03-07 06:42:55.000000000 +0400
>>>> +++ b/src/ssl/Makefile.am 2012-06-07 19:25:52.391644141 +0400
>>>> @@ -45,5 +45,5 @@
>>>>
>>>> if USE_SSL_CRTD
>>>> ssl_crtd_SOURCES = ssl_crtd.cc certificate_db.cc certificate_db.h
>>>> -ssl_crtd_LDADD = $(SSLLIB) -lsslutil $(COMPAT_LIB)
>>>> +ssl_crtd_LDADD = libsslutil.la $(SSLLIB) -lsslutil $(COMPAT_LIB)
>>>> endif
>>>
>>> Hello Alexander,
>>>
>>> We already have -lsslutil in ssl_crtd_LDADD. One of the two
>>> references to the libutil.la library has to be removed. I found this in
>>> automake documentation:
>>>
>>>> `maude_LDADD'
>>>> Extra objects (`*.$(OBJEXT)') and libraries (`*.a', `*.la')
>>>> can be
>>>> added to a _program_ by listing them in the `_LDADD' variable.
>>> and, more importantly:
>>>
>>>> We recommend that you avoid using `-l' options in `LDADD' or
>>>> `PROG_LDADD' when referring to libraries built by your package.
>>>> Instead, write the file name of the library explicitly [...]
>>>> Use `-l' only to list third-party libraries.
>>>
>>> It looks like -lsslutil should be _replaced_ with libsslutil.la. Could
>>> you test that theory, please?
>>>
>> Hello Alex. You definitely right - no reason to add the same lib twice.
>> Without -lsslutil crtd links and works fine. So the patch may be
>> altered:
>>
>> --- a/src/ssl/Makefile.am 2012-03-07 06:42:55.000000000 +0400
>> +++ b/src/ssl/Makefile.am 2012-06-07 19:25:52.391644141 +0400
>> @@ -45,5 +45,5 @@
>>
>> if USE_SSL_CRTD
>> ssl_crtd_SOURCES = ssl_crtd.cc certificate_db.cc certificate_db.h
>> -ssl_crtd_LDADD = $(SSLLIB) -lsslutil $(COMPAT_LIB)
>> +ssl_crtd_LDADD = libsslutil.la $(SSLLIB) $(COMPAT_LIB)
>> endif
>>
>>
>>> Thank you,
>>>
>>> Alex.
>
> Excellent. Thank you.
>
> +1 from me on this and applying immediately.

The committed patch changes the order in which the libraries are added.
I hope that does not cause problems in this particular case, but, in
general, we should not make such changes accidentally.

Alex.
Received on Fri Jun 08 2012 - 14:56:01 MDT