[PATCH 0/2] emacs: Add "M-x guix-package-from-file.

[PATCH 0/2] emacs: Add "M-x guix-package-from-file.

[Alex Kost]

This is to add an Emacs companion of "guix build --file" and
"guix package --install-from-file".

Patches:

[PATCH 1/2] emacs: main: Remove top-level package tables.

  This patch finally rewrites that part of code in a functional style.
  Many thanks to Ludovic for past comments on using 'delay' and vhashes.

[PATCH 2/2] emacs: Add 'guix-package-from-file' command.

  And this patch partially breaks the functional style.  A new package
  (from an arbitrary file) must be registered in a table of packages, so
  that it can be addressed later (for example, if a user would want to
  install it).  So this is done using not very functional 'set!' in
  'register-package' procedure (at least it sets a local variable).  Is
  there a better way to write this?

[PATCH 1/2] emacs: main: Remove top-level package tables.

[Alex Kost]

* emacs/guix-main.scm (%package-vhash, package-vhash, %package-table)
(package-table, name+version->key, key->name+version): Remove.
(package-by-address, packages-by-name+version): Adjust by using delayed
local package tables.
---
 emacs/guix-main.scm | 73 ++++++++++++++++++-----------------------------------
 1 file changed, 24 insertions(+), 49 deletions(-)

diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 2be86f0..5d7df2a 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -32,14 +32,6 @@
 ;; still "name-version" string.  So ‘id’ package parameter in the code
 ;; below is either an object-address number or a full-name string.
 
-;; To speed-up the process of getting information, the following
-;; auxiliary variables are used:
-;;
-;; - `%packages' - VHash of "package address"/"package" pairs.
-;;
-;; - `%package-table' - Hash table of
-;;   "name+version key"/"list of packages" pairs.
-
 ;;; Code:
 
 (use-modules
@@ -101,38 +93,6 @@ return two values: name and version.  For example, for SPEC
         (string-append full-name ":" output)
         full-name)))
 
-(define name+version->key cons)
-(define key->name+version car+cdr)
-
-(define %package-vhash
-  (delay
-    (fold-packages (lambda (pkg res)
-                     (vhash-consq (object-address pkg) pkg res))
-                   vlist-null)))
-
-(define (package-vhash)
-  "Return vhash of 'package ID (address)'/'package' pairs."
-  (force %package-vhash))
-
-(define %package-table
-  (delay
-    (let ((table (make-hash-table (vlist-length (package-vhash)))))
-      (vlist-for-each
-       (lambda (elem)
-         (match elem
-           ((address . pkg)
-            (let* ((key (name+version->key (package-name pkg)
-                                           (package-version pkg)))
-                   (ref (hash-ref table key)))
-              (hash-set! table key
-                         (if ref (cons pkg ref) (list pkg)))))))
-       (package-vhash))
-      table)))
-
-(define (package-table)
-  "Return hash table of 'name+version key'/'list of packages' pairs."
-  (force %package-table))
-
 (define (manifest-entry->name+version+output entry)
   (values
    (manifest-entry-name    entry)
@@ -340,15 +300,30 @@ Example:
 \f
 ;;; Finding packages.
 
-(define (package-by-address address)
-  (match (vhash-assq address (package-vhash))
-    ((_ . package) package)
-    (_ #f)))
-
-(define (packages-by-name+version name version)
-  (or (hash-ref (package-table)
-                (name+version->key name version))
-      '()))
+(define package-by-address
+  (let ((table (delay (fold-packages
+                       (lambda (package table)
+                         (vhash-consq (object-address package)
+                                      package table))
+                       vlist-null))))
+    (lambda (address)
+      "Return package by its object ADDRESS."
+      (match (vhash-assq address (force table))
+        ((_ . package) package)
+        (_ #f)))))
+
+(define packages-by-name+version
+  (let ((table (delay (fold-packages
+                       (lambda (package table)
+                         (let ((file (location-file
+                                      (package-location package))))
+                           (vhash-cons (cons (package-name package)
+                                             (package-version package))
+                                       package table)))
+                       vlist-null))))
+    (lambda (name version)
+      "Return packages matching NAME and VERSION."
+      (vhash-fold* cons '() (cons name version) (force table)))))
 
 (define (packages-by-full-name full-name)
   (call-with-values
-- 
2.7.3

Re: [PATCH 1/2] emacs: main: Remove top-level package tables.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> * emacs/guix-main.scm (%package-vhash, package-vhash, %package-table)
> (package-table, name+version->key, key->name+version): Remove.
> (package-by-address, packages-by-name+version): Adjust by using delayed
> local package tables.

LGTM, thanks.  :-)

Ludo’.

[PATCH 2/2] emacs: Add 'guix-package-from-file' command.

[Alex Kost]

* emacs/guix-main.scm (register-package, packages-from-file): New procedures.
(%patterns-makers): Add 'from-file' search type.
* emacs/guix-messages.el (guix-messages): Add messages for it.
* emacs/guix-ui-package.el (guix-package-from-file): New command.
(guix-package-info-insert-location): Adjust for 'from-file' type.
* doc/emacs.texi (Emacs Commands): Document it.
---
 doc/emacs.texi           |  5 +++++
 emacs/guix-main.scm      | 47 ++++++++++++++++++++++++++++++++++++-----------
 emacs/guix-messages.el   |  7 +++++++
 emacs/guix-ui-package.el | 33 ++++++++++++++++++++++++---------
 4 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/doc/emacs.texi b/doc/emacs.texi
index 575e87c..1962ffc 100644
--- a/doc/emacs.texi
+++ b/doc/emacs.texi
@@ -166,6 +166,11 @@ Display package(s) located in the specified file.  These files usually
 have the following form: @file{gnu/packages/emacs.scm}, but don't type
 them manually!  Press @key{TAB} to complete the file name.
 
+@item M-x guix-package-from-file
+Display package that the code within the specified file evaluates to.
+@xref{Invoking guix package, @code{--install-from-file}}, for an example
+of how a file may look like.
+
 @item M-x guix-search-by-regexp
 Search for packages by a specified regexp.  By default ``name'',
 ``synopsis'' and ``description'' of the packages will be searched.  This
diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 5d7df2a..5068ba1 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -300,17 +300,29 @@ Example:
 \f
 ;;; Finding packages.
 
-(define package-by-address
-  (let ((table (delay (fold-packages
-                       (lambda (package table)
-                         (vhash-consq (object-address package)
-                                      package table))
-                       vlist-null))))
-    (lambda (address)
-      "Return package by its object ADDRESS."
-      (match (vhash-assq address (force table))
-        ((_ . package) package)
-        (_ #f)))))
+(define-values (package-by-address
+                register-package)
+  (let* ((table (delay (fold-packages
+                        (lambda (package table)
+                          (vhash-consq (object-address package)
+                                       package table))
+                        vlist-null)))
+         (table* (lambda ()
+                   (if (promise? table)
+                       (force table)
+                       table))))
+    (values
+     (lambda (address)
+       "Return package by its object ADDRESS."
+       (match (vhash-assq address (table*))
+         ((_ . package) package)
+         (_ #f)))
+     (lambda (package)
+       "Register PACKAGE by its 'object-address', so that later
+'package-by-address' can be used to access it."
+       (set! table
+             (vhash-consq (object-address package)
+                          package (table*)))))))
 
 (define packages-by-name+version
   (let ((table (delay (fold-packages
@@ -410,6 +422,15 @@ MATCH-PARAMS is a list of parameters that REGEXP can match."
               '()
               (find-newest-available-packages)))
 
+(define (packages-from-file file)
+  "Return a list of packages from FILE."
+  (let ((package (load (canonicalize-path file))))
+    (if (package? package)
+        (begin
+          (register-package package)
+          (list package))
+        '())))
+
 \f
 ;;; Making package/output patterns.
 
@@ -662,6 +683,8 @@ ENTRIES is a list of installed manifest entries."
                                    (lookup-license license-name))))
          (location-proc         (lambda (_ location)
                                   (packages-by-location-file location)))
+         (file-proc             (lambda (_ file)
+                                  (packages-from-file file)))
          (all-proc              (lambda _ (all-available-packages)))
          (newest-proc           (lambda _ (newest-available-packages))))
     `((package
@@ -672,6 +695,7 @@ ENTRIES is a list of installed manifest entries."
        (regexp           . ,regexp-proc)
        (license          . ,license-proc)
        (location         . ,location-proc)
+       (from-file        . ,file-proc)
        (all-available    . ,all-proc)
        (newest-available . ,newest-proc))
       (output
@@ -682,6 +706,7 @@ ENTRIES is a list of installed manifest entries."
        (regexp           . ,regexp-proc)
        (license          . ,license-proc)
        (location         . ,location-proc)
+       (from-file        . ,file-proc)
        (all-available    . ,all-proc)
        (newest-available . ,newest-proc)))))
 
diff --git a/emacs/guix-messages.el b/emacs/guix-messages.el
index 7ebe7e8..52436af 100644
--- a/emacs/guix-messages.el
+++ b/emacs/guix-messages.el
@@ -44,6 +44,9 @@
       ,(lambda (_ entries locations)
          (apply #'guix-message-packages-by-location
                 entries 'package locations)))
+     (from-file
+      (0 "No package in file '%s'." val)
+      (1 "Package from file '%s'." val))
      (regexp
       (0 "No packages matching '%s'." val)
       (1 "A single package matching '%s'." val)
@@ -80,6 +83,10 @@
       ,(lambda (_ entries locations)
          (apply #'guix-message-packages-by-location
                 entries 'output locations)))
+     (from-file
+      (0 "No package in file '%s'." val)
+      (1 "Package from file '%s'." val)
+      (many "Package outputs from file '%s'." val))
      (regexp
       (0 "No package outputs matching '%s'." val)
       (1 "A single package output matching '%s'." val)
diff --git a/emacs/guix-ui-package.el b/emacs/guix-ui-package.el
index 38f0c08..edc3648 100644
--- a/emacs/guix-ui-package.el
+++ b/emacs/guix-ui-package.el
@@ -393,15 +393,17 @@ formatted with this string, an action button is inserted.")
       (guix-format-insert nil)
     (let ((location-file (car (split-string location ":"))))
       (guix-info-insert-value-indent location 'guix-package-location)
-      (guix-info-insert-indent)
-      (guix-info-insert-action-button
-       "Packages"
-       (lambda (btn)
-         (guix-package-get-display (guix-ui-current-profile)
-                                   'location
-                                   (button-get btn 'location)))
-       (format "Display packages from location '%s'" location-file)
-       'location location-file))))
+      ;; Do not show "Packages" button if a package 'from file' is displayed.
+      (unless (eq (guix-ui-current-search-type) 'from-file)
+        (guix-info-insert-indent)
+        (guix-info-insert-action-button
+         "Packages"
+         (lambda (btn)
+           (guix-package-get-display (guix-ui-current-profile)
+                                     'location
+                                     (button-get btn 'location)))
+         (format "Display packages from location '%s'" location-file)
+         'location location-file)))))
 
 (defun guix-package-info-insert-systems (systems entry)
   "Insert supported package SYSTEMS at point."
@@ -1001,6 +1003,19 @@ Interactively with prefix, prompt for PROFILE."
   (guix-package-get-display profile 'location location))
 
 ;;;###autoload
+(defun guix-package-from-file (file &optional profile)
+  "Display Guix package that the code from FILE evaluates to.
+If PROFILE is nil, use `guix-current-profile'.
+Interactively with prefix, prompt for PROFILE."
+  (interactive
+   (list (read-file-name "File with package: ")
+         (guix-ui-read-profile)))
+  (guix-buffer-get-display-entries
+   'info 'package
+   (list (or profile guix-current-profile) 'from-file file)
+   'add))
+
+;;;###autoload
 (defun guix-search-by-regexp (regexp &optional params profile)
   "Search for Guix packages by REGEXP.
 PARAMS are package parameters that should be searched.
-- 
2.7.3

Re: [PATCH 2/2] emacs: Add 'guix-package-from-file' command.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> * emacs/guix-main.scm (register-package, packages-from-file): New procedures.
> (%patterns-makers): Add 'from-file' search type.
> * emacs/guix-messages.el (guix-messages): Add messages for it.
> * emacs/guix-ui-package.el (guix-package-from-file): New command.
> (guix-package-info-insert-location): Adjust for 'from-file' type.
> * doc/emacs.texi (Emacs Commands): Document it.

[...]

> +@item M-x guix-package-from-file
> +Display package that the code within the specified file evaluates to.
> +@xref{Invoking guix package, @code{--install-from-file}}, for an example
> +of how a file may look like.

s/how a file/what such a file/

> +(define-values (package-by-address
> +                register-package)
> +  (let* ((table (delay (fold-packages
> +                        (lambda (package table)
> +                          (vhash-consq (object-address package)
> +                                       package table))
> +                        vlist-null)))
> +         (table* (lambda ()
> +                   (if (promise? table)
> +                       (force table)
> +                       table))))

It may be easier to always make ‘table’ a promise…

> +    (values
> +     (lambda (address)
> +       "Return package by its object ADDRESS."
> +       (match (vhash-assq address (table*))
> +         ((_ . package) package)
> +         (_ #f)))
> +     (lambda (package)
> +       "Register PACKAGE by its 'object-address', so that later
> +'package-by-address' can be used to access it."
> +       (set! table
> +             (vhash-consq (object-address package)
> +                          package (table*)))))))

… by wrapping ‘vhash-consq’ in ‘delay’.

I think this approach is OK.

The rest LGTM!

To avoid ‘set!’ above, the options that come to mind would be:

  1. To not provide M-x guix-package-from-file and instead provide, say,
     M-x guix-install-package-from-file.  That way, we wouldn’t need to
     remember the package.

  2. To thread the state, consisting mainly of lookup tables/procedures,
     through the state monad, and to change the state in this particular
     case.

     Converting to this new style would be quite a bit of work, for just
     this one special case.

Thanks,
Ludo’.

Re: [PATCH 2/2] emacs: Add 'guix-package-from-file' command.

[Alex Kost]

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

Ludovic Courtès (2016-05-08 21:40 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> * emacs/guix-main.scm (register-package, packages-from-file): New procedures.
>> (%patterns-makers): Add 'from-file' search type.
>> * emacs/guix-messages.el (guix-messages): Add messages for it.
>> * emacs/guix-ui-package.el (guix-package-from-file): New command.
>> (guix-package-info-insert-location): Adjust for 'from-file' type.
>> * doc/emacs.texi (Emacs Commands): Document it.
>
> [...]
>
>> +@item M-x guix-package-from-file
>> +Display package that the code within the specified file evaluates to.
>> +@xref{Invoking guix package, @code{--install-from-file}}, for an example
>> +of how a file may look like.
>
> s/how a file/what such a file/

Fixed.

>> +(define-values (package-by-address
>> +                register-package)
>> +  (let* ((table (delay (fold-packages
>> +                        (lambda (package table)
>> +                          (vhash-consq (object-address package)
>> +                                       package table))
>> +                        vlist-null)))
>> +         (table* (lambda ()
>> +                   (if (promise? table)
>> +                       (force table)
>> +                       table))))
>
> It may be easier to always make ‘table’ a promise…

I don't like this 'table*' procedure (especially its name), and I would
like to get rid of it, but I couldn't make your suggestion work…

>> +    (values
>> +     (lambda (address)
>> +       "Return package by its object ADDRESS."
>> +       (match (vhash-assq address (table*))
>> +         ((_ . package) package)
>> +         (_ #f)))
>> +     (lambda (package)
>> +       "Register PACKAGE by its 'object-address', so that later
>> +'package-by-address' can be used to access it."
>> +       (set! table
>> +             (vhash-consq (object-address package)
>> +                          package (table*)))))))
>
> … by wrapping ‘vhash-consq’ in ‘delay’.

… I tried it, but I got an error I don't know what to do with:

  Throw to key `vm-error' with args `(vm-run "VM: Stack overflow" ())'.

I attach the file that illustrates this problem.  Here is the recipe to
reproduce it using geiser:

1. M-x run-guile
2. Open the attached file
3. Evaluate it: C-c C-b

And the last expression leads to an error (at least for me).  Is there
some problem with combining force/delay or did I do something wrong?

> To avoid ‘set!’ above, the options that come to mind would be:
>
>   1. To not provide M-x guix-package-from-file and instead provide, say,
>      M-x guix-install-package-from-file.  That way, we wouldn’t need to
>      remember the package.

I don't like this solution.  With "M-x guix-package-from-file" you get a
full-featured *Guix Package Info* buffer, where you can not only install
the package, but download its source, look at its license, inputs,
etc. (and soon it will be possible just to build it without installing).

>   2. To thread the state, consisting mainly of lookup tables/procedures,
>      through the state monad, and to change the state in this particular
>      case.
>
>      Converting to this new style would be quite a bit of work, for just
>      this one special case.

Ouch, this looks scary to me, so I stay on the current solution, but
thanks for the pointers!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: from-file-delay-bug.scm --]
[-- Type: text/x-scheme, Size: 943 bytes --]

(use-modules
 (ice-9 vlist)
 (ice-9 match)
 (gnu packages)
 (gnu packages curl))

(define-values (package-by-address
                register-package)
  (let ((table (delay (fold-packages
                       (lambda (package table)
                         (vhash-consq (object-address package)
                                      package table))
                       vlist-null))))
    (values
     (lambda (address)
       "Return package by its object ADDRESS."
       (match (vhash-assq address (force table))
         ((_ . package) package)
         (_ #f)))
     (lambda (package)
       "Register PACKAGE by its 'object-address', so that later
'package-by-address' can be used to access it."
       (set! table
             (delay (vhash-consq (object-address package)
                                 package (force table))))))))

(register-package curl)

;; This expressions errors:
(package-by-address (object-address curl))

Re: [PATCH 2/2] emacs: Add 'guix-package-from-file' command.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2016-05-08 21:40 +0300) wrote:

[...]

>> … by wrapping ‘vhash-consq’ in ‘delay’.
>
> … I tried it, but I got an error I don't know what to do with:
>
>   Throw to key `vm-error' with args `(vm-run "VM: Stack overflow" ())'.

I needs to be changed like this:

--8<---------------cut here---------------start------------->8---
(define-values (package-by-address
                register-package)
  (let ((table (delay (fold-packages
                       (lambda (package table)
                         (vhash-consq (object-address package)
                                      package table))
                       vlist-null))))
    (values
     (lambda (address)
       "Return package by its object ADDRESS."
       (match (vhash-assq address (force table))
         ((_ . package) package)
         (_ #f)))
     (lambda (package)
       "Register PACKAGE by its 'object-address', so that later
'package-by-address' can be used to access it."
       (let ((table (force table)))
         (set! table
           (delay (vhash-consq (object-address package)
                               package table))))))))
--8<---------------cut here---------------end--------------->8---

In the example you posted, ‘register-package’ would turn ‘table’ into a
self-reference, hence the stack overflow.

>> To avoid ‘set!’ above, the options that come to mind would be:
>>
>>   1. To not provide M-x guix-package-from-file and instead provide, say,
>>      M-x guix-install-package-from-file.  That way, we wouldn’t need to
>>      remember the package.
>
> I don't like this solution.  With "M-x guix-package-from-file" you get a
> full-featured *Guix Package Info* buffer, where you can not only install
> the package, but download its source, look at its license, inputs,
> etc. (and soon it will be possible just to build it without installing).

Yeah, I mentioned it for completeness but I don’t like it either.

Thanks!

Ludo’.

Re: [PATCH 2/2] emacs: Add 'guix-package-from-file' command.

[Alex Kost]

Ludovic Courtès (2016-05-09 23:29 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2016-05-08 21:40 +0300) wrote:
>
> [...]
>
>>> … by wrapping ‘vhash-consq’ in ‘delay’.
>>
>> … I tried it, but I got an error I don't know what to do with:
>>
>>   Throw to key `vm-error' with args `(vm-run "VM: Stack overflow" ())'.
>
> I needs to be changed like this:
>
> (define-values (package-by-address
>                 register-package)
>   (let ((table (delay (fold-packages
>                        (lambda (package table)
>                          (vhash-consq (object-address package)
>                                       package table))
>                        vlist-null))))
>     (values
>      (lambda (address)
>        "Return package by its object ADDRESS."
>        (match (vhash-assq address (force table))
>          ((_ . package) package)
>          (_ #f)))
>      (lambda (package)
>        "Register PACKAGE by its 'object-address', so that later
> 'package-by-address' can be used to access it."
>        (let ((table (force table)))
>          (set! table
>            (delay (vhash-consq (object-address package)
>                                package table))))))))
>
> In the example you posted, ‘register-package’ would turn ‘table’ into a
> self-reference, hence the stack overflow.

Heh, so simple, thanks a lot!  I used this variant and committed this
patch, thanks again.

-- 
Alex