Ludovic Courtès writes:

Hi!

Find attached v4 of my mingw cross build patches.  I'm also working with
Manolis and using this patch set rebased on wip-hurd have managed to
produce the hurd boostrap binaries.

> 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.

Hmm...I have been using cross-* versions.  I'll probably have to learn
to be more careful when moving changes from say, cross-ncurses to the
real ncurses, and more trusting that my mingw cross changes don't affect
the native builds.  I have been avoiding to create cross-guile to depend
on cross-readline, depending on cross-ncurses; possibly that's the way
to go though.

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

Ah, I think this is what I eventually did with cross-gcc-core and then
forgot that choice.

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

That sounds interesting, esp. if I want to attempt another cross build.
Do you have an example of this?

> 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?

You're right; merged it into cross-gcc[-sans-libc].

> 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)))

Thanks, added and

> and use that in all such cases.

done everywhere...except in the ncurses package.  I don't know how to
access (guix utils) mingw-target? there, for now I'm using

        (post-install-phase
         `(lambda* (#:key outputs target #:allow-other-keys)
            (let ((out (assoc-ref outputs "out"))
                  (mingw-target? (lambda* (#:optional (target target))
                                   (and target
                                        (string-suffix? "-mingw32" target))))

any help much appreciated.

>> +                           `("--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.

I added

     ;; Static and shared cannot be built in one go:
     ;; they produce different headers.  We need shared.
     `("--disable-static"
       "--enable-shared"))

>> * 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.

Thanks, done.

>> +              (with-directory-excursion (string-append out "/bin")
>> +                (for-each (lambda (lib)
>> +                            (define lib.dll
>> +                              (string-append "lib" lib ".dll"))

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

Ah, yes!  Thanks, this removes the need for disabling validate-runpath.

>> -        `("--with-shared" "--without-debug" "--enable-widec"
>> +        (append
>> +         `("--with-shared" "--without-debug" "--enable-widec"
>
> It seems there are no functional changes above.

Right, only to enable the (append of:

>> +         (cond ((equal? ,(%current-target-system) "i686-w64-mingw32")
>> +                '("--enable-term-driver"
>> +                  "--without-cxx"
>> +                  "--without-cxx-binding"))
>
> This would need to be justified.

Removed append, now changed using splicing to

    ;; MinGW: Use terminal driver, created for the MinGW port.
    ,@(if ,(mingw-target?) '("--enable-term-driver") '()))

the --without-cxx, --without-cxx-binding were probably to help me
bootstrapping.

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

I have worked on this all morning.  It is no longer necessary to disable
runpath, it appeared to be collateral damage.

> 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))

Not sure.  There are no elf files produced on MinGW, so validate-runpath
is a no-op anyway.  WDYT?

>> --- /dev/null
>> +++ b/gnu/packages/patches/ncurses-mingw.patch
>
> Please add a note on the origin and upstream status.

Added

    Taken from Eli Zaretskii's gdb bug report

        https://sourceware.org/ml/gdb-patches/2012-04/msg01052.html

    Upstream status: Not presented to upstream.

>> +++ 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?

Added

    Upstream status: Not presented to upstream.

>>                           ;; 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.

This was a bit crude, changed back to

       ,@(if (%current-target-system)
             '("bash_cv_wcwidth_broken=no")
             '())

and added

       ;; MinGW: ncurses provides the termcap api.
       ,@(if (mingw-target?)
             '("bash_cv_termcap_lib=ncurses")
             '()))
    #:make-flags (list ,@(if (mingw-target?)
                             ;; MinGW: termcap in ncurses
                             '("TERMCAP_LIB=-lncurses")
                             '()))

>> +             ,@(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))
>         '()))

Thanks!  Added to base.scm, changed guile's inputs to

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

>> +(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?  :-)

Well, it can come-in handy ;-)  Sorry, removed.

>> +++ b/gnu/packages/patches/guile-remove-utf8.patch
>> +-# 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?

Removed.  This fixed substitute* failing when cross compiling and I figured
that using utf8 quotes here was probabl unintentional; I cannot reproduce the
problem anymore, possibly an artifact of cross-gcc-core.

> 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.

Sure, I can appreciate that.

> 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 agree.  GUB's approach of target-specialisations has the nice feature
of separating out weird ports such as MinGW instead of touching most
every package definition, which I'm a stil bit uncomfortable with in
Guix.  However, that approach is even more susceptible to bitrot.

> 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!

Well, I am aware that adding a new port might take some time to land and
your comments are helpful, thanks a lot for taking this time!  What's
disappointing is having to wait to see result of rebuild world when I
move my changes from cross-ncurses to ncurses [too early], my lack of
iron to make that fast, my lack of knowledge of Guix to possibly use the
repl more :-)

Greetings,
Jan