From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] guix package: Add '--delete-generations'. Date: Sun, 22 Sep 2013 22:55:02 +0200 Message-ID: <878uyo60gp.fsf@gnu.org> References: <87vc2o4qwc.fsf@gnu.org> <87y57kljro.fsf@karetnikov.org> <87hae81uvo.fsf@gnu.org> <87li2oslzh.fsf_-_@karetnikov.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:34912) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VNqlH-0000ix-3q for guix-devel@gnu.org; Sun, 22 Sep 2013 17:00:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VNqlB-00051s-CI for guix-devel@gnu.org; Sun, 22 Sep 2013 17:00:11 -0400 Received: from hera.aquilenet.fr ([141.255.128.1]:52278) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VNqlB-0004yl-2O for guix-devel@gnu.org; Sun, 22 Sep 2013 17:00:05 -0400 In-Reply-To: <87li2oslzh.fsf_-_@karetnikov.org> (Nikita Karetnikov's message of "Sun, 22 Sep 2013 23:19:14 +0400") List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Nikita Karetnikov Cc: guix-devel@gnu.org Nikita Karetnikov skribis: > Can I push this patch to =E2=80=98master=E2=80=99? Do you see any proble= ms? Looks good! Minor issues discussed below. > I had noticed that =E2=80=98--roll-back=E2=80=99 doesn=E2=80=99t output a= nything with > =E2=80=98--dry-run=E2=80=99, so I implemented =E2=80=98--delete-generatio= ns=E2=80=99 similarly. Maybe > it would be better to print something. WDYT? Agreed (in a separate patch.) > +@item --delete-generations[=3D@var{pattern}] > +@itemx -d [@var{pattern}] > +Delete generations. =E2=80=9CDelete the generations matching @var{patterns} or ... when omitted= .=E2=80=9D 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 =E2=80=98begin=E2=80=99 in the body of a =E2=80=98cond=E2=80=99= 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=E2=80=99s just reindenting, could you ar= range to remove this hunk? Thanks! Ludo=E2=80=99.