Re: [PATCH 10/10] gnu: base: Added glibc-for-target macro.

[ludo@gnu.org (Ludovic Courtès)]

Manolis Ragkousis <manolis837@gmail.com> skribis:

> The macro that doesn't work as expected.

How doesn’t it work exactly?  :-)

Note that because this patch changes ‘glibc’ from a “normal” variable to
a syntax object, you need to ‘make clean-go && make’.

> From 8534372800703610b1436870da2caf1fa3c9796c Mon Sep 17 00:00:00 2001
> From: Manolis Ragkousis <manolis837@gmail.com>
> Date: Tue, 19 May 2015 01:42:14 +0300
> Subject: [PATCH 10/10] gnu: base: Added glibc-for-target macro.
>
> gnu/packages/base.scm (glibc): Add macro.

Missing star.

> +(define-public glibc/hurd
> +  (package (inherit glibc/linux)
> +    (name "glibc-hurd")

I see you moved this definition and ‘glibc/hurd-headers’ upward, but
this is not necessary AFAICS.  Could you avoid this change in the next
iteration of the patch?

> +(define (glibc-for-target target)

Please make it:

  (define* (glibc-for-target #:optional
                             (target (or (%current-target-system)
                                         (%current-system))))
    ;; ...
    )

> +  "Return the glibc for TARGET, glibc/linux for a linux host or
> +glibc/hurd for a hurd host"

Use capital letters when referring to variables, so GLIBC/LINUX and
GLIBC/Hurd.  Spelling: Linux, Hurd.

> +(define-syntax glibc
> +  (identifier-syntax (glibc-for-target (or (%current-target-system) (%current-system)))))

Now this can become:

  (define-syntax glibc
    (identifier-syntax (glibc-for-target)))

which reduces code duplication.

That looks good to me, but now I want to know what doesn’t work.  :-)

Ludo’.