Re: [PATCH] guix package: Add '--delete-generations'.

[ludo@gnu.org (Ludovic Courtès)]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> Can I push this patch to ‘master’?  Do you see any problems?

Looks good!  Minor issues discussed below.

> I had noticed that ‘--roll-back’ doesn’t output anything with
> ‘--dry-run’, so I implemented ‘--delete-generations’ similarly.  Maybe
> it would be better to print something.  WDYT?

Agreed (in a separate patch.)

> +@item --delete-generations[=@var{pattern}]
> +@itemx -d [@var{pattern}]
> +Delete generations.

“Delete the generations matching @var{patterns} or ... when omitted.”

Or what actually?  I would expect it to delete all the generations but
the current one when PATTERN is omitted, right?

> +(define (link-to-empty-environment generation)
> +  "Link GENERATION, a string, to the empty environment."

s/environment/profile/ maybe?  (I know there are other inconsistencies
in these files.)

Ideally this factorization would go in a patch of its own, before the
one that adds --delete-generations.  Is that doable for you?

> +  (define (apply-to-generations function profile pattern)

s/function/proc/ to follow the convention.

> +        (cond ((zero? number))  ; do not delete generation 0
> +              ((and (current-generation? profile generation)
> +                    (not (file-exists? previous-generation)))
> +               (begin (link-to-empty-environment previous-generation)
> +                      (switch-to-previous-generation profile)
> +                      (display-and-delete generation)))
> +              ((current-generation? profile generation)
> +               (begin (roll-back profile)
> +                      (display-and-delete generation)))

No need for ‘begin’ in the body of a ‘cond’ clause.

>      ;; First roll back if asked to.
> -    (if (and (assoc-ref opts 'roll-back?) (not dry-run?))
> -        (begin
> -          (roll-back profile)
> -          (process-actions (alist-delete 'roll-back? opts)))
> -        (let* ((installed (manifest-packages (profile-manifest profile)))
> -               (upgrade-regexps (filter-map (match-lambda
> -                                             (('upgrade . regexp)
> -                                              (make-regexp (or regexp "")))
> -                                             (_ #f))

[...]

Why is there this big hunk?  If it’s just reindenting, could you arrange
to remove this hunk?

Thanks!

Ludo’.