bug-guix
[Top][All Lists]
Advanced

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

Re: [PATCH] gnu-maintenance: Improve 'official-gnu-packages'; add the re


From: Ludovic Courtès
Subject: Re: [PATCH] gnu-maintenance: Improve 'official-gnu-packages'; add the related procedures.
Date: Sun, 17 Mar 2013 00:13:46 +0100
User-agent: Gnus/5.130005 (Ma Gnus v0.5) Emacs/24.2 (gnu/linux)

Nikita Karetnikov <address@hidden> skribis:

> I apologize for the delay.  What do you think about the attached patch?

No problem, thanks for working on this!

> I noticed that 'guile-gnome' has two 'doc-url' fields.  How can I handle
> this?  (I ignored it for now.)

I think it’s a mistake in this case (could you report it to Karl?).  It
seems safe to ignore.

> Also, is there a better way to create 'gnu-package-descriptor'?  Note
> that I don't want to use setters.

Yes, use the wonderful ‘define-record-type*’ from (guix utils).  This is
what we use for ‘package’, etc.  See
<http://lists.gnu.org/archive/html/guile-user/2012-11/msg00016.html>,
for details.

>> This code is run by the user’s Guile, which may be older than 2.0.7
>> (‘http-get*’ was introduced in 2.0.7), so you can’t rely on it.
>
>> What you can do is something along the lines of what (guix build
>> download) does, but always return a port.  Maybe there’s a way to share
>> code.
>
> But how can I return a port with 'http-get'?

On Guile < 2.0.7, you’ll get a string, so you can just call
‘open-input-string’ to wrap it in a port.

On later versions, you’ll get a port.

>> I’d rather change ‘group-packages’ to ‘read-package-fields’ or something
>> like that.
>
> I changed it to 'group-package-fields' and added some comments.
>
>> identity
>
> (identity sublst) won't work.
>
> If (regexp-exec package-line-rx (first sublst)) returns #t,
> 'and=>' will call 'identity' with the result of 'regexp-exec'.  But it
> should return 'sublst' instead.

Ah, you want (and=> (regexp-exec ...) (const sublst)).
But then, it’s equivalent to just (and (regexp-exec ...) sublst).

> +(define-record-type <gnu-package-descriptor>

Yes, define-record-type* will definitely help.

> +  (package          get-gnu-package-name)

Here the field name should rather be ‘name’, for consistency.

Never ever write ‘get-’ for getters.  ;-)  It provides no useful
information.  See <package> and other records, as examples.

>  (define (official-gnu-packages)
>    "Return a list of GNU packages."

This should return directly a list of <gnu-package-descriptor> IMO, so
‘create-gnu-package-descriptor’ would be moved here.

> +  (define (group-package-fields port state)
> +    ;; Return a list of lists where /most/ inner lists are the GNU
> +    ;; packages.  Note that some lists are not packages at all; they
> +    ;; contain additional information.  So it is necessary to filter
> +    ;; the output.
> +    (let ((line (read-line port)))
> +      (define (match-field str)
> +        ;; Packages are separated by empty strings.  Each package is
> +        ;; represented as a list.  If STR is an empty string, create a new
> +        ;; list to store fields of a different package.  Otherwise, add STR 
> to
> +        ;; the same list.
> +        (match str
> +          ('""

Extra quote.

> +           (group-package-fields port (cons '() state)))
> +          (str
> +           (group-package-fields port (cons (cons str (first state))
> +                                            (drop state 1))))))
> +
> +      (if (eof-object? line)
> +          (remove null-list? state)
> +          (match-field line))))
> +
> +  (reverse (map reverse
> +                (group-package-fields (http-fetch* %package-list-url)
> +                                      '(())))))

Apparently ‘create-gnu-package-descriptor’ doesn’t rely on the order of
fields, so (map reverse ...) can be omitted, no?

> +(define (find-packages regexp)
> +  "Find packages that match REGEXP."
> +  (define (create-gnu-package-descriptor package)

s/create-gnu-package-descriptor/alist->package-descriptor/ ?

With a comment saying that PACKAGE is an alist.

>  network to check in GNU's database."
>       ;; TODO: Find a way to determine that a package is non-GNU without going
>       ;; through the network.
> -     (let ((url (and=> (package-source package) origin-uri)))
> +     (let ((url   (and=> (package-source package) origin-uri))
> +           (pname (package-name package)))

s/pname/name/

>         (or (and (string? url) (string-prefix? "mirror://gnu" url))
> -           (and (member (package-name package) (official-gnu-packages))

I think the last expression will become:

  (find (compose (cut equal? name <>) package-name)
        (official-gnu-packages))

Thanks!

Ludo’.



reply via email to

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