Re: [PATCH] gnu: Add Mlucas.

[Alex Vong]

Hi Mathieu,

On 05/10/2015, Mathieu Lirzin <address@hidden> wrote:
> Alex Vong <address@hidden> writes:
>
>> From e5155b52f636bfee849268b19b81f5b6608540fd Mon Sep 17 00:00:00 2001
>> From: Alex Vong <address@hidden>
>> Date: Mon, 5 Oct 2015 12:49:49 +0800
>> Subject: [PATCH] gnu: Add Mlucas.
>>
>> * gnu/packages/mlucas.scm: New file.
>> * gnu-system.am (GNU_SYSTEM_MODULES): Register it.
>> ---
>
> This is quite an unusual patch.  :)
>
>>  gnu-system.am           |   1 +
>>  gnu/packages/mlucas.scm | 283
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 284 insertions(+)
>>  create mode 100644 gnu/packages/mlucas.scm
>>
> [...]
>> +(define-module (gnu packages mlucas)
>> +  #:use-module (srfi srfi-1)
>> +  #:use-module (guix packages)
>> +  #:use-module (guix download)
>> +  #:use-module (guix build-system gnu)
>> +  #:use-module (guix licenses)
>> +  #:use-module (gnu packages autogen)
>> +  #:use-module (gnu packages autotools)
>> +  #:use-module (gnu packages perl))
>> +
>> +
>> +;;; Procedures to manupulate build flags, similar to dpkg-buildflags.
>> +;;;
>> +;;; The data strcture flag-list is constrcuted by (flag-list
>> <flag-sublist>...)
>                                             ^^
> “constructed”
How do you manage to detect this? I have run codespell on the file but
it doesn't report anything!

>
>> +;;; The constructor flag-list does something to the argument,
>> +;;; such as trimming whitespaces, to ensure no two arguments mean the
>> same.
>> +;;;
>> +;;; The data structure flag-sublist is in fact an ordinary list
>> +;;; with the following structure (<flag-type-symbol> <flag-string>...)
>> +;;;
>> +;;; Here is an example:
>> +;;; (flag-list
>> +;;;  '(CFLAGS "-O2" "-g")
>> +;;;  '(LDFLAGS "-lm" "-lpthread"))
>> +;;;
>> +;;; flag-list+ and flag-list- are analogous to
>> +;;; numberic + and - but operate on flag-list.
>> +;;;
>> +;;; flag-list->string-list converts flag-list into
>> +;;; configure-flags-compatible string-list.
>> +;;;
>
> IIUC these procedures are not meant to be specific the definition of
> package
> mlucas.  So this should be in some other commit and maybe located in other
> file in “guix/guix/build/...” directory.
>
> Can you explain the problem you faced with the current Guix methods to
> achieve
> that, and what are the benefits of ‘flag-list’ comparing to them?
>
Flag-list is something similar to dpkg-buildflags(1), it has several
sets of flags which are useful to almost all programs. For example,
fortify flags will attempt to replace insecure unlimited length buffer
function calls with length-limited ones. stackprotectorstrong will
protect your program from stack smashing. In Debian, there is also a
set of default build flags for packaging. I think it is a good idea,
since not all upstream know these source hardening related flags, we
should be using those by default but with an option to turn off
individual sets of flags. This is like a layer above configure flags
and make flags.

> Example:
> --8<---------------cut here---------------start------------->8---
>     (arguments
>      '(#:configure-flags ...
>        #:make-flags ...
> --8<---------------cut here---------------end--------------->8---
>
> [...]
>> +;;; implement the bootstrap-build-system using syntax-case macro
>> +;;; bootstrap-build-system use a bootstrap script
>> +;;; to run autoreconf and generate documentation.
>> +(define-syntax package*
>> +  (lambda(x)
>> +    ;; add autoconf, automake and perl as build dependencies
>> +    ;; Modify the gnu-build-system
>> +    ;; by adding bootstrap phase before configure phase.
>> +    (define (extend-fields s-exp)
>
> I'm not competent enough to judge if it's a useful build-system to add but
> this should be done in another commit and in
> “guix/guix/build/bootstap-build-system.scm” if yes.
>
I should be more a newbie than you do. I just wonder if it is possible
to modify the build system by a little, such as adding a bootstrap
phase and some new inputs, and then give a name to the new build
system. Since it seems many packages are repeating this step, I think
it will be great if we have this mean of abstraction.

By the way, I know the macro looks ugly since this is my first time
writing a macro.

>> +
>> +(define-public mlucas
>> +  ;; descriptions of the package
>> +  (let ((short-description
>> +         "Program to perform Lucas-Lehmer test on a Mersenne number")
>> +        (long-description
>> +         "mlucas is an open-source (and free/libre) program
>                           ^^^
>
> Being a GNU project, we use the term “free software”, but in the context of
> a
> description it is not relevant to describe freedom of a package since every
> package in Guix is free software.
>
"open-source" is actually mentioned in the upstream description, so I
add the description inside the parenthesis. But I can remove both if
it is desired.

> I think there will be more details to cover for an extensive review, but
> let's
> discuss the big lines first.
>
> Thanks for your contribution!
>
Thanks!

> --
> Mathieu Lirzin
>
>

Cheers,
Alex