[bug#33805] : refresh: Allow searching recursively

Message ID 20181219201313.GC2581@macbook41
State Accepted
Headers show
Series [bug#33805] : refresh: Allow searching recursively | expand

Checks

Context Check Description
cbaines/applying patch fail Apply failed

Commit Message

Efraim Flashner Dec. 19, 2018, 8:13 p.m. UTC
Currently no test cases since `guix refresh` needs network access. I
currently have two implementations for refresh-recursive, the commented
one I started with and was adding cases which caused errors for guix
refresh. The second, with par-for-each¹, has an undefined return value,
so it doesn't care about errors (libuv-julia, autoconf-wrapper, etc).

I'm pretty sure the par-for-each implementation is faster too.


¹ https://www.gnu.org/software/guile/manual/guile.html#Parallel-Forms

Comments

Eric Bavier Dec. 19, 2018, 10:54 p.m. UTC | #1
On Wed, 19 Dec 2018 22:13:13 +0200
Efraim Flashner <efraim@flashner.co.il> wrote:

> From 8e73b5e0b7ecbac3f4c407cee407b918060b7d1b Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Wed, 19 Dec 2018 22:08:18 +0200
> Subject: [PATCH] scripts: refresh: Allow searching recursively.
> 
> * guix/scripts/refresh.scm (refresh-recursive, list-recursive): New
> procedures.
> (show-help): Document it.
> (guix-refresh): Add flags and checks for new options.
> * doc/guix.texi (Invoking guix refresh): Document new options.
> ---
>  doc/guix.texi            | 32 +++++++++++++++++++++
>  guix/scripts/refresh.scm | 62 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 20b5013fd..2888d1efb 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -7370,6 +7370,22 @@ are many packages, though, for which it lacks a method to determine
>  whether a new upstream release is available.  However, the mechanism is
>  extensible, so feel free to get in touch with us to add a new method!
>  
> +@table @code
> +
> +@item --recursive
> +Consider the packages specified, and all the packages upon which they depend.
<...>
> +@item --list-recursive
> +List all the packages which one or more packages depend upon.

I think it would be nice to have some more symmetry between the names
of these options and existing options.  Initially I was confused about
what business 'guix refresh' would have in being "recursive".

'guix build --sources=transitive' seems like it behaves similarly, in
that it visits package inputs.

'guix refresh --list-dependents' is the reverse of this
'--list-recursive', but the names do not immediately suggest this to
me.

`~Eric
Efraim Flashner Dec. 23, 2018, 10:06 a.m. UTC | #2
On Wed, Dec 19, 2018 at 04:54:32PM -0600, Eric Bavier wrote:
> On Wed, 19 Dec 2018 22:13:13 +0200
> Efraim Flashner <efraim@flashner.co.il> wrote:
> 
> > From 8e73b5e0b7ecbac3f4c407cee407b918060b7d1b Mon Sep 17 00:00:00 2001
> > From: Efraim Flashner <efraim@flashner.co.il>
> > Date: Wed, 19 Dec 2018 22:08:18 +0200
> > Subject: [PATCH] scripts: refresh: Allow searching recursively.
> > 
> > * guix/scripts/refresh.scm (refresh-recursive, list-recursive): New
> > procedures.
> > (show-help): Document it.
> > (guix-refresh): Add flags and checks for new options.
> > * doc/guix.texi (Invoking guix refresh): Document new options.
> > ---
> >  doc/guix.texi            | 32 +++++++++++++++++++++
> >  guix/scripts/refresh.scm | 62 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 94 insertions(+)
> > 
> > diff --git a/doc/guix.texi b/doc/guix.texi
> > index 20b5013fd..2888d1efb 100644
> > --- a/doc/guix.texi
> > +++ b/doc/guix.texi
> > @@ -7370,6 +7370,22 @@ are many packages, though, for which it lacks a method to determine
> >  whether a new upstream release is available.  However, the mechanism is
> >  extensible, so feel free to get in touch with us to add a new method!
> >  
> > +@table @code
> > +
> > +@item --recursive
> > +Consider the packages specified, and all the packages upon which they depend.
> <...>
> > +@item --list-recursive
> > +List all the packages which one or more packages depend upon.
> 
> I think it would be nice to have some more symmetry between the names
> of these options and existing options.  Initially I was confused about
> what business 'guix refresh' would have in being "recursive".
> 
> 'guix build --sources=transitive' seems like it behaves similarly, in
> that it visits package inputs.
> 
> 'guix refresh --list-dependents' is the reverse of this
> '--list-recursive', but the names do not immediately suggest this to
> me.
> 

I created '--list-dependents' mostly on my way to '--recursive', but I
didn't really like the name either. '--list-transitive' works well for
me.

I also think it would be nice if I would parse the other command line
arguements, remove '--recursive', and pass the rest of them to 'guix
refresh' for each of the (now transitive) packages. Then 'guix refresh
--recursive --update foo' would work as expected™.
Efraim Flashner Dec. 24, 2018, 10:35 a.m. UTC | #3
Patch pushed with list-recursive -> list-transitive

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 20b5013fd..2888d1efb 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -7370,6 +7370,22 @@  are many packages, though, for which it lacks a method to determine
 whether a new upstream release is available.  However, the mechanism is
 extensible, so feel free to get in touch with us to add a new method!
 
+@table @code
+
+@item --recursive
+Consider the packages specified, and all the packages upon which they depend.
+
+@example
+$ guix refresh --recursive coreutils
+gnu/packages/acl.scm:35:2: warning: no updater for acl
+gnu/packages/m4.scm:30:12: info: 1.4.18 is already the latest version of m4
+gnu/packages/xml.scm:68:2: warning: no updater for expat
+gnu/packages/multiprecision.scm:40:12: info: 6.1.2 is already the latest version of gmp
+@dots{}
+@end example
+
+@end table
+
 Sometimes the upstream name differs from the package name used in Guix,
 and @command{guix refresh} needs a little help.  Most updaters honor the
 @code{upstream-name} property in package definitions, which can be used
@@ -7543,6 +7559,22 @@  hop@@2.4.0 geiser@@0.4 notmuch@@0.18 mu@@0.9.9.5 cflow@@1.4 idutils@@4.6 @dots{}
 The command above lists a set of packages that could be built to check
 for compatibility with an upgraded @code{flex} package.
 
+@table @code
+
+@item --list-recursive
+List all the packages which one or more packages depend upon.
+
+@example
+$ guix refresh --list-recursive flex
+flex@2.6.4 depends on the following 25 packages: perl@5.28.0 help2man@1.47.6
+bison@3.0.5 indent@2.2.10 tar@1.30 gzip@1.9 bzip2@1.0.6 xz@5.2.4 file@5.33 @dote{}
+@end example
+
+@end table
+
+The command above lists a set of packages which, when changed, would cause
+@code{flex} to be rebuilt.
+
 The following options can be used to customize GnuPG operation:
 
 @table @code
diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 1d86f949c..08fd25118 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -5,6 +5,7 @@ 
 ;;; Copyright © 2015 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -40,6 +41,7 @@ 
   #:use-module (ice-9 regex)
   #:use-module (ice-9 vlist)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 threads) ; par-for-each
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
@@ -88,6 +90,12 @@ 
         (option '(#\l "list-dependent") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'list-dependent? #t result)))
+        (option '(#\r "recursive") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'recursive? #t result)))
+        (option '("list-recursive") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'list-recursive? #t result)))
 
         (option '("keyring") #t #f
                 (lambda (opt name arg result)
@@ -140,6 +148,10 @@  specified with `--select'.\n"))
   (display (G_ "
   -l, --list-dependent   list top-level dependent packages that would need to
                          be rebuilt as a result of upgrading PACKAGE..."))
+  (display (G_ "
+  -r, --recursive        check the PACKAGE and its inputs for upgrades"))
+  (display (G_ "
+      --list-recursive   list all the packages that PACKAGE depends on"))
   (newline)
   (display (G_ "
       --keyring=FILE     use FILE as the keyring of upstream OpenPGP keys"))
@@ -323,6 +335,50 @@  dependent packages are rebuilt: ~{~a~^ ~}~%"
                  (map full-name covering))))
       (return #t))))
 
+(define (refresh-recursive packages)
+  "Check all of the package inputs of PACKAGES for newer upstream versions."
+  (mlet %store-monad ((edges (node-edges %bag-node-type
+                                         ;; Here we don't want the -boot0 packages.
+                                         (fold-packages cons '()))))
+    (let ((dependent (node-transitive-edges packages edges)))
+      ;; par-for-each has an undefined return value, so packages which cause
+      ;; errors can be ignored.
+      (par-for-each (lambda (package)
+      ;(for-each (lambda (package)
+      ;            (unless (any (cute string-suffix? <> package)
+      ;                         ;; A partial list of package suffixes that
+      ;                         ;; cause guix-refresh to error.
+      ;                         '("bootstrap" "for-build" "intermediate"
+      ;                           "minimal" "union" "without-mesa" "wrapper"))
+                    (guix-refresh package))
+      ;              )
+                (map package-name dependent)))
+    (return #t)))
+
+(define (list-recursive packages)
+  "List all the things that would cause PACKAGES to be rebuilt if they are changed."
+  ;; Using %BAG-NODE-TYPE is more accurate than using %PACKAGE-NODE-TYPE
+  ;; because it includes implicit dependencies.
+  (define (full-name package)
+    (string-append (package-name package) "@"
+                   (package-version package)))
+
+  (mlet %store-monad ((edges (node-edges %bag-node-type
+                                         ;; Here we don't want the -boot0 packages.
+                                         (fold-packages cons '()))))
+    (let ((dependent (node-transitive-edges packages edges)))
+      (match packages
+        ((x)
+         (format (current-output-port)
+                 (G_ "~a depends on the following ~d packages: ~{~a~^ ~}~%.")
+                 (full-name x) (length dependent) (map full-name dependent)))
+        (lst
+         (format (current-output-port)
+                 (G_ "The following ~d packages \
+all are dependent packages: ~{~a~^ ~}~%")
+                 (length dependent) (map full-name dependent))))
+      (return #t))))
+
 
 ;;;
 ;;; Manifest.
@@ -402,7 +458,9 @@  update would trigger a complete rebuild."
   (let* ((opts            (parse-options))
          (update?         (assoc-ref opts 'update?))
          (updaters        (options->updaters opts))
+         (recursive?      (assoc-ref opts 'recursive?))
          (list-dependent? (assoc-ref opts 'list-dependent?))
+         (list-recursive? (assoc-ref opts 'list-recursive?))
          (key-download    (assoc-ref opts 'key-download))
 
          ;; Warn about missing updaters when a package is explicitly given on
@@ -441,6 +499,10 @@  update would trigger a complete rebuild."
           (cond
            (list-dependent?
             (list-dependents packages))
+           (list-recursive?
+            (list-recursive packages))
+           (recursive?
+            (refresh-recursive packages))
            (update?
             (parameterize ((%openpgp-key-server
                             (or (assoc-ref opts 'key-server)