Re: All updaters are broken

[Hartmut Goebel <h.goebel@crazy-compilers.com>]

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

Hello Ricardo,

Am 02.01.23 um 14:16 schrieb Ricardo Wurmus:
> Attached is a crude implementation of that.  I just consed the lists
> together instead of returning multiple values, because the compound
> value is to be used inside the store monad where we can’t easily access
> multiple values.

Thanks for providing the patch. For me this looks huge and hard to 
maintain. I'd rather make "options->update-specs" return update-specs in 
any cases. This adds a small overhead only in the case of --recursive.

Enclosed please find my proposal. WDY?

Tested cases

./pre-inst-env guix refresh --list-updaters
./pre-inst-env guix refresh -u python-flask
./pre-inst-env guix refresh -u python-flask=2.2.1
./pre-inst-env guix refresh python-flask
./pre-inst-env guix refresh python-flask=2.2.1
./pre-inst-env guix refresh --list-transitive python-flask
./pre-inst-env guix refresh --list-dependent python-flask
./pre-inst-env guix refresh -l python-flask

./pre-inst-env guix refresh -t hexpm -u
./pre-inst-env guix refresh -t hexpm
./pre-inst-env guix refresh -t hexpm erlang-relx -u
./pre-inst-env guix refresh -t hexpm erlang-relx

./pre-inst-env guix refresh -e '(@ (gnu packages erlang) erlang-relx)'
./pre-inst-env guix refresh -m test-manifest.scm

./pre-inst-env guix refresh --recursive python-flask
./pre-inst-env guix refresh --select=core

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

[-- Attachment #2: refresh.diff --]
[-- Type: text/x-patch, Size: 3946 bytes --]

diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index e0b94ce48d..f9c4a4c87c 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -184,8 +184,8 @@ specified with `--select'.\n"))
   (show-bug-report-information))
 
 (define (options->update-specs opts)
-  "Return the list of packages requested by OPTS, honoring options like
-'--recursive'."
+  "Return the list of <update-spec> records requested by OPTS, honoring
+options like '--recursive'."
   (define core-package?
     (let* ((input->package (match-lambda
                              ((name (? package? package) _ ...) package)
@@ -220,15 +220,15 @@ update would trigger a complete rebuild."
         (_
          (cons package lst)))))
 
-  (define args-packages
-    ;; Packages explicitly passed as command-line arguments.
+  (define args-packages->update-specs
+    ;; update-specs for packages explicitly passed as command-line arguments.
     (match (filter-map (match-lambda
                          (('argument . spec)
                           ;; Take either the specified version or the
                           ;; latest one.
                           (update-specification->update-spec spec))
                          (('expression . exp)
-                          (read/eval-package-expression exp))
+                          (package->update-spec (read/eval-package-expression exp)))
                          (_ #f))
                        opts)
       (()                                         ;default to all packages
@@ -236,25 +236,29 @@ update would trigger a complete rebuild."
                         ('core core-package?)
                         ('non-core (negate core-package?))
                         (_ (const #t)))))
-         (fold-packages (lambda (package result)
-                          (if (select? package)
-                              (keep-newest package result)
-                              result))
-                        '())))
+         (map package->update-spec
+              (fold-packages (lambda (package result)
+                               (if (select? package)
+                                   (keep-newest package result)
+                                   result))
+                             '()))))
       (some                                       ;user-specified packages
        some)))
 
-  (define packages
+  (define update-specs
     (match (assoc-ref opts 'manifest)
-      (#f args-packages)
-      ((? string? file) (packages-from-manifest file))))
+      (#f args-packages->update-specs)
+      ((? string? file) (map package->update-spec
+                             (packages-from-manifest file)))))
 
   (if (assoc-ref opts 'recursive?)
       (mlet %store-monad ((edges (node-edges %bag-node-type
                                              (all-packages))))
-        (return (node-transitive-edges packages edges)))
+        (return (map package->update-spec
+                     (node-transitive-edges (map update-spec-package update-specs)
+                                            edges))))
       (with-monad %store-monad
-        (return packages))))
+        (return update-specs))))
 
 \f
 ;;;
@@ -268,13 +272,17 @@ update would trigger a complete rebuild."
   (version update-spec-version))
 
 (define (update-specification->update-spec spec)
-  "Given SPEC, a package name like \"guile@2.0=2.0.8\", return a <update>
+  "Given SPEC, a package name like \"guile@2.0=2.0.8\", return a <update-spec>
 record with two fields: the package to upgrade, and the target version."
   (match (string-rindex spec #\=)
     (#f  (update-spec (specification->package spec) #f))
     (idx (update-spec (specification->package (substring spec 0 idx))
                       (substring spec (1+ idx))))))
 
+(define (package->update-spec package)
+  "Given PACKAGE return an <update-spec> record."
+  (update-spec package #f))
+
 \f
 ;;;
 ;;; Updates.