[Mesa-dev] [PATCH v3 08/25] configure.ac: Move LLVM version check to the top

[Emil Velikov]

On 12 October 2016 at 19:11, Tobias Droste <tdroste at gmx.de> wrote:
> Am Mittwoch, 12. Oktober 2016, 10:11:50 CEST schrieb Emil Velikov:
>> On 12 October 2016 at 00:02, Tobias Droste <tdroste at gmx.de> wrote:
>> > A function with the LLVM version checked is moved to the top.
>> > The function is called where the old code was.
>>
>> Replace the second line with "... in order to reuse/rework X"
>>
>> > No functional change.
>> >
>> > Signed-off-by: Tobias Droste <tdroste at gmx.de>
>> > ---
>> >
>> >  configure.ac | 91
>> >  ++++++++++++++++++++++++++++++++---------------------------- 1 file
>> >  changed, 49 insertions(+), 42 deletions(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 933e7b5..0f19a4f 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -861,6 +861,54 @@ fi
>> >
>> >  AC_SUBST([SELINUX_CFLAGS])
>> >  AC_SUBST([SELINUX_LIBS])
>> >
>> > +dnl
>> > +dnl LLVM
>> > +dnl
>> > +llvm_get_version() {
>>
>> Nit: The code below does a lot more than "get_version" -
>> get_environment/set_variables/other
>
> Any ideas? :-D
>
There's a couple above - I'm leaning towards [llvm_]get_environment
but feel free to use something else.

>>
>> With the above
>> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>>
>> > +    if test -z "${LLVM_CONFIG}"; then
>> > +        if test -n "$llvm_prefix"; then
>> > +            AC_PATH_TOOL([LLVM_CONFIG], [llvm-config], [no],
>> > ["$llvm_prefix/bin"]) +        else
>> > +            AC_PATH_TOOL([LLVM_CONFIG], [llvm-config], [no])
>> > +        fi
>> > +    fi
>> > +
>> > +    if test "x$LLVM_CONFIG" != xno; then
>> > +        LLVM_VERSION=`$LLVM_CONFIG --version | egrep -o '^[[0-9.]]+'`
>>
>> ...
>>
>> > +    else
>> > +        MESA_LLVM=0
>> > +        LLVM_VERSION_INT=0
>>
>> Just realised that we should error out in this case. After all one
>> requests llvm, so silently ignoring that they're missing llvm-config
>> isn't a smart idea. Something like below (be that as a preparatory,
>> in-between or at the end of the series) would be great.
>
> At this point in time we don't know if we actually need LLVM.
Looking at the code in master (and at this point in your series) I see
no way how this can happen.
Can you point out where/how we can get that ?

Either way... this is in the "follow-up" ideas category.

-Emil