Re: All updaters are broken

[Ricardo Wurmus]

Hi Hartmut,

> Am 02.01.23 um 20:17 schrieb Ricardo Wurmus:
>
>  Thanks for providing the patch. For me this looks huge and hard to
> maintain.
>
> “Hard to maintain”?  How so?
>
> For me this double structure is hard to understand and thus to maintain. YMMV.

Okay.  Here’s something simpler using “partition”:

commit 96fb123832b262a3453fe1b7646758da235a343e
Author: Ricardo Wurmus <rekado@elephly.net>
Date:   Tue Jan 3 10:14:52 2023 +0100

    WIP

diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index e0b94ce48d..bbda2df35a 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -183,9 +183,9 @@ (define (show-help)
   (newline)
   (show-bug-report-information))
 
-(define (options->update-specs opts)
-  "Return the list of packages requested by OPTS, honoring options like
-'--recursive'."
+(define (options->packages+update-specs opts)
+  "Return the list of packages and update-specs requested by OPTS, honoring
+options like '--recursive'."
   (define core-package?
     (let* ((input->package (match-lambda
                              ((name (? package? package) _ ...) package)
@@ -220,7 +220,7 @@ (define (keep-newest package lst)
         (_
          (cons package lst)))))
 
-  (define args-packages
+  (define args-packages+update-specs
     ;; Packages explicitly passed as command-line arguments.
     (match (filter-map (match-lambda
                          (('argument . spec)
@@ -244,17 +244,18 @@ (define args-packages
       (some                                       ;user-specified packages
        some)))
 
-  (define packages
+  (define packages+update-specs
     (match (assoc-ref opts 'manifest)
-      (#f args-packages)
+      (#f args-packages+update-specs)
       ((? string? file) (packages-from-manifest file))))
 
   (if (assoc-ref opts 'recursive?)
+      (let ((packages update-specs (partition package? packages+update-specs)))
         (mlet %store-monad ((edges (node-edges %bag-node-type
                                                (all-packages))))
-        (return (node-transitive-edges packages edges)))
+          (return (append (node-transitive-edges packages edges) 
update-specs))))
       (with-monad %store-monad
-        (return packages))))
+        (return packages+update-specs))))
 
 
 ;;;
@@ -561,12 +562,13 @@ (define (options->updaters opts)
     (with-error-handling
       (with-store store
         (run-with-store store
-          (mlet %store-monad ((update-specs (options->update-specs opts)))
+          (mlet %store-monad ((packages+update-specs 
(options->packages+update-specs opts)))
+            (let ((packages update-specs (partition package? 
packages+update-specs)))
               (cond
                (list-dependent?
-              (list-dependents (map update-spec-package update-specs)))
+                (list-dependents (append packages (map update-spec-package 
update-specs))))
                (list-transitive?
-              (list-transitive (map update-spec-package update-specs)))
+                (list-transitive (append packages (map update-spec-package 
update-specs))))
                (update?
                 (parameterize ((%openpgp-key-server
                                 (or (assoc-ref opts 'key-server)
@@ -587,9 +589,18 @@ (define (options->updaters opts)
                                      #:key-download key-download
                                      #:warn? warn?))
                    update-specs)
+                  (for-each
+                   (lambda (package)
+                     (update-package store
+                                     package
+                                     #false
+                                     updaters
+                                     #:key-download key-download
+                                     #:warn? warn?))
+                   packages)
                   (return #t)))
                (else
                 (for-each (cut check-for-package-update <> updaters
                                #:warn? warn?)
-                        (map update-spec-package update-specs))
-              (return #t)))))))))
+                          (append packages (map update-spec-package 
update-specs)))
+                (return #t))))))))))

(This patch ignores white-space.) 

Here’s the patch with white-space changes:

refresh.diff
Description: Text Data

I can’t say whether this is better than your proposal as I’m biased, so
maybe let’s get someone else’s opinion on this before merging either of
them.  I have a slight preference for this approach over wrapping and
unwrapping.  Ideally we would avoid mixing up packages and update specs
in the first place, but that’s not easily accomplished now.

-- 
Ricardo