[bug#37027] [PATCH] gnu: zlib: Add support for MinGW targets.

["Ludovic Courtès" <ludo@gnu.org>]

Hi David,

David Thompson <dthompson2@worcester.edu> skribis:

> ---
>  gnu/packages/compression.scm | 55 +++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 19 deletions(-)

Please add a commit log.  :-)

> +               `((delete 'configure)
> +                 (add-before 'install 'set-install-paths
> +                             (lambda* (#:key outputs #:allow-other-keys)
> +                               (let ((out (assoc-ref outputs "out")))
> +                                 (setenv "INCLUDE_PATH" (string-append out "/include"))
> +                                 (setenv "LIBRARY_PATH" (string-append out "/lib"))
> +                                 (setenv "BINARY_PATH" (string-append out "/bin"))
> +                                 #t))))
> +               `((replace 'configure
> +                          (lambda* (#:key outputs #:allow-other-keys)

Minor issue: could you adjust indentation to look like:

  (add-before 'x 'y
    (lambda* …

?  guix.el should take care of that.

> -               (with-directory-excursion (string-append out "/lib")
> -                 (install-file "libz.a" (string-append static "/lib"))
> -                 (delete-file "libz.a")
> -                 #t)))))))
> +                    (lambda* (#:key outputs #:allow-other-keys)
> +                      (let ((out (assoc-ref outputs "out"))
> +                            (static (assoc-ref outputs "static")))
> +                        (with-directory-excursion (string-append out "/lib")
> +                                                  (install-file "libz.a" (string-append static "/lib"))

The ‘with-directory-excursion’ indentation went off here.

> +       ,@(if (target-mingw?)
> +             `(#:make-flags
> +               '("-fwin32/Makefile.gcc"
> +                 "SHARED_MODE=1"
> +                 ,(string-append "CC=" (%current-target-system) "-gcc")
> +                 ,(string-append "RC=" (%current-target-system) "-windres")
> +                 ,(string-append "AR=" (%current-target-system) "-ar")))
> +             '())))

I wonder if we could have a single ‘if’:

  (arguments
    (if (target-mingw?)
        …
        ))

?  Thoughts?

Otherwise LGTM, thanks!

Ludo’.