From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: guix-package --roll-back Date: Thu, 10 Jan 2013 23:26:58 +0100 Message-ID: <8738y8ekst.fsf@gnu.org> References: <871uejyq9z.fsf@karetnikov.org> <874nj4sbfe.fsf@karetnikov.org> <87y5gf8sm1.fsf@gnu.org> <87hamy4yaj.fsf@karetnikov.org> <87pq1m5nxy.fsf@gnu.org> <87obh43j7r.fsf@karetnikov.org> <87vcbbqvw1.fsf@gnu.org> <87fw2ai3e1.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 ([208.118.235.92]:48579) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TtQaW-0001Pr-9A for bug-guix@gnu.org; Thu, 10 Jan 2013 17:27:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TtQaT-00039C-UC for bug-guix@gnu.org; Thu, 10 Jan 2013 17:27:04 -0500 Received: from mail4-relais-sop.national.inria.fr ([192.134.164.105]:32915) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TtQaT-00038v-KE for bug-guix@gnu.org; Thu, 10 Jan 2013 17:27:01 -0500 In-Reply-To: <87fw2ai3e1.fsf@karetnikov.org> (Nikita Karetnikov's message of "Wed, 09 Jan 2013 14:04:37 -0500") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org To: Nikita Karetnikov Cc: bug-guix@gnu.org Hi, Nikita Karetnikov skribis: > I'm attaching a slightly modified version. Thanks! >> Yeah, that=E2=80=99s expected. Basically, if you do > >> guix-package -p /dev/null --roll-back > >> it should fail with an error message saying that there is no previous >> profile or something like that. > > 'profile-number' will fail if I call it with a bogus file name. > > For instance: > > scheme@(guile-user)> (profile-number "/foo/bar") > ERROR: In procedure readlink: > ERROR: In procedure readlink: No such file or directory > > What should I use to handle this error? Please show an example. The > ones from the manual aren't helpful. At the command-line level, an example would be: $ guix-package -p /dev/null --roll-back ; echo $? error: no previous profile to roll back to 1 (Please, add this example to tests/guix-package.sh.) Internally, I would expect this =E2=80=98profile-number=E2=80=99 to return = #f. > I don't understand how to add a command-line option that should accept > an optional argument. I commented out my attempts. (option '("foo") #f ; no required argument #t ; an optional argument (lambda (opt name arg result) ;; Function called when =E2=80=98--foo=E2=80=99 is passed. ;; ARG is #f or the optional argument. ;; ... )) See SRFI-37 in Guile=E2=80=99s manual. > +(define (profile-rx profile) Perhaps =E2=80=98profile-regexp=E2=80=99 (since it=E2=80=99s a global varia= ble.) > +(define (profile-number profile) > + "Return PROFILE's number. An absolute file name must be used." I think it works even if PROFILE is not an absolute file name, no? Just like =E2=80=98latest-profile-number=E2=80=99 returns 0 if there was no= file matching %PROFILE-RX, =E2=80=98profile-number=E2=80=99 should return 0. So you=E2=80=99d want to enclose the whole body like this: (or (and=3D> ...) 0) > + (and=3D> (regexp-exec (profile-rx profile) > + (basename (readlink profile))) > + (cut match:substring <> 1))) First, you need to move =E2=80=98readlink=E2=80=99 above, in =E2=80=98false= -if-exception=E2=80=99. Second, it should return a number, not a string. So the result should be along these lines: (let ((target (false-if-exception (readlink profile)))) (and target (and=3D> (regexp-exec (profile-regexp profile) (basename target)) (compose string->number (cut match:substring <> 1)) > +(define* (roll-back #:optional profile) Make =E2=80=98profile=E2=80=99 mandatory. > + "Roll back to the previous profile." Rather: =E2=80=9CRoll back to the previous generation of PROFILE.=E2=80=9D > + (let* ((current-profile-number > + (string->number (profile-number (or profile %current-profile))= )) The =E2=80=98or=E2=80=99 and =E2=80=98string->number=E2=80=99 can now be re= moved. Name the variable just =E2=80=98number=E2=80=99, because long names for loc= al vars hinder clarity. Now you need to introduce an (if number ;; then do the thing (format (current-error-port) (_ "error: =E2=80=98~a=E2=80=99 is not a= valid profile~%") profile)) > + (previous-profile-number (number->string (1- current-profile-nu= mber))) > + (previous-profile > + (string-append (or profile %current-profile) "-" > + previous-profile-number "-link")) > + (manifest (string-append previous-profile "/manifest"))) > + > + (define (switch-link) > + ;; Switch to the previous generation. > + (let ((tmp-profile (string-append (dirname (or profile %current-pr= ofile)) > + "/tmp-" > + (basename previous-profile)))) > + > + (simple-format #t "guix-package: switching from generation ~a to= ~a~%" > + current-profile-number previous-profile-number) > + (symlink previous-profile tmp-profile) > + (rename-file tmp-profile (or profile %current-profile)))) Looks good. However, just use =E2=80=98format=E2=80=99, and also make sure that message= s and i18n=E2=80=99d: (format #t (_ "switching from generation ~a to ~a...~%") number previous-number) > + (if (equal? (map (cut file-exists? <>) > + (list previous-profile manifest)) > + '(#t #t)) I think you just need to check for =E2=80=98previous-profile=E2=80=99. If = it turns out not to be a genuine profile, that=E2=80=99s not our problem. Besides, this is a mundane (and inefficient) way of writing: (every file-exists? (list previous-profile manifest)) or simply: (and (file-exists? previous-profile) (file-exists? manifest)) :-) > + (switch-link) > + (leave (_ (string-append > + "guix-package: previous profile doesn't exist; " > + "not rolling back~%")))))) Strip =E2=80=9Cguix-package: =E2=80=9D. Woow, this was long. Thanks, Ludo=E2=80=99.