diff mbox

[07/11] gnu: ncurses: support mingw.

Message ID 87vaz2vvir.fsf@gnu.org
State New
Headers show

Commit Message

Jan Nieuwenhuizen Aug. 15, 2016, 4:37 p.m. UTC
Mark H Weaver writes:

Hi Mark!

> As I wrote elsewhere, this patch would force about 10000 rebuilds
> (across all four architectures), which would rule out applying it to
> master.

That could very well be...I've been rebuilding quite a lot.  Is there a
command I can run to get (an estimate of) the number of rebuilds that
a change triggers?

> However, I've attached an alternative patch that I hope will do the same
> job, but it doesn't force any rebuilds at all, so it could be applied to
> master right now.  It carefully arranges to add tests only on the client
> side, and to generate the same build-side code, and the same derivations
> for normal builds.
>
> Can you try it and see if it works?  If so, I will write more about how
> it works.

Thanks for looking at this!  I think that I see what you're
doing/attempting: evaluate all changes early when reading the package
description, so that the builder (?) sees no changes.

Are these kind of temporary workarounds that should somehow get
changed/cleaned-up later?  I had the impression that this

              (patches (map search-patch
                            (if (target-mingw?)
                                '("ncurses-mingw.patch")
                                '())))))

i.e., conditional patches, was a big nono (this patch was already
tested, but a conditional patch might break x86_64 when applied).

Also, we'll get more conditionals just to keep the original description
unchanged.

I have tested your patch and found two small bits missing.  The
additional patch below adds these and then ncurses builds fine....but it
still seems that rebuilds get still triggered.  Could that be because of
you moved the let into arguments, or did we/I miss something else?

> Maybe the other mingw-related changes to core packages could
> follow a similar pattern, and thus be eligible to be applied to master
> directly.

Sure, let's look at that when we this right.

Greetings,
Jan

Comments

Ricardo Wurmus Aug. 16, 2016, 10:26 a.m. UTC | #1
Jan Nieuwenhuizen <janneke@gnu.org> writes:

> Mark H Weaver writes:
>
> Hi Mark!
>
>> As I wrote elsewhere, this patch would force about 10000 rebuilds
>> (across all four architectures), which would rule out applying it to
>> master.
>
> That could very well be...I've been rebuilding quite a lot.  Is there a
> command I can run to get (an estimate of) the number of rebuilds that
> a change triggers?

The command “guix refresh -l package-name” gives you a very rough list
of packages (on the same architecture) that would be affected by an
update to “package-name”.  This isn’t always correct, but in the case of
ncurses it shows at least that a lot of packages are affected:

    guix refresh -l ncurses
    …
    Building the following 1018 packages would ensure 2616 dependent
    packages are rebuilt: …

~~ Ricardo
Jan Nieuwenhuizen Aug. 16, 2016, 11:53 a.m. UTC | #2
Ricardo Wurmus writes:

> The command “guix refresh -l package-name” gives you a very rough list
> of packages (on the same architecture) that would be affected by an
> update to “package-name”.  This isn’t always correct, but in the case of
> ncurses it shows at least that a lot of packages are affected:
>
>     guix refresh -l ncurses
>     …
>     Building the following 1018 packages would ensure 2616 dependent
>     packages are rebuilt: …

Thanks!

I found why my ncurses still triggers rebuilds: that was in my fixes
that I added on top of Mark's patch, adding necessary MinGW configure
flags

    `(#:configure-flags
      `(
       ...
       ;; MinGW: Provide termcap api, created for the MinGW port.
       ,,@(if (target-mingw?) '("--enable-term-driver") '())
       )

While this `works' build-wise, it still modifies configure-flags when
not target-mingw?.

That is solved technically by doing it like so

    `(#:configure-flags
      ,(cons*
        'quasiquote
        `("--with-shared" "--without-debug" "--enable-widec"

          ;; By default headers land in an `ncursesw' subdir, which is not
          ;; what users expect.
          ,(list 'unquote '(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.
          ,(list 'unquote '(string-append "LDFLAGS=-Wl,-rpath=" (assoc-ref %outputs "out")
                                          "/lib")))
        ;; MinGW: Provide termcap api, created for the MinGW port.
        (if (target-mingw?) '("--enable-term-driver") '()))

...is there an easier/cleaner way, this looks overly complicated.

Greetings,
Jan
Jan Nieuwenhuizen Aug. 16, 2016, 6:24 p.m. UTC | #3
Jan Nieuwenhuizen writes:

> Ricardo Wurmus writes:
>
>> The command “guix refresh -l package-name” gives you a very rough list
>> of packages (on the same architecture) that would be affected by an
>> update to “package-name”.  This isn’t always correct, but in the case of
>> ncurses it shows at least that a lot of packages are affected:
>>
>>     guix refresh -l ncurses
>>     …
>>     Building the following 1018 packages would ensure 2616 dependent
>>     packages are rebuilt: …
>
> Thanks!
>
> I found why my ncurses still triggers rebuilds: that was in my fixes
> that I added on top of Mark's patch, adding necessary MinGW configure
> flags
>
>     `(#:configure-flags
>       `(
>        ...
>        ;; MinGW: Provide termcap api, created for the MinGW port.
>        ,,@(if (target-mingw?) '("--enable-term-driver") '())
>        )
>
> While this `works' build-wise, it still modifies configure-flags when
> not target-mingw?.
>
> That is solved technically by doing it like so

Oops, make that:

    `(#:configure-flags
      ,(cons*
        'quasiquote
        `(("--with-shared" "--without-debug" "--enable-widec"

           ;; By default headers land in an `ncursesw' subdir, which is not
           ;; what users expect.
           ,(list 'unquote '(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.
           ,(list 'unquote '(string-append "LDFLAGS=-Wl,-rpath=" (assoc-ref %outputs "out")
                                           "/lib"))
           ;; MinGW: Use term-driver created for the MinGW port, libtool.
           ,@(if (target-mingw?) '("--enable-term-driver" "--with-libtool") '()))))


Greetings,
Jan
diff mbox

Patch

From fad3d7d3929d0397e193c5993d1966e9b6f171fc Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen <janneke@gnu.org>
Date: Mon, 15 Aug 2016 15:39:04 +0200
Subject: [PATCH] gnu: ncurses: support mingw: fixlets: libraries, term driver.

---
 gnu/packages/ncurses.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/ncurses.scm b/gnu/packages/ncurses.scm
index 1366dc0..8002cbe 100644
--- a/gnu/packages/ncurses.scm
+++ b/gnu/packages/ncurses.scm
@@ -102,7 +102,7 @@ 
                               (when (file-exists? libw6.dll)
                                 (format #t "creating symlinks for `lib~a'~%" lib)
                                 (symlink libw6.dll lib.dll)))
-                            libraries)))
+                            '("curses" "ncurses" "form" "panel" "menu"))))
                        '())
                  (with-directory-excursion (string-append out "/lib")
                    (for-each (lambda (lib)
@@ -143,7 +143,9 @@ 
            ;; Make sure programs like 'tic', 'reset', and 'clear' have a
            ;; correct RUNPATH.
            ,(string-append "LDFLAGS=-Wl,-rpath=" (assoc-ref %outputs "out")
-                           "/lib"))
+                           "/lib")
+           ;; MinGW: Provide termcap api, created for the MinGW port.
+           ,,@(if (target-mingw?) '("--enable-term-driver") '()))
          #:tests? #f                  ; no "check" target
          #:phases (modify-phases %standard-phases
                     (replace 'configure ,configure-phase)
-- 
2.9.2