guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] gnu: Add Mlucas.


From: Ludovic Courtès
Subject: Re: [PATCH 2/2] gnu: Add Mlucas.
Date: Thu, 05 Nov 2015 23:08:16 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Alex Vong <address@hidden> skribis:

> From 876bbbeafaa2dae8e584820645ca21efecf6ae7c Mon Sep 17 00:00:00 2001
> From: Alex Vong <address@hidden>
> Date: Sun, 25 Oct 2015 00:18:29 +0800
> Subject: [PATCH 2/2] gnu: Add Mlucas.
>
> * gnu/packages/mlucas.scm: New file.
> * gnu-system.am (GNU_SYSTEM_MODULES): Register it.

I agree with Paul that maths.scm would be more appropriate.

> +;;; the Free Software Foundation; either version 3 of the License, or
> (at +;;; your option) any later version.

The MUA is breaking lines.  Could you try to disable it?

> +         (flag-list-
> +          (flag-list+ all-flag-list
> +                      (flag-list
> +                       #:CFLAGS '("-Ofast"
> +                                  "-pipe"
> +                                  "-flto"
> +                                  "-fno-aggressive-loop-optimizations")
> +                       #:LDFLAGS '("-Wl,--as-needed")))

For now, could you just pass these as #:configure-flags?

I think ‘-pipe’ can be omitted nowdays (and cpp is no longer a separate
process.)

I’m not sure if ‘-Ofast’ is OK: it might entail something equivalent to
‘-mtune=native’, which is inappropriate if the software is to be
distributed by Hydra.

> +          default-flag-list)))

[...]

> +        (modify-phases %standard-phases
> +                       (add-before 'configure
> +                                   'bootstrap
> +                                   (lambda _
> +                                     (zero? (system "./bootstrap")))))

Please use ‘system*’.  Also indent as in the other files.

> +        #:configure-flags
> +        '("--disable-NORMAL-CFLAGS"
> +          "--disable-TRICKY-CFLAGS"

Really?  :-)

> +     ;; run-time dependencies
> +     (propagated-inputs `(("coreutils" ,coreutils)
> +                          ("sed" ,sed)))

Why is this needed?  Could you put the justification in a comment?

> +     ;; build-time dependencies
> +     (native-inputs `(("autogen" ,autogen)
> +                      ("autoconf" ,autoconf)
> +                      ("automake" ,automake)
> +                      ("perl" ,perl)))

Does upstream provide a bootstrapped tarball, resulting from ‘make
dist’?  That would be best (no need to depend on Autoconf and Automake.)

> +     (description "mlucas performs Lucas-Lehmer test

“Mlucas” maybe?

> +on prime-exponent Mersenne numbers,
> +that is, integers of the form 2 ^ p - 1, with prime exponent p.

@math{2^p - 1}

> +For more information on GIMPS,
> +see <http://www.mersenne.org/prime.html> for details.

@uref{http://www.mersenne.org/prime.html}

Could you send an updated patch?

Thanks in advance, and sorry for the delay!

Ludo’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]