[squid-dev] [PATCH] Fix libatomic detection

[Kinkie]

Hi,
  I haven't checked, but it should.
MacOS builds for me right now; probably as a result of the LIBS
side-effect (although I get a "none required" result).
Attached patch fixes, build still failes due to shm not working at
runtime (thus testRock failing), but for me it's a fix candidate.

On Tue, Jan 5, 2016 at 9:06 AM, Amos Jeffries <squid3 at treenet.co.nz> wrote:
> On 5/01/2016 7:34 a.m., Kinkie wrote:
>> On Mon, Jan 4, 2016 at 6:47 AM, Amos Jeffries <squid3 at treenet.co.nz> wrote:
>>> On 28/12/2015 10:25 p.m., Kinkie wrote:
>>>> Hi,
>>>>
>>>> On Mon, Dec 28, 2015 at 1:32 AM, Amos Jeffries wrote:
>>>>> On 24/12/2015 11:32 a.m., Kinkie wrote:
>>>>>> Hi,
>>>>>>   libatomic detection is currently broken in configure.ac; it will
>>>>>> define -latomic even in case where it wouldn't be required (e.g.
>>>>>> because it's already provided by the compiler).
>>>>>> The attached patch fixes the broken case. Unfortunately I don't know a
>>>>>> system where this library is required, I'm convinced there's room for
>>>>>> further simplification.
>>>>>
>>>>> It was for Clang builds IIRC. At least 3.5 needs it added.
>>>>>
>>>>> Does this work instead for shorter code?
>>>>>   AC_SEARCH_LIBS([__atomic_load_8],[atomic],[
>>>>>     ATOMICLIB="$ac_cv_search___atomic_load_8"
>>>>>   ],[])
>>>>
>>>> unfortunately not, as the result could be "none required" or "no",
>>>> too. The former is actually the error case which bought me to the
>>>> topic.
>>>> Documentation states that if a library is needed it should get
>>>> automatically added to LIBS though.
>>>
>>> "Unless the 3rd and 4th parameters are specified". So it will be
>>> automatically added under your patch, but not under the existing/older code.
>>>
>>> Which was an intentional omission from LIBS to avoid build warnings on
>>> some distros about unnecessary dependencies being linked in (such as
>>> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=770928>). Since most
>>> of the helper binaries do not use atomics (yet, though we could thread
>>> them in future) the -latomic addition is only needed by squid_LDADD not
>>> the generic LIBS / LDADD.
>>>
>>> Amos
>>>
>>
>> Are you sure? This from the doc I could find:
>> ===
>> the default action-if-found code, adding the library to the LIBS
>> variables, is always executed, even if a parameter is passed
>> ===
>>
>> I have no objection on the intention, of course.
>> If you are right, then the one-liner below should then do the trick.
>> If I am right, we need to go through the
>> SQUID_SAVE_STATE/SQUID_ROLLBACK_STATE dance on top of that.
>>
>> === modified file 'configure.ac'
>> --- configure.ac    2016-01-04 14:39:06 +0000
>> +++ configure.ac    2016-01-04 18:25:48 +0000
>> @@ -458,7 +458,7 @@
>>  fi
>>
>>  ## check for atomics library before anything that might need it
>> -AC_SEARCH_LIBS([__atomic_load_8],[atomic])
>> +AC_SEARCH_LIBS([__atomic_load_8],[atomic],[],[])
>>  if test "x$ac_cv_search___atomic_load_8" = "-latomic"; then
>>    ATOMICLIB="-latomic"
>>  fi
>>
>
> Seems I was wrong. I have just checked the two versions and the above
> patch generates exactly the same code as current trunk and sets LIBS. So
> we will have to do the state preservation dance. But I think wait until
> we have evidence of complaints for that.
>
> I also see the test is missing the "x" on the RHS of the conditional.
> Which is a bigger problem for clang where it needs to be set.
>
> PS. this is all fixes bug 4393 right?
>
> Amos
>
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev



-- 
    Francesco
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug4393-v1.patch
Type: text/x-diff
Size: 559 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20160105/e5c9b51b/attachment.patch>