guix-package --search

guix-package --search

[Nikita Karetnikov]

[-- Attachment #1: Type: text/plain, Size: 80 bytes --]

Hi,

I've decided to let you know that I'm going to implement 'search'.

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --search

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 1697 bytes --]

Hi,

I'm attaching the diff.

Some comments:

1. 'psd-list' is a potential bottleneck.  It will cause problems when we
   have more packages.  If it's possible to evaluate it lazily, I'll
   rewrite it.  (I haven't checked yet.)

2. The above is valid for 'package?' too.  Also, it's probably possible
   to rewrite it using 'match', but I haven't found a way to do so.

3. The command-line part just mimics 'list-available'.  (Maybe there is
   a better way.)

4. I've noticed that 'fold-packages' returns duplicates.  Should it be
   fixed?  Should I filter the output instead?

   Examples:

   # ./pre-inst-env guile
   scheme@(guile-user)> ,use (guix ui) (gnu packages) (guix packages)
   scheme@(guile-user)> (for-each display
                                  (sort 
                                   (map (lambda (x) 
                        		          (string-append (package-name x) " "
                                                         (location->string (package-location x))
                                                         "\n"))
				                        (fold-packages cons '()))
                                   string<?))
   [...]

   glibc gnu/packages/base.scm:497:3
   glibc gnu/packages/base.scm:497:3

   [...]

   guile gnu/packages/guile.scm:93:3
   guile gnu/packages/guile.scm:93:3

   # ./pre-inst-env guix-package -s "C library"

   [...]

   glibc   2.17    gnu/packages/base.scm:497:3
   glibc   2.17    gnu/packages/base.scm:497:3

   # ./pre-inst-env guix-package -s Guile

   [...]

   guile   2.0.7   gnu/packages/guile.scm:93:3
   guile   2.0.7   gnu/packages/guile.scm:93:3

Nikita


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: guix-package-search.diff --]
[-- Type: text/x-diff, Size: 2973 bytes --]

--- guix-package-orig	2013-01-20 16:43:13.000000000 +0000
+++ guix-package	2013-01-20 20:47:38.000000000 +0000
@@ -230,6 +230,32 @@
            (leave (_ "error: no previous profile; not rolling back~%")))
           (else (switch-link)))))
 
+(define (sd-search rx)
+  "Search in SYNOPSIS and DESCRIPTION using RX.  Return a list of
+matching packages."
+  (define psd-list
+    ;; Return a list of lists (each inner list contains PACKAGE-NAME,
+    ;; SYNOPSIS, and DESCRIPTION of every package).
+    (map (lambda (x)
+           (list x (package-synopsis x) (package-description x)))
+         (fold-packages cons '())))
+
+  (define (matcher str)
+    ;; Match RX against STR.  Return the result or #f if nothing was
+    ;; found.
+    (false-if-exception (match:substring (regexp-exec rx str) 0)))
+
+  (define (package? lst)
+    ;; Return PACKAGE wrapped in a list or '() if match failed.
+    (if (any (cut matcher <>) (list (cadr lst) (caddr lst)))
+        (list (car lst))
+        '()))
+
+  (sort (append-map package? psd-list)
+        (lambda (p1 p2)
+          (string<? (package-name p1)
+                    (package-name p2)))))
+
 \f
 ;;;
 ;;; Command-line options.
@@ -261,6 +287,8 @@
       --verbose          produce verbose output"))
   (newline)
   (display (_ "
+  -s, --search=REGEXP    search in synopsis and description using REGEXP"))
+  (display (_ "
   -I, --list-installed[=REGEXP]
                          list installed packages matching REGEXP"))
   (display (_ "
@@ -306,6 +334,10 @@
         (option '("verbose") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'verbose? #t result)))
+        (option '(#\s "search") #t #f
+                  (lambda (opt name arg result)
+                    (cons `(query search ,(or arg ""))
+                          result)))
         (option '(#\I "list-installed") #f #t
                 (lambda (opt name arg result)
                   (cons `(query list-installed ,(or arg ""))
@@ -526,6 +558,7 @@
                                  name (or version "?") output path))))
                      installed)
            #t))
+
         (('list-available regexp)
          (let* ((regexp    (and regexp (make-regexp regexp)))
                 (available (fold-packages
@@ -548,6 +581,16 @@
                              (string<? (package-name p1)
                                        (package-name p2)))))
            #t))
+
+        (('search regexp)
+         (let ((regexp (and regexp (make-regexp regexp))))
+           (for-each (lambda (p)
+                       (format #t "~a\t~a\t~a~%"
+                               (package-name p)
+                               (package-version p)
+                               (location->string (package-location p))))
+                     (sd-search regexp))
+           #t))
         (_ #f))))
 
   (setlocale LC_ALL "")

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --search

[Ludovic Courtès]

Hi,

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> 1. 'psd-list' is a potential bottleneck.  It will cause problems when we
>    have more packages.  If it's possible to evaluate it lazily, I'll
>    rewrite it.  (I haven't checked yet.)

See below.

[...]

> 4. I've noticed that 'fold-packages' returns duplicates.  Should it be
>    fixed?  Should I filter the output instead?

These are not really duplicates, actually: these are variants of the
same packages, created using ‘inherit’, and keeping the original
package’s source location info (which could be fixed).

That said, it would make sense to ‘delete-duplicates’ any two packages
whose ‘package-location’ are ‘eq?’.  But that can be left as a separate
commit.

The patch looks good overall.  A few comments:

> +(define (sd-search rx)

Rather ‘find-packages-by-description’ or similar.

> +  "Search in SYNOPSIS and DESCRIPTION using RX.  Return a list of
> +matching packages."
> +  (define psd-list
> +    ;; Return a list of lists (each inner list contains PACKAGE-NAME,
> +    ;; SYNOPSIS, and DESCRIPTION of every package).
> +    (map (lambda (x)
> +           (list x (package-synopsis x) (package-description x)))
> +         (fold-packages cons '())))

Instead, you can directly build the list of matching packages, like:

  (fold-packages (lambda (package result)
                   (if (or (regexp-exec rx (package-synopsis package))
                           (regexp-exec rx (package-description package)))
                       (cons package result)
                       result))
                 '())

This way, only one traversal is done.

For i18n, we should actually use (gettext (package-description
package)), likewise for synopsis.  This way, that will search through
text in the user’s native language.

> +        (('search regexp)
> +         (let ((regexp (and regexp (make-regexp regexp))))
> +           (for-each (lambda (p)
> +                       (format #t "~a\t~a\t~a~%"
> +                               (package-name p)
> +                               (package-version p)
> +                               (location->string (package-location p))))
> +                     (sd-search regexp))
> +           #t))

Perfect.

Thanks!

Ludo’.

Re: guix-package --search

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 1036 bytes --]

> Instead, you can directly build the list of matching packages, like:

>   (fold-packages (lambda (package result)
>                    (if (or (regexp-exec rx (package-synopsis package))
>                            (regexp-exec rx (package-description package)))
>                        (cons package result)
>                        result))
>                  '())

> This way, only one traversal is done.

'regexp-exec' will raise an error if either 'synopsis' or 'description'
is not a string.  That's why I wrapped it in 'false-if-exception'.

> For i18n, we should actually use (gettext (package-description
> package)), likewise for synopsis.  This way, that will search through
> text in the user’s native language.

I added it, but I haven't properly tested it.  Also, there is
'remove-duplicates' that works in the REPL.  But it doesn't remove
duplicates when I call it via 'guix-package -s'.  (Sometimes they appear
and sometimes they don't show up.)  Anyway, I'm attaching the patch.

Nikita


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-guix-package-Add-search.patch --]
[-- Type: text/x-diff, Size: 4229 bytes --]

From a365d7f796d0db711bb67902f65826af9e951d06 Mon Sep 17 00:00:00 2001
From: Nikita Karetnikov <nikita@karetnikov.org>
Date: Wed, 23 Jan 2013 15:22:34 +0000
Subject: [PATCH] guix-package: Add '--search'.

* guix-package.in (find-packages-by-description): New procedure.
  (show-help): Add '--search'.
  (%options): Likewise.
  (guix-package)[process-query]: Add support for '--search'.
---
 guix-package.in |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/guix-package.in b/guix-package.in
index 85ac358..16a268b 100644
--- a/guix-package.in
+++ b/guix-package.in
@@ -229,6 +229,39 @@ all of PACKAGES, a list of name/version/output/path tuples."
            (leave (_ "error: no previous profile; not rolling back~%")))
           (else (switch-link)))))
 
+(define (find-packages-by-description rx)
+  "Search in SYNOPSIS and DESCRIPTION using RX.  Return a list of
+matching packages."
+  (define (remove-duplicates pred lst)
+    ;; Remove duplicates from sorted LST using PRED.
+    (cond ((null-list? lst)   lst)
+          ((= (length lst) 1) lst)
+          ((= (length lst) 2)
+           (if (pred (first lst) (second lst)) (cdr lst) lst))
+          ((pred (first lst) (second lst))
+           (remove-duplicates pred (cdr lst)))
+          (else (cons (first lst) (remove-duplicates pred (cdr lst))))))
+
+  (define (same-location? p1 p2)
+    ;; Compare locations of two packages.
+    (eq? (package-location p1) (package-location p2)))
+
+  (remove-duplicates
+   same-location?
+   (stable-sort
+    (fold-packages
+     (lambda (package result)
+       (if (or (false-if-exception
+                (regexp-exec rx (gettext (package-synopsis package))))
+               (false-if-exception
+                (regexp-exec rx (gettext (package-description package)))))
+           (cons package result)
+           result))
+     '())
+    (lambda (p1 p2)
+      (string<? (package-name p1)
+                (package-name p2))))))
+
 \f
 ;;;
 ;;; Command-line options.
@@ -251,6 +284,8 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
       --roll-back        roll back to the previous generation"))
   (newline)
   (display (_ "
+  -s, --search=REGEXP    search in synopsis and description using REGEXP"))
+  (display (_ "
   -p, --profile=PROFILE  use PROFILE instead of the user's default profile"))
   (display (_ "
   -n, --dry-run          show what would be done without actually doing it"))
@@ -305,6 +340,10 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
         (option '("verbose") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'verbose? #t result)))
+        (option '(#\s "search") #t #f
+                (lambda (opt name arg result)
+                  (cons `(query search ,(or arg ""))
+                        result)))
         (option '(#\I "list-installed") #f #t
                 (lambda (opt name arg result)
                   (cons `(query list-installed ,(or arg ""))
@@ -525,6 +564,7 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                                  name (or version "?") output path))))
                      installed)
            #t))
+
         (('list-available regexp)
          (let* ((regexp    (and regexp (make-regexp regexp)))
                 (available (fold-packages
@@ -547,6 +587,16 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                              (string<? (package-name p1)
                                        (package-name p2)))))
            #t))
+
+        (('search regexp)
+         (let ((regexp (and regexp (make-regexp regexp))))
+           (for-each (lambda (p)
+                       (format #t "~a\t~a\t~a~%"
+                               (package-name p)
+                               (package-version p)
+                               (location->string (package-location p))))
+                     (find-packages-by-description regexp))
+           #t))
         (_ #f))))
 
   (setlocale LC_ALL "")
-- 
1.7.5.4


[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --search

[Ludovic Courtès]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> Instead, you can directly build the list of matching packages, like:
>
>>   (fold-packages (lambda (package result)
>>                    (if (or (regexp-exec rx (package-synopsis package))
>>                            (regexp-exec rx (package-description package)))
>>                        (cons package result)
>>                        result))
>>                  '())
>
>> This way, only one traversal is done.
>
> 'regexp-exec' will raise an error if either 'synopsis' or 'description'
> is not a string.  That's why I wrapped it in 'false-if-exception'.

OK.

>> For i18n, we should actually use (gettext (package-description
>> package)), likewise for synopsis.  This way, that will search through
>> text in the user’s native language.
>
> I added it, but I haven't properly tested it.  Also, there is
> 'remove-duplicates' that works in the REPL.

Yes, see below.

> +(define (find-packages-by-description rx)
> +  "Search in SYNOPSIS and DESCRIPTION using RX.  Return a list of
> +matching packages."
> +  (define (remove-duplicates pred lst)
> +    ;; Remove duplicates from sorted LST using PRED.
> +    (cond ((null-list? lst)   lst)
> +          ((= (length lst) 1) lst)
> +          ((= (length lst) 2)
> +           (if (pred (first lst) (second lst)) (cdr lst) lst))
> +          ((pred (first lst) (second lst))
> +           (remove-duplicates pred (cdr lst)))
> +          (else (cons (first lst) (remove-duplicates pred (cdr lst))))))

First, can you use ‘delete-duplicates’ from (srfi srfi-1) instead?

> +  (define (same-location? p1 p2)
> +    ;; Compare locations of two packages.
> +    (eq? (package-location p1) (package-location p2)))

Here you need to use ‘equal?’, not ‘eq?’: ‘equal?’ checks for structural
equality, whereas ‘eq?’ checks for pointer equality (info "(guile) Equality").

> +  (remove-duplicates
> +   same-location?
> +   (stable-sort
> +    (fold-packages
> +     (lambda (package result)
> +       (if (or (false-if-exception
> +                (regexp-exec rx (gettext (package-synopsis package))))
> +               (false-if-exception
> +                (regexp-exec rx (gettext (package-description package)))))
> +           (cons package result)
> +           result))

Instead of ‘false-if-exception’, I’d prefer:

  (lambda (package result)
    (define matches?
      (cut regexp-exec rx <>))

    (if (or (and=> (package-synopsis package)
                   (compose matches? gettext))
            ...

Other than that it looks good to me!

Could you please also add a couple of tests in tests/guix-package.sh,
and the corresponding doc in guix.texi?

Thanks!

Ludo’.

[PATCH] guix-package: Add '--search'. (was: guix-package --search)

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 228 bytes --]

>    (if (or (and=> (package-synopsis package)
>                   (compose matches? gettext))

I came up with a different solution, which seems more readable.  What
do you think?  (If you want, I'll use your version.)

Nikita


[-- Attachment #1.2: 0001-guix-package-Add-search.patch --]
[-- Type: text/x-diff, Size: 5709 bytes --]

From 0083eff18eb584213f55974807d4e0e6e29d3c73 Mon Sep 17 00:00:00 2001
From: Nikita Karetnikov <nikita@karetnikov.org>
Date: Sat, 26 Jan 2013 08:36:31 +0000
Subject: [PATCH] guix-package: Add '--search'.

* guix-package.in (find-packages-by-description): New procedure.
  (show-help, %options): Add '--search'.
  (guix-package)[process-query]: Add support for '--search'.
* doc/guix.texi (Invoking guix-package): Document it.
* tests/guix-package.sh: Add tests.
---
 doc/guix.texi         |    9 +++++++++
 guix-package.in       |   39 +++++++++++++++++++++++++++++++++++++++
 tests/guix-package.sh |    9 +++++++++
 3 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index e1ca095..01c60ae 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23,6 +23,7 @@
 @title{GNU Guix Reference Manual}
 @subtitle{Using the GNU Guix Functional Package Manager}
 @author Ludovic Courtès
+@author Nikita Karetnikov
 
 @page
 @vskip 0pt plus 1filll
@@ -533,6 +534,14 @@ availability of packages:
 
 @table @option
 
+@item --search=@var{regexp}
+@itemx -s @var{regexp}
+Search in the @emph{synopsis} and @emph{description} fields of the
+available packages.  And list the ones that match @var{regexp}.
+
+For each package, print the following items, separated by tabs: its
+name, version, and the source location of its definition.
+
 @item --list-installed[=@var{regexp}]
 @itemx -I [@var{regexp}]
 List currently installed packages in the specified profile.  When
diff --git a/guix-package.in b/guix-package.in
index 37a1df0..b88928a 100644
--- a/guix-package.in
+++ b/guix-package.in
@@ -229,6 +229,28 @@ all of PACKAGES, a list of name/version/output/path tuples."
            (leave (_ "error: no previous profile; not rolling back~%")))
           (else (switch-link)))))
 
+(define (find-packages-by-description rx)
+  "Search in SYNOPSIS and DESCRIPTION using RX.  Return a list of
+matching packages."
+  (define (same-location? p1 p2)
+    ;; Compare locations of two packages.
+    (equal? (package-location p1) (package-location p2)))
+
+  (delete-duplicates
+   (sort
+    (fold-packages (lambda (package result)
+                     (if (any (lambda (f)
+                                (false-if-exception
+                                 (regexp-exec rx (gettext (f package)))))
+                              (list package-synopsis package-description))
+                         (cons package result)
+                         result))
+                   '())
+    (lambda (p1 p2)
+      (string<? (package-name p1)
+                (package-name p2))))
+   same-location?))
+
 \f
 ;;;
 ;;; Command-line options.
@@ -260,6 +282,8 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
       --verbose          produce verbose output"))
   (newline)
   (display (_ "
+  -s, --search=REGEXP    search in synopsis and description using REGEXP"))
+  (display (_ "
   -I, --list-installed[=REGEXP]
                          list installed packages matching REGEXP"))
   (display (_ "
@@ -305,6 +329,10 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
         (option '("verbose") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'verbose? #t result)))
+        (option '(#\s "search") #t #f
+                (lambda (opt name arg result)
+                  (cons `(query search ,(or arg ""))
+                        result)))
         (option '(#\I "list-installed") #f #t
                 (lambda (opt name arg result)
                   (cons `(query list-installed ,(or arg ""))
@@ -525,6 +553,7 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                                  name (or version "?") output path))))
                      installed)
            #t))
+
         (('list-available regexp)
          (let* ((regexp    (and regexp (make-regexp regexp)))
                 (available (fold-packages
@@ -547,6 +576,16 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                              (string<? (package-name p1)
                                        (package-name p2)))))
            #t))
+
+        (('search regexp)
+         (let ((regexp (and regexp (make-regexp regexp))))
+           (for-each (lambda (p)
+                       (format #t "~a\t~a\t~a~%"
+                               (package-name p)
+                               (package-version p)
+                               (location->string (package-location p))))
+                     (find-packages-by-description regexp))
+           #t))
         (_ #f))))
 
   (setlocale LC_ALL "")
diff --git a/tests/guix-package.sh b/tests/guix-package.sh
index 02ece68..89b2712 100644
--- a/tests/guix-package.sh
+++ b/tests/guix-package.sh
@@ -1,5 +1,6 @@
 # GNU Guix --- Functional package management for GNU
 # Copyright © 2012, 2013 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 #
 # This file is part of GNU Guix.
 #
@@ -68,6 +69,14 @@ then
 
     test "`guix-package -p "$profile" -I 'g.*e' | cut -f1`" = "guile-bootstrap"
 
+    # Search.
+    echo "Testing 'search'..."
+    if test "`guix-package -s "GNU Hello" | cut -f1`" = "hello";
+    then echo "Test1: OK"; else echo "Test1: failed"; fi
+
+    if test "`guix-package -s "n0t4r341p4ck4g3"`" = "";
+    then echo "Test2: OK"; else echo "Test2: failed"; fi
+
     # Remove a package.
     guix-package --bootstrap -p "$profile" -r "guile-bootstrap"
     test -L "$profile-3-link"
-- 
1.7.5.4


[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] guix-package: Add '--search'.

[Ludovic Courtès]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>>    (if (or (and=> (package-synopsis package)
>>                   (compose matches? gettext))
>
> I came up with a different solution, which seems more readable.  What
> do you think?  (If you want, I'll use your version.)

[...]

> From 0083eff18eb584213f55974807d4e0e6e29d3c73 Mon Sep 17 00:00:00 2001
> From: Nikita Karetnikov <nikita@karetnikov.org>
> Date: Sat, 26 Jan 2013 08:36:31 +0000
> Subject: [PATCH] guix-package: Add '--search'.
>
> * guix-package.in (find-packages-by-description): New procedure.
>   (show-help, %options): Add '--search'.
>   (guix-package)[process-query]: Add support for '--search'.
> * doc/guix.texi (Invoking guix-package): Document it.
> * tests/guix-package.sh: Add tests.

[...]

> +@item --search=@var{regexp}
> +@itemx -s @var{regexp}
> +Search in the @emph{synopsis} and @emph{description} fields of the
> +available packages.  And list the ones that match @var{regexp}.

Rather:

  List the available packages whose synopsis or description matches
  @var{regexp}.

(The end-user shouldn’t have to think in terms of structure fields.)

> +  (delete-duplicates
> +   (sort
> +    (fold-packages (lambda (package result)
> +                     (if (any (lambda (f)
> +                                (false-if-exception
> +                                 (regexp-exec rx (gettext (f package)))))
> +                              (list package-synopsis package-description))
> +                         (cons package result)
> +                         result))
> +                   '())

I still prefer the solution that avoids ‘false-if-exception’, because
‘false-if-exception’ could hide real issues other than wrong-type-arg.

> +    # Search.
> +    echo "Testing 'search'..."
> +    if test "`guix-package -s "GNU Hello" | cut -f1`" = "hello";
> +    then echo "Test1: OK"; else echo "Test1: failed"; fi
> +
> +    if test "`guix-package -s "n0t4r341p4ck4g3"`" = "";
> +    then echo "Test2: OK"; else echo "Test2: failed"; fi

Please remove the ‘echo’ and ‘if’: it’s in a ‘set -x’ so everything gets
written to guix-package.log, and it’s in a ‘set -e’ so any non-zero exit
causes a test failure.

You can commit once those tiny issues are fixed.

Thank you!

Ludo’.