Re: Display diffs between generations.

[Benz Schenk <benz.schenk@uzh.ch>]

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

On Tue, 25 Oct 2016 18:01:23 +0200
Roel Janssen <roel@gnu.org> wrote:

> Ludovic Courtès writes:
> 
> > Hi!
> >
> > Benz Schenk <benz.schenk@uzh.ch> skribis:
> >  
> >> On Fri, 21 Oct 2016 11:37:00 +0200
> >> Roel Janssen <roel@gnu.org> wrote:  
> >
> > [...]
> >  
>  [...]  
> >>
> >> I adapted your patch to hopefully implement the desired behaviour, but
> >> it might need some cleaning up as I'm just getting started learning
> >> scheme.  
> >  
> >>From what I can see that Benz’ patch does indeed work as expected (but  
> > really, the example above is a corner case that we shouldn’t worry too
> > much about.)
> >
> > Roel, if that’s fine with you, please commit with proper commit log and
> > acknowledgment.
> >
> > Thanks to both of you.  :-)  
> 
> Thanks a lot Benz!
> 
> There's only one thing:
> Would it make more sense to stick to the chronology of the generations
> (sorting them before displaying them)?

IMO it's useful to see the diffs in reverse when before switching
to some previous generation, although you can easily see the changes
no matter how you order the generations, so I don't really have a strong
opinion either way.

>
> 
> If you think Benz's patch is good, then I will push that one.  Otherwise
> I'll adapt it to sort the generations.
> 
> @Benz, what's the copyright line you want to have in the patch?

I guess
Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>

> 
> Kind regards,
> Roel Janssen

Kind regards,
Benz Schenk

PS:

@Roel Janssen, sorry for double-posting I forgot to cc guix-devel

@everyone
on the bright side, I updated the patch to use display-generation
instead of the copy+pasted mess I created in the last patch and added
my copyright lines.

I also realized that with this patch, list-generations with
generations that do not differ, will simply display the generation number 
and date like

>Generation 54	Oct 19 2016 13:42:16
>[... <packages>]
>Generation 56	Oct 21 2016 15:12:24
>Generation 57	Oct 23 2016 18:15:03
>[... <package-diff>]

I'm not sure if that might be confusing.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-guix-package-Display-generation-diffs.patch --]
[-- Type: text/x-patch, Size: 4824 bytes --]

From 880d95a3779341cb2305a638fbad9364455a02eb Mon Sep 17 00:00:00 2001
From: Benz Schenk <benz.schenk@uzh.ch>
Date: Wed, 26 Oct 2016 12:31:03 +0200
Subject: [PATCH] guix package: Display generation diffs.

* guix/ui.scm (display-profile-content-diff): New variable.
* guix/scripts/package.scm (process-query): Use display-profile-content-diff.

---
 guix/scripts/package.scm | 17 +++++++++++++----
 guix/ui.scm              | 27 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index b87aee0..e85f3fb 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2013, 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2014, 2016 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -667,24 +668,32 @@ processed, #f otherwise."
                      ((head tail ...) head))))
     (match (assoc-ref opts 'query)
       (('list-generations pattern)
-       (define (list-generation number)
+       (define (list-generation display-function number)
          (unless (zero? number)
            (display-generation profile number)
-           (display-profile-content profile number)
+           (display-function profile number)
            (newline)))
+       (define (diff-profiles profile numbers)
+         (unless (null-list? (cdr numbers))
+           (display-profile-content-diff profile (car numbers) (cadr numbers)))
+         (unless (null-list? (cdr numbers))
+           (diff-profiles profile (cdr numbers))))
 
        (cond ((not (file-exists? profile))      ; XXX: race condition
               (raise (condition (&profile-not-found-error
                                  (profile profile)))))
              ((string-null? pattern)
-              (for-each list-generation (profile-generations profile)))
+              (list-generation display-profile-content
+                               (car (profile-generations profile)))
+              (diff-profiles profile (profile-generations profile)))
              ((matching-generations pattern profile)
               =>
               (lambda (numbers)
                 (if (null-list? numbers)
                     (exit 1)
                     (leave-on-EPIPE
-                     (for-each list-generation numbers)))))
+                     (list-generation display-profile-content (car numbers))
+                     (diff-profiles profile numbers)))))
              (else
               (leave (_ "invalid syntax: ~a~%")
                      pattern)))
diff --git a/guix/ui.scm b/guix/ui.scm
index eb85df3..306c14c 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2014, 2015 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015, 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2016 Benz Schenk <benz.schenk@uzh.ch>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -87,6 +88,7 @@
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1072,31 @@ DURATION-RELATION with the current time."
           (format #t (_ "~a\t(current)~%") header)
           (format #t "~a~%" header)))))
 
+(define (display-profile-content-diff profile number1 number2)
+  "Display the changed packages in PROFILE NUMBER2 compared to generation NUMBER2."
+
+  (define (equal-entry? first second)
+    (string= (manifest-entry-item first) (manifest-entry-item second)))
+
+  (define (display-entry entry prefix)
+    (match entry
+      (($ <manifest-entry> name version output location _)
+       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+
+  (define (list-entries number)
+    (manifest-entries (profile-manifest (generation-file-name profile number))))
+
+  (define (display-diff profile old new)
+    (display-generation profile new)
+    (let ((added (lset-difference
+                  equal-entry? (list-entries new) (list-entries old)))
+          (removed (lset-difference
+                    equal-entry? (list-entries old) (list-entries new))))
+      (for-each (cut display-entry <> "+") added)
+      (for-each (cut display-entry <> "-") removed)))
+
+  (display-diff profile number1 number2))
+
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-- 
2.10.1