Re: rfc/rfh: i686-w64-mingw32 cross target

[Ludovic Courtès]

Jan Nieuwenhuizen <address@hidden> skribis:

> The fact that almost every little change triggers a full world rebuild
> makes creating a cross build on Guix quite annoying, though.  I wonder
> if we would like to make this friendlier, even if only to bootstrap
> cross builds.

I think it’s always possible to create variants of the core packages
while testing a new feature, such that you don’t end up rebuilding the
world.

> Our old GUB cross build system has two features that vastly reduce the
> number of toolchain/world rebuilds
>
>    * every package (class Name) can possbly have a architecture-specific
>      specialisation: class Name__<arch>, that the build system looks
>      for and uses if present
>
>    * there is a `-x,--no-dependencies' option that says: just rebuild
>      this package, even if some of its dependencies are out of date
>
> I can speficy e.g. a modified Gcc for mingw only (GUB uses Python, my
> favourite before I changed to Guile) like so
>
>     class Gcc (cross.AutoBuild):
>         ...
>
>     class Gcc_core (Gcc):
>         ...build minimal c-only compiler to build a libc...
>
>     class Gcc__mingw (Gcc):
>          ...add a patch for the c++ compiler to build...
>
> Then the native toolchain won't be rebuilt when experimenting with
> mingw.  To avoid rebuilding the core gcc and the c library when
> working to get the c++ compiler built, I can use --no-dependencies
> until it works, then suffer a full rebuild only once at the end.

Here what I would recommend is to temporary switch to your own GCC
variant in cross-base.scm, for the purposes for accelerating testing.

In some cases, one could use ‘replacement’ fields, which in effect is
equivalent, AIUI, to --no-dependencies.

>> My main concern is the complexity of the cross-base stuff.  Why is
>> ‘cross-gcc-core’ needed, for instance?
>
> It is needed to break a circular bootstrap dependency.  To build a new c
> library, you need a cross compiler.  A full flegded gcc can only be
> built when you have a c library: the circular dependency.  This simple,
> minimal `core-gcc' can be built with only the headers of the new c
> library; breaking the circle.

I understand, but ‘cross-gcc’ already handles that: it can be called
with or without a libc.  In the latter case, it produces a “sans-libc”
GCC that is then used to build the C library.

So I’m under the impression that ‘core-gcc’ duplicates this logic.  Or
am I missing something?

Some comments on the “easy” parts:

> From aaff9752fd10b7860bdba62fba3107fe8133367f Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Tue, 12 Apr 2016 15:22:51 +0200
> Subject: [PATCH 5/9] gnu: gmp: build shared library for mingw.
>
> * gnu/packages/multiprecision.scm (gmp)[MINGW]: Use --enable-shared.

[...]

> +                  ,@(cond ((equal? (%current-target-system) 
> "i686-w64-mingw32")

I think we should have a procedure like this in (guix utils), below
‘%current-target-system’:

  (define* (mingw-target? #:optional (target (%current-target-system)))
    (and target
         (string-suffix? "-mingw32" target)))

and use that in all such cases.

> +                           `("--enable-shared"
> +                             "--disable-static"))

In general, I think it’s important to add a comment to justify such
changes, because ideally MinGW wouldn’t diverge from other systems, and
because we want to be able to maintain such workarounds in the future.

> From 6e1d4f58d4a7dca54dbbfeacc45f187bf31451ba Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Tue, 12 Apr 2016 15:15:04 +0200
> Subject: [PATCH 6/9] gnu: Add libiconv.
>
> * gnu/packages/libiconv.scm: New file.
>   gnu-system.am: Add it.

Could you please add it to base.scm?  Make sure to run ‘guix lint’,
which will suggest a synopsis and description, among other things.

> From 2e676615104f1605357eabe80fa7d716d80bc486 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Tue, 12 Apr 2016 15:24:53 +0200
> Subject: [PATCH 7/9] gnu: ncurses: support mingw.
>
> * gnu/packages/patches/ncurses-mingw.patch: New file.
> * gnu/packages/ncurses.scm (ncurses)[MINGW]: Support mingw.


[...]

>                                    (format p "INPUT (-l~aw)~%" lib)))))
> +                          '("curses" "ncurses" "form" "panel" "menu")))
> +              (with-directory-excursion (string-append out "/bin")
> +                (for-each (lambda (lib)
> +                            (define lib.dll
> +                              (string-append "lib" lib ".dll"))
> +                            (define libw6.dll
> +                              (string-append "lib" lib "w6.dll"))
> +                            (when (file-exists? libw6.dll)
> +                              (format #t "creating symlinks for `lib~a'~%" 
> lib)
> +                              (symlink libw6.dll lib.dll)))
>                            '("curses" "ncurses" "form" "panel" "menu")))))))

Obviously this would need to be made conditional.  :-)

[...]

> -        `("--with-shared" "--without-debug" "--enable-widec"
> +        (append
> +         `("--with-shared" "--without-debug" "--enable-widec"
>  
> -          ;; By default headers land in an `ncursesw' subdir, which is not
> -          ;; what users expect.
> -          ,(string-append "--includedir=" (assoc-ref %outputs "out")
> -                          "/include")
> -          "--enable-overwrite"                    ;really honor --includedir
> +           ;; By default headers land in an `ncursesw' subdir, which is not
> +           ;; what users expect.
> +           ,(string-append "--includedir=" (assoc-ref %outputs "out")
> +                           "/include")
> +           "--enable-overwrite"         ;really honor --includedir
>  
> -          ;; Make sure programs like 'tic', 'reset', and 'clear' have a
> -          ;; correct RUNPATH.
> -          ,(string-append "LDFLAGS=-Wl,-rpath=" (assoc-ref %outputs "out")
> -                          "/lib"))
> +           ;; Make sure programs like 'tic', 'reset', and 'clear' have a
> +           ;; correct RUNPATH.
> +           ,(string-append "LDFLAGS=-Wl,-rpath=" (assoc-ref %outputs "out")
> +                           "/lib"))

It seems there are no functional changes above.

> +         (cond ((equal? ,(%current-target-system) "i686-w64-mingw32")
> +                '("--enable-term-driver"
> +                  "--without-cxx"
> +                  "--without-cxx-binding"))

This would need to be justified.

> +                   ,@(cond ((equal? (%current-target-system) 
> "i686-w64-mingw32")
> +                            `((delete 'validate-runpath)))
> +                           (else '()))

Use #:validate-runpath? #f instead.

If the reason is that RUNPATH does not exist on Windows, then we should
instead do something like:

diff --git a/guix/build-system/gnu.scm b/guix/build-system/gnu.scm
index a7d1952..8a817a5 100644
--- a/guix/build-system/gnu.scm
+++ b/guix/build-system/gnu.scm
@@ -430,7 +430,7 @@ is one of `host' or `target'."
                                           "--enable-deterministic-archives"))
                           (strip-directories ''("lib" "lib64" "libexec"
                                                 "bin" "sbin"))
-                          (validate-runpath? #t)
+                          (validate-runpath? (not (mingw-target? target)))
                           (phases '%standard-phases)
                           (locale "en_US.utf8")
                           (system (%current-system))

> --- /dev/null
> +++ b/gnu/packages/patches/ncurses-mingw.patch

Please add a note on the origin and upstream status.

> From f112c5d09b77b2d89cb5c002516c3e78a715d53f Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Tue, 12 Apr 2016 15:26:10 +0200
> Subject: [PATCH 8/9] gnu: readline: support mingw.
>
> * gnu/packages/patches/readline-6.3-mingw.patch: New file.
> * gnu-system.am: Add it.
> * gnu/packages/readline.scm (readline): Support mingw.

[...]

> +++ b/gnu/packages/patches/readline-6.3-mingw.patch
> @@ -0,0 +1,126 @@
> +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?

[...]

>                                          (assoc-ref %build-inputs "ncurses")
> -                                        "/lib")
> +                                        ,(if (equal? 
> (%current-target-system) "i686-w64-mingw32") "/bin"
> +                                             "/lib"))
>  
>                           ;; This test does an 'AC_TRY_RUN', which aborts when
>                           ;; cross-compiling, so provide the correct answer.
>                           ,@(if (%current-target-system)
> -                               '("bash_cv_wcwidth_broken=no")
> +                               '("bash_cv_wcwidth_broken=no"
> +                                 "bash_cv_termcap_lib=ncurses")
>                                 '()))
> -
> +                   #:make-flags (list ,@(if (%current-target-system)
> +                                            '("TERMCAP_LIB=-lncurses")
> +                                            '()))

These would need to be justified.

> From 1f513d155890453948c6599e57cc2bb384effbb8 Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <address@hidden>
> Date: Tue, 12 Apr 2016 15:27:33 +0200
> Subject: [PATCH 9/9] gnu: guile-2.0: support mingw.  WIP, builds, links,
>  segfaults.
>
> * gnu/packages/patches/guile-remove-utf8.patch: New file.
> * gnu-system.am: Add it.
> * gnu/packages/guile.scm (guile-2.0): Support mingw.

[...]

> +             ,@(if (equal? (%current-target-system) "i686-w64-mingw32")
> +                    `(("libiconv" ,libiconv))
> +                    `(("bash" ,bash)))))

I think we’d need something like:

  (define* (libiconv-if-needed #:optional (target (%current-target-system)))
    (if (mingw-target? target)
        `(("libiconv" ,libiconv))
        '()))

> +(define-public cross-guile
> +  (package
> +    (inherit guile-2.0)
> +    (name "cross-guile")
> +    (version "2.0.11")

I don’t think it’s needed, is it?  :-)

> +++ b/gnu/packages/patches/guile-remove-utf8.patch
> @@ -0,0 +1,16 @@
> +--- guile-2.0.11/libguile/Makefile.in.orig   2016-04-11 07:46:38.792593661 
> +0200
> ++++ guile-2.0.11/libguile/Makefile.in        2016-04-11 07:55:44.410618808 
> +0200
> +@@ -3735,11 +3735,11 @@
> +     flex -t $(srcdir)/c-tokenize.lex > $@ || { rm $@; false; }
> + 
> + # This page is for maintenance of the lists of CPP symbols that are 
> eventually
> +-# included in error.c (‘errno’ values: E*) and posix.c (signal names: SIG*),
> ++# included in error.c (`errno' values: E*) and posix.c (signal names: SIG*),

Why is this needed?

In summary, as a maintainer, I want to minimize maintenance work.  :-)
So I think we must pay a lot of attention to how we integrate support
for alternate platforms.

Most likely, cross-compilation support for MinGW will remain seldom
used, and thus subject to bitrot.  Thus, we must make sure that
adjustments made to packages for MinGW support are as little intrusive
as possible, and well documented so people know why they are there and
what to do about them.

WDYT?

I guess it’s disappointing because you bring a whole lot of patches and
I respond on a lot of minor points to address, but I just want to make
sure we don’t overcommit!

Thank you,
Ludo’.