[Mesa-dev] [PATCH] Use IMP_LIB_EXT when checking for LLVM shared libraries

[Emil Velikov]

On 8 September 2015 at 16:19, Tom Stellard <tom at stellard.net> wrote:
> On Mon, Sep 07, 2015 at 10:06:49AM +0100, Emil Velikov wrote:
>> Hi Jon,
>>
>> On 4 September 2015 at 14:00, Jon TURNEY <jon.turney at dronecode.org.uk> wrote:
>> > When checking for LLVM shared libraries, use IMP_LIB_EXT for the extension for
>> > shared libraries appropriate to the target, rather than hardcoding '.so'
>> >
>> > Also add some comments to explain why we have this circus of pain.
>> >
>> > Signed-off-by: Jon TURNEY <jon.turney at dronecode.org.uk>
>> > ---
>> >  configure.ac | 33 +++++++++++++++++++++++++++------
>> >  1 file changed, 27 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 90ba4fe..9960dd0 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -533,15 +533,32 @@ AM_CONDITIONAL(HAVE_COMPAT_SYMLINKS, test "x$HAVE_COMPAT_SYMLINKS" = xyes)
>> >  dnl
>> >  dnl library names
>> >  dnl
>> > +dnl Unfortunately we need to do a few things that libtool can't help us with,
>> > +dnl so we need some knowledge of shared library filenames:
>> > +dnl
>> > +dnl LIB_EXT is the extension used when creating symlinks for alternate
>> > +dnl filenames for a shared library which will be dynamically loaded
>> > +dnl
>> > +dnl IMP_LIB_EXT is the extension used when checking for the presence of a
>> > +dnl the file for a shared library we wish to link with
>> > +dnl
>> >  case "$host_os" in
>> >  darwin* )
>> > -    LIB_EXT='dylib' ;;
>> > +    LIB_EXT='dylib'
>> > +    IMP_LIB_EXT=$LIB_EXT
>> > +    ;;
>> >  cygwin* )
>> > -    LIB_EXT='dll' ;;
>> > +    LIB_EXT='dll'
>> > +    IMP_LIB_EXT='dll.a'
>> > +    ;;
>> >  aix* )
>> > -    LIB_EXT='a' ;;
>> > +    LIB_EXT='a'
>> > +    IMP_LIB_EXT=$LIB_EXT
>> > +    ;;
>> >  * )
>> > -    LIB_EXT='so' ;;
>> > +    LIB_EXT='so'
>> > +    IMP_LIB_EXT=$LIB_EXT
>> > +    ;;
>> >  esac
>> >
>> >  AC_SUBST([LIB_EXT])
>> > @@ -2163,10 +2180,14 @@ if test "x$MESA_LLVM" != x0; then
>> >
>> >      LLVM_LIBS="`$LLVM_CONFIG --libs ${LLVM_COMPONENTS}`"
>> >
>> > +    dnl llvm-config may not give the right answer when llvm is a built as a
>> > +    dnl single shared library, so we must work the library name out for
>> > +    dnl ourselves.
>> > +    dnl (See https://llvm.org/bugs/show_bug.cgi?id=6823)
>> >      if test "x$enable_llvm_shared_libs" = xyes; then
>> >          dnl We can't use $LLVM_VERSION because it has 'svn' stripped out,
>> >          LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version`
>> > -        AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.so"], [llvm_have_one_so=yes])
>> > +        AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], [llvm_have_one_so=yes])
>> >
>> >          if test "x$llvm_have_one_so" = xyes; then
>> >              dnl LLVM was built using auto*, so there is only one shared object.
>> > @@ -2174,7 +2195,7 @@ if test "x$MESA_LLVM" != x0; then
>> >          else
>> >              dnl If LLVM was built with CMake, there will be one shared object per
>> >              dnl component.
>> > -            AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.so"],
>> > +            AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"],
>> Fwiw I would prefer if we drop support for non have_one_so file LLVM
>> builds and just fail miserably, rather than having this around :-)
>> I suspect that either a) llvm people may have dropped support for it,
>> b) it's tested/used extremely rarely or c) busted with latest
>> llvm/mesa.
>>
>> Tom how do you feel on the subject ? Can we drop this workaround,
>> pretty please (/me does the puppy eyes)
>>
>
> I have an LLVM patch which I need to commit that should simplify a lot of this code.
> Lets hold this discussion until I get a chance to clean this up.
>
> I don't think we need to hold up this patch, though, it looks fine to me.
>
Ack. I've pushed this to master for now.

Tom,
Have you noticed that a few of your patches (wrt mesa-stable) has been
reviewed and/or are awaiting trivial changes for quite a while now ? I
realise you're busy but I would suggest sparing a few minutes while
they still apply cleanly :-)

Thanks
Emil