Jean Pierre De Jesus DIAZ via Guix-patches via schreef op ma 27-06-2022
om 10:52 [+0000]:
> +               (replace 'build
> +                 (lambda* (#:key inputs make-flags parallel-build?
> target
> +                           #:allow-other-keys)
> +                   ;; When cross-compiling, the bash script
> 'libgcrypt-config'
> +                   ;; must be accessible during the configure phase.
> +                   (when target
> +                     (setenv "PATH"
> +                             (string-append
> +                               (dirname
> +                                 (search-input-file inputs
> +                                                    "bin/libgcrypt-
> config"))

It would be simpler to replace 'libgcrypt-config --libs' by 'pkg-config
--libs' (with a substitute*, post-unpack) and likewise for
'libgcrypt-config --libs', to avoid executing a script compiled for
another architecture locally (it works because of a preceding $(shell
..., though doesn't seem great to me).

(Additionally, maybe upstream would be interested in pkg-config which
mostly just works when cross-compiling, in constrast to ...-config
scripts?)

> +           #~(let ((out (assoc-ref %outputs "out")))

If you're using G-exps, you might as well replace (assoc-ref %outputs
"out") by #$output.  Also, there's no configure phase, it has been
removed.


>+                   ;; manpage. This step is not necessary when the target is
>+                   ;; the host.
>+                   (when target
>+                     (apply invoke "make" "vpnc.8" "CC=gcc"
>+                            (if parallel-build?
>+                               (list "-j" (number->string (parallel-job-count)))
>+                               '()))
>+                     (install-file "vpnc.8"
>+                                   (string-append (assoc-ref outputs "out")
>+                                                  "/share/man/man8"))
>+                     (invoke "make" "clean"))))

You can simplify this by adding vpnc itself to the native-inputs (conditional
on %current-target-system to avoid a cycle) and when cross-compiling, copy the
man page from the native input to vpnc.8 (pre-build, instead of replacing build,
then you don't needd to fiddle with parallelism flags or the install phase).

>+                   ;; Remove installation of COPYING as 'install-
license-files
>+                   ;; phase does it with a proper version number.
>+                   (substitute* "Makefile"
>+                     (("install -m644 COPYING.*") "")))))))

Independent change, so for a separate commit.  Also, I don't think it's
worth it to remove the double installation -- we would be deviating
from upstream and at most some KB would be saved but we have automated
depulication.  Or maybe it's nice for consistency, dunno.

>+   (license (list license:gpl2+
>+                  ;; dh.c
>+                  ;; dh.h
>+                  ;; math_group.c
>+                  ;; math_group.h
>+                  license:bsd-2))

Again independent change -- I don't think it was necessary to mention
the file names (but I don't think they have to be removed either). More
precise license information looks nice.

Greetings,
Maxime.