guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system hea


From: Andy Wingo
Subject: Re: [PATCH v6 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers.
Date: Thu, 28 Apr 2016 10:18:26 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Hi :)

Mostly formatting nits, only a couple substantial comments.  Looking
great.

On Wed 27 Apr 2016 21:27, Jan Nieuwenhuizen <address@hidden> writes:

> From d81bd3a288f3298d11983da6050958af99780dfe Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Sun, 17 Apr 2016 18:42:43 +0200
> Subject: [PATCH 04/10] gnu: cross-build: i686-w64-mingw32: new cross target.
>
> +                     (if libc
> +                         (setenv "CROSS_LIBRARY_PATH"
> +                                 (string-append
> +                                  libc "/lib"
> +                                  ":" libc "/i686-w64-mingw32/lib")))

Use "when" please

> +(define (native-libc target)
> +  (if (mingw-target? target) mingw-w64
> +      glibc))

The if should be either all on one line, or on three lines

> From aacfb151046372046446e083bb6056e086f3cf68 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Tue, 12 Apr 2016 15:47:54 +0200
> Subject: [PATCH 07/10] gnu: ncurses: support mingw.
>
> +              (cond ((mingw-target? target)
> +                  (with-directory-excursion (string-append out "/bin")
> +                    (for-each
> +                     (lambda (lib)

Because the consequent is so large, please indent the condition on the
line after "cond", i.e.

  (cond
   ((mingw-target? target)
    ...))

> +++ b/gnu/packages/patches/readline-6.3-mingw.patch
> @@ -0,0 +1,128 @@
> +Mingw lacks some SIG*.  Taken from
> +
> +    wget 
> https://raw.githubusercontent.com/Alexpux/MINGW-packages/master/mingw-w64-readline/readline-6.3-mingw.patch
> +
> +some updates to make it apply.
> +
> +Upstream status: Not presented to upstream.

Probably needs to go upstream I think, and we should clarify the
licensing on it or rewrite it.

> +                                        ,(if (mingw-target?) "/bin"
> +                                             "/lib"))

One line or three lines please

> From 5d77ef604ba7c89cfb14275ae9dc0d9b792517ee Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Sun, 24 Apr 2016 14:06:56 +0200
> Subject: [PATCH 10/10] gnu: guile-2.0: support mingw.
>
> * gnu/packages/guile.scm (guile-2.0): Support mingw.
> ---
>  gnu/packages/guile.scm | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
> index fe043cb..82e2cfc 100644
> --- a/gnu/packages/guile.scm
> +++ b/gnu/packages/guile.scm
> @@ -134,11 +134,16 @@ without requiring the source code to be rewritten.")
>                "1qh3j7308qvsjgwf7h94yqgckpbgz2k3yqdkzsyhqcafvfka9l5f"))
>              (patches (list (search-patch "guile-arm-fixes.patch")))))
>     (build-system gnu-build-system)
> -   (native-inputs `(("pkgconfig" ,pkg-config)))
> +   (native-inputs `(("pkgconfig" ,pkg-config)
> +                    ,@(if (mingw-target?)
> +                          `(("bash" ,bash)
> +                            ("guile" ,guile-2.0))
> +                          '())))

Would we not need Guile always when cross-compiling?

>     (inputs `(("libffi" ,libffi)
>               ("readline" ,readline)
> -             ("bash" ,bash)))
> -
> +             ,@(libiconv-if-needed)
> +             ,@(if (mingw-target?) '()
> +                   `(("bash" ,bash)))))

One line please.

> @@ -167,7 +172,11 @@ without requiring the source code to be rewritten.")
>                    (let ((bash (assoc-ref inputs "bash")))
>                      (substitute* "module/ice-9/popen.scm"
>                        (("/bin/sh")
> -                       (string-append bash "/bin/bash")))))
> +                       ,(if (mingw-target?)
> +                            "cmd.exe"
> +                            `(if bash
> +                                 (string-append bash "/bin/bash")
> +                                 "bash"))))))
>                  %standard-phases)))

I guess the thing is, cmd.exe is part of the system, so it should be
linked "dynamically" (i.e. via the PATH); but is cmd.exe really a bash
replacement?  Is this the right way for open-pipe in Guile to work?

Finally at the very end of all of this I think it would be great to have
some kind of document, "how to compile software for windows using
Guix".   WDYT?  That document could include any relevant details about
the internal structure of a cross-compile -- how Guix links things
together.  I know that for me, this information is verrrry easily paged
out to long-term storage; takes a while to pull it back in :)

Andy



reply via email to

[Prev in Thread] Current Thread [Next in Thread]