Re: 01/06: gnu: llvm, clang: Update to 6.0.0.

[Mark H Weaver]

Hi Marius,

address@hidden (Marius Bakke) writes:

> mbakke pushed a commit to branch master
> in repository guix.
>
> commit 9bdbabe963e48bcac11c053a38d990873ca48dca
> Author: Marius Bakke <address@hidden>
> Date:   Sat Mar 31 22:04:44 2018 +0200
>
>     gnu: llvm, clang: Update to 6.0.0.

Thanks for this, but ...

> +                          ;; Link to libclang_rt files from clang-runtime.
> +                          ;; This substitution needed slight adjustment in 
> 3.8.
> +                          (if (< 3.8 (string->number ,(version-major+minor
> +                                                       (package-version
> +                                                        clang-runtime))))
> +                              (substitute* "lib/Driver/Tools.cpp"
> +                                (("TC\\.getDriver\\(\\)\\.ResourceDir")
> +                                 (string-append "\"" compiler-rt "\"")))
> +                              (substitute* "lib/Driver/ToolChain.cpp"
> +                                (("getDriver\\(\\)\\.ResourceDir")
> +                                 (string-append "\"" compiler-rt "\""))))

The version comparison code above is incorrect.  This code will judge
version 3.10 to be older than 3.8.

'string->number' is never a sensible procedure to apply to version
numbers with more than one component, because it is not injective, let
alone order-preserving.  For example, "3.1" and "3.10" map to the same
number 3.1, although they are different versions.

I suggest that you use 'version-compare', 'version>?', or 'version>=?'
from (guix utils).  Note that these are not available to build-side
code, so the entire 'if' should be lifted to client-side code, which is
better anyway.  Maybe something like this (untested):

    ;; Link to libclang_rt files from clang-runtime.
    ;; This substitution needed slight adjustment in 3.8.
    ,(let ((runtime-version (version-major+minor
                             (package-version
                              clang-runtime))))
       (if (version>? runtime-version "3.8")
           '(substitute* "lib/Driver/Tools.cpp"
              (("TC\\.getDriver\\(\\)\\.ResourceDir")
               (string-append "\"" compiler-rt "\"")))
           '(substitute* "lib/Driver/ToolChain.cpp"
              (("getDriver\\(\\)\\.ResourceDir")
               (string-append "\"" compiler-rt "\"")))))

What do you think?

      Mark