From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] utils: Allow wrap-program to be called multiple times. Date: Thu, 11 Sep 2014 15:10:26 +0200 Message-ID: <8738byti9p.fsf@gnu.org> References: <871trk2yis.fsf@member.fsf.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]:49222) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS49v-00076u-8T for guix-devel@gnu.org; Thu, 11 Sep 2014 09:12:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XS48q-0005Ml-VX for guix-devel@gnu.org; Thu, 11 Sep 2014 09:11:35 -0400 Received: from hera.aquilenet.fr ([2a01:474::1]:38338) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS48q-0005LY-Co for guix-devel@gnu.org; Thu, 11 Sep 2014 09:10:28 -0400 In-Reply-To: <871trk2yis.fsf@member.fsf.org> (Eric Bavier's message of "Tue, 09 Sep 2014 17:56:59 -0500") List-Id: "Development of GNU Guix and the GNU System distribution." 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: Eric Bavier Cc: guix-devel@gnu.org Eric Bavier skribis: > Currently, if (@ (guix build utils) wrap-program) is called multiple > times for the same file, the original file ends up being overwritten. OK, you=E2=80=99ve convinced me that this improvement is welcome. Some comments on the patch: > From 231130db4444685d8f3264e61d680634eaead9fb Mon Sep 17 00:00:00 2001 > From: Eric Bavier > Date: Tue, 9 Sep 2014 17:47:31 -0500 > Subject: [PATCH] utils: Allow wrap-program to be called multiple times. > > * guix/build/utils.scm (wrap-program): Multiple invocations of > wrap-program for the same file create successive wrappers. > --- > guix/build/utils.scm | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/guix/build/utils.scm b/guix/build/utils.scm > index 2f3dc9c..d4435b4 100644 > --- a/guix/build/utils.scm > +++ b/guix/build/utils.scm > @@ -711,8 +711,24 @@ contents: > This is useful for scripts that expect particular programs to be in $PAT= H, for > programs that expect particular shared libraries to be in $LD_LIBRARY_PA= TH, or > modules in $GUILE_LOAD_PATH, etc." > - (let ((prog-real (string-append (dirname prog) "/." (basename prog) "-= real")) > - (prog-tmp (string-append (dirname prog) "/." (basename prog) "-= tmp"))) > + (define (wrapper-path num) > + (format #f "~a/.~a-wrap-~2'0d" (dirname prog) (basename prog) num)) Make it =E2=80=98wrapper-file-name=E2=80=99 (in GNU, =E2=80=9Cpath=E2=80=9D= means =E2=80=9Csearch path=E2=80=9D.) > + (let* ((current-wrappers > + (find-files (dirname prog) > + (string-append "\\." (basename prog) "-wrap-.*"))) > + (wrapper-num (if (null? current-wrappers) > + 0 > + (string->number > + (string-take-right (last current-wrappers) 2)= ))) These two could be factorized as a local =E2=80=98next-wrapper-number=E2=80= =99 procedure. For local variables, it=E2=80=99s better to use shorter names, such as =E2=80=98wrappers=E2=80=99 and =E2=80=98number=E2=80=99 here. > + (wrapper-tgt (if (zero? wrapper-num) > + (let ((prog-real (string-append > + (dirname prog) "/." > + (basename prog) "-real"))) > + (copy-file prog prog-real) > + prog-real) > + (wrapper-path wrapper-num))) Make it a =E2=80=98wrapper-target=E2=80=99 local procedure, and change =E2= =80=98wrapper-tgt=E2=80=99 to =E2=80=98target=E2=80=99. It looks OK. It would be ideal if a test in tests/build-utils.scm made sure that =E2=80=98wrap-program=E2=80=99 uses the right file names when cal= led multiple times, but I won=E2=80=99t object if the patch doesn=E2=80=99t have it. Thanks, Ludo=E2=80=99.