Re: [PATCH v5 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers.

[Jan Nieuwenhuizen]

Eric Bavier writes:

>> * gnu/packages/patches/gcc-cross-environment-variables.patch: Also use CROSS_
>>   variants: CROSS_C_INCLUDE_PATH, CROSS_CPLUS_INCLUDE_PATH,
>>   CROSS_OBJC_INCLUDE_PATH, CROSS_OBJCPLUS_INCLUDE_PATH to be used for system
>>   libraries, see

> We've started following the Changelog conventions more closely lately,
> notably the lack of indentation spacing on newlines.

Ah that's good; I've been hand-editing this

>> -                    (setenv "CROSS_CPATH"
>> -                            (string-append libc "/include:"
>> -                                           linux "/include"))
>> +                    (let ((cpath (string-append
>> +                                  libc "/include"
>> +                                  ":" linux "/include")))
>
> I prefer the old alignment of the string-append (nitpick).

You cannot mean like this

                         (let ((cpath (string-append libc "/include"
                                                     ":" libc 
"/i686-w64-mingw32/include")))

...that's 92 characters, how would you do it?

>> +                                (and-let* ((value (getenv var))
>> +                                           (path 
>> (search-path-as-string->list
>> +                                                  value))
>> +                                           (native-path
>> +                                            (list->search-path-as-string
>> +                                             (remove cross? path) ":")))
>> +                                  (setenv var native-path)))
>> +                              '("C_INCLUDE_PATH"
>> +                                "CPLUS_INCLUDE_PATH"
>> +                                "OBJC_INCLUDE_PATH"
>> +                                "OBJCPLUS_INCLUDE_PATH"
>> +                                "LIBRARY_PATH"))))
>> +               ,phases))
>> +            (else phases))))))))
>
> The phase overall should result in a boolean, but the for-each here
> returns undefined.  There has been sentiment around here to avoid
> and-let* where possibly; 'and=>' should work nicely in this case, since
> there's only a single application that could return #f.

Ok; changed to

(for-each (lambda (var)
            (and=> (getenv var)
                   (lambda (value)
                     (let* ((path (search-path-as-string->list value))
                            (native-path (list->search-path-as-string
                                          (remove cross? path) ":")))
                       (setenv var native-path)))))
          '("C_INCLUDE_PATH"
            "CPLUS_INCLUDE_PATH"
            "OBJC_INCLUDE_PATH"
            "OBJCPLUS_INCLUDE_PATH"
            "LIBRARY_PATH"))

and added a #t

>>  (define (cross-gcc-patches target)
>>    "Return GCC patches needed for TARGET."
>> @@ -228,6 +234,7 @@ GCC that does not target a libc; otherwise, target that 
>> libc."
>>       `(#:implicit-inputs? #f
>>         #:modules ((guix build gnu-build-system)
>>                    (guix build utils)
>> +                  (ice-9 and-let-star)
>
> Based on the module added above, did you mean to use and-let* here?

Ah, yes I'm using that in
0004-gnu-cross-build-i686-w64-mingw32-new-cross-target.patch;
removed.  I'll have a look at removing and-let* there too.

Thanks!
Greetings,
Jan

-- 
Jan Nieuwenhuizen <address@hidden> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar®  http://AvatarAcademy.nl