diff mbox

Display diffs between generations.

Message ID 20161021164053.2961bd6d@gondolin.arda
State New
Headers show

Commit Message

Benz Schenk Oct. 21, 2016, 2:40 p.m. UTC 
On Fri, 21 Oct 2016 11:37:00 +0200
Roel Janssen <roel@gnu.org> wrote:

> Ludovic Courtès writes:
> 
> > Roel Janssen <roel@gnu.org> skribis:
> >  
> >> Ludovic Courtès writes:  
> >
> > [...]
> >  
>  [...]  
> >>
> >> Ah, sorry, I forgot about this.  This makes sense.  But then, what should we
> >> see when we do:
> >>
> >>   guix package --list-generations=42,46
> >>
> >> Should we see:
> >>   Generation 42 ...:
> >>   <full contents>
> >>
> >>   Generation 46 ...:
> >>   <diff with 42>  
> >
> > This one, IMO.  
> 
> The attached patch implements this behavior.  However, because I use
> @code{previous-generation-number} to determine which generation to diff
> with, the following will not provide a diff as expected:
> 
>   guix package --list-generations=42,41,40
> 
> Any ideas on how to fix this?  Otherwise I'll give it another go over
> the weekend.
> 
> Kind regards,
> Roel Janssen

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.

HANWE

Benz

Comments

Ludovic Courtès Oct. 24, 2016, 8:33 p.m. UTC | #1 
Hi!

Benz Schenk <benz.schenk@uzh.ch> skribis:

> On Fri, 21 Oct 2016 11:37:00 +0200
> Roel Janssen <roel@gnu.org> wrote:

[...]

>> The attached patch implements this behavior.  However, because I use
>> @code{previous-generation-number} to determine which generation to diff
>> with, the following will not provide a diff as expected:
>> 
>>   guix package --list-generations=42,41,40
>> 
>> Any ideas on how to fix this?  Otherwise I'll give it another go over
>> the weekend.
>> 
>> Kind regards,
>> Roel Janssen
>
> 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.  :-)

Ludo’.
Roel Janssen Oct. 25, 2016, 4:01 p.m. UTC | #2 
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:
>
> [...]
>
>>> The attached patch implements this behavior.  However, because I use
>>> @code{previous-generation-number} to determine which generation to diff
>>> with, the following will not provide a diff as expected:
>>> 
>>>   guix package --list-generations=42,41,40
>>> 
>>> Any ideas on how to fix this?  Otherwise I'll give it another go over
>>> the weekend.
>>> 
>>> Kind regards,
>>> Roel Janssen
>>
>> 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)?


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?

Kind regards,
Roel Janssen
diff mbox

Patch

From f920c91e0572539790952d9c28aec547d7bc0000 Mon Sep 17 00:00:00 2001
From: Benz Schenk <benz.schenk@uzh.ch>
Date: Fri, 21 Oct 2016 16:20:30 +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 | 16 ++++++++++++----
 guix/ui.scm              | 30 ++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index b87aee0..8d148d7 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -667,24 +667,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..c93b44f 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -87,6 +87,7 @@ 
             matching-generations
             display-generation
             display-profile-content
+            display-profile-content-diff
             roll-back*
             switch-to-generation*
             delete-generation*
@@ -1070,6 +1071,35 @@  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 compared to generation NUMBER."
+
+  (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)
+    (format #t (_ "Generation ~a\t~a ~%") new
+                          (date->string
+                           (time-utc->date
+                            (generation-time profile new))
+                           "~b ~d ~Y ~T"))
+    (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