From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id MEALEf8opGCj/QAAgWs5BA (envelope-from ) for ; Tue, 18 May 2021 22:52:15 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id CJm7DP8opGD1TgAA1q6Kng (envelope-from ) for ; Tue, 18 May 2021 20:52:15 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 7CFE21F2AF for ; Tue, 18 May 2021 22:52:14 +0200 (CEST) Received: from localhost ([::1]:55446 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lj6hJ-0000y9-Cp for larch@yhetil.org; Tue, 18 May 2021 16:52:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52294) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lj6h8-0000vo-1w for guix-patches@gnu.org; Tue, 18 May 2021 16:52:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:45900) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lj6h7-000612-QC for guix-patches@gnu.org; Tue, 18 May 2021 16:52:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lj6h7-00038V-Oi for guix-patches@gnu.org; Tue, 18 May 2021 16:52:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#47869] [PATCH core-updates] =?UTF-8?Q?=E2=80=98which=E2=80=99?= looks in PATH, incorrect when cross-compiling Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 18 May 2021 20:52:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47869 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxime Devos Cc: 47869@debbugs.gnu.org Received: via spool by 47869-submit@debbugs.gnu.org id=B47869.162137107612005 (code B ref 47869); Tue, 18 May 2021 20:52:01 +0000 Received: (at 47869) by debbugs.gnu.org; 18 May 2021 20:51:16 +0000 Received: from localhost ([127.0.0.1]:57446 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lj6gO-00037Y-9E for submit@debbugs.gnu.org; Tue, 18 May 2021 16:51:16 -0400 Received: from eggs.gnu.org ([209.51.188.92]:58400) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lj6gJ-00037I-6F for 47869@debbugs.gnu.org; Tue, 18 May 2021 16:51:14 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:51724) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lj6gC-0005QA-WF; Tue, 18 May 2021 16:51:05 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=37438 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lj6gC-00043M-Ml; Tue, 18 May 2021 16:51:04 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <0892bdfbc097b07631190c8526a41d57b456d343.camel@telenet.be> Date: Tue, 18 May 2021 22:51:02 +0200 In-Reply-To: <0892bdfbc097b07631190c8526a41d57b456d343.camel@telenet.be> (Maxime Devos's message of "Mon, 19 Apr 2021 21:04:40 +0200") Message-ID: <87v97f7oll.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1621371134; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=txagOg7gG1DhaRjx9eT9wt5XnJLLtQaXvyh+a/2SggE=; b=nztxjAofJ/Ns7hOkMsrqBjWp9uNinSbtpSxCPW5mp05AO+umtSsOLXQ2RjmbfxsTnioS/k 9eAv3U6Hc0SxHUL9sHrp2DfjEcRLbuaqrUJmbKi56XpJF5OFvZj2yY9j4niolcAz8173kV hjkDVK7gLgkJmA7WQYnrXKooibCVGCKFhdu/CIqcM4cJhJvM6zhOfmby3D/scROx2B/9nk drLkBAuc9XLEXtv+Vj2ScYu0m8ht2cLOL/1CBj8152b3vpu0surWHZ48qwlf7QYGlkbRt7 WbsfzA3weuKZtB9HC/lCDAsasKqitn1035YeN7LQGVZYUt6rTpJMUd0F2Y8v1A== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1621371134; a=rsa-sha256; cv=none; b=KN5Qlxb6FIYS78CGqxbqSNq5jt6NEvwgh/FyQVwpW8A5FQAjH2vU2TgvEF5E4KX/U7/69z NSw658zlPLRoSgFzAuzBV0O7CpzPcUkoWaHzt2VopJwGK9/y8dfPNWALlNM8AfUbBKdqx7 pCUAaJinAQlsV81+QSEXU2nUhS2nXNs5IFHrWNpGUaCC0TL1GuBkRmplh5CtN10rzSu5g2 K0Df7yyzoq69uuDbu2YEKkXSEErMj9kCxRKpqLSisdYgaQQWlzf4CG/5dUZzv44eljODdx PnvEzrj9eSfIKdcpd2HeNlt3R3pZ7n8JxNY9CgngDvYmvVi2Fev3PTiD01G1dA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Spam-Score: -2.94 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: 7CFE21F2AF X-Spam-Score: -2.94 X-Migadu-Scanner: scn0.migadu.com X-TUID: TvIrmTKd9pEH Hi Maxime, Maxime Devos skribis: > This is version two of the patch series, removing a 'pk' that > I added for debugging, and also fixing 'wrap-script' and 'wrap-program'. > To fix 'wrap-script' and 'wrap-program', I added a required 'inputs' argu= ment. > All callers have been adjusted to pass it. > > Perhaps 'patch-shebang' can be fixed and needs to be fixed, but I don't > have investigated that closely yet. ('patch-shebangs' (with a #\s) works > properly IIUC). Thanks for this long patch series, and sorry for the equally long delay! Since we don=E2=80=99t get to change those interfaces very often, I=E2=80= =99m going to nitpick somewhat because I think we=E2=80=99d rather get them right. > From 42e7cf4ca6e4d6e1cd31c2807f608275a5ca759a Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Sun, 18 Apr 2021 12:45:13 +0200 > Subject: [PATCH 1/7] build: Add argument to which for specifying where to > search. > MIME-Version: 1.0 > Content-Type: text/plain; charset=3DUTF-8 > Content-Transfer-Encoding: 8bit > > The procedure =E2=80=98which=E2=80=99 from (guix build utils) > is used for two different purposes: > > 1. for finding the absolute file name of a binary > that needs to run during the build process > > 2. for finding the absolute file name of a binary, > for the target system (as in --target=3DTARGET), > e.g. for substituting sh->/gnu/store/.../bin/sh, > python->/gnu/store/.../bin/python. But note that only #1 is the intended purpose. > When compiling natively (SYSTEM=3DTARGET modulo nix/autotools differences= ), > this is perfectly fine. Rather =E2=80=9Ctarget=3D#f=E2=80=9D in Guix parlance [...] > +(define* (which program #:optional inputs) > + "Return the complete file name for PROGRAM as found in $PATH, or #fals= e if > +PROGRAM could not be found. If INPUTS is not #false, instead look in the > +/bin and /sbin subdirectories of INPUTS. INPUTS is an alist; its keys > +are ignored." I find that this leads to a weird interface; =E2=80=98which=E2=80=99 is int= ended to be like the same-named shell command, and the notion of =E2=80=9Cinput alists= =E2=80=9D seems out of place to me. I was thinking we could make it: --8<---------------cut here---------------start------------->8--- (define* (which program #:optional (path (search-path-as-string->list (getenv "PATH")))) "Return the complete file name for PROGRAM as found in $PATH, or #f if PROGRAM could not be found." (search-path path program)) --8<---------------cut here---------------end--------------->8--- =E2=80=A6 but that doesn=E2=80=99t buy us much. I think what we need is to do is find and fix misuses of =E2=80=98which=E2= =80=99. WDYT? [...] > +Here is an example using the @code{which} procedure in a build phase: > + > +@lisp > +(lambda* (#:key outputs inputs #:allow-other-keys) > + (let ((growpart (string-append (assoc-ref outputs "out") > + "/bin/growpart"))) > + (wrap-program growpart > + `("PATH" ":" prefix (,(dirname (which "sfdisk" inputs= )) > + ,(dirname (which "readlink" inpu= ts))))))) That looks weird to me. The =E2=80=9Ccorrect=E2=80=9D way we do it right n= ow is like this: (lambda* (#:key outputs inputs #:allow-other-keys) (let ((out (assoc-ref outputs "out")) (curl (assoc-ref inputs "curl"))) (wrap-program (string-append out "/bin/akku") `("LD_LIBRARY_PATH" ":" prefix (,(string-append curl "/lib"))= )) #t)) Here, when cross-compiling, (assoc-ref inputs "curl") returns the target cURL. > From e78d2d8651d5f56afa7d57be78c5cccccebb117a Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Sun, 18 Apr 2021 20:44:28 +0200 > Subject: [PATCH 3/7] build: utils: Make inputs of 'wrap-script' explicit. > > Previously, 'wrap-script' used (which "guile") to determine where to loca= te > the guile interpreter. But this is incorrect when cross-compiling. When > cross-compiling, this would locate the (native) guile interpreter that is > in the PATH, while a guile interpreter for the target is required. > > Remove the optional #:guile argument which is only used in tests and repl= ace > it with a required 'inputs' argument and adjust all callers. Write a new > test verifying a guile for the target is located, instead of a native gui= le. I think the #:guile argument was a fine interface: clear and to-the-point. The problem IMO is just that it=E2=80=99s not use where it should. :-) [...] > --- a/gnu/packages/audio.scm > +++ b/gnu/packages/audio.scm > @@ -4712,9 +4712,9 @@ as is the case with audio plugins.") > (chmod (string-append out "/share/carla/carla") #o555) > #t))) > (add-after 'install 'wrap-executables > - (lambda* (#:key outputs #:allow-other-keys) > + (lambda* (#:key inputs outputs #:allow-other-keys) > (let ((out (assoc-ref outputs "out"))) > - (wrap-script (string-append out "/bin/carla") > + (wrap-script (string-append out "/bin/carla") inputs > `("GUIX_PYTHONPATH" ":" prefix (,(getenv "GU= IX_PYTHONPATH")))) This would become: (wrap-script (string-append out "/bin/carla") `(=E2=80=A6) #:guile (assoc-ref inputs "guile")) WDYT? > From 8b843f0dd8803120718747b480983bd5888b1617 Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Mon, 19 Apr 2021 16:56:00 +0200 > Subject: [PATCH 6/7] build: utils: wrap-program: look up bash in inputs, = not > in PATH > > 'wrap-program' is almost always used for creating wrappers for the > target system. It is only rarely (once) used for creating wrappers for > the native system. However, 'wrap-program' always creates wrappers for > the native system and provides no option for creating wrappers for the > target system instead. [...] > - (wrap-program > - (string-append libexec "/dhclient-script") > + (wrap-program (string-append libexec "/dhclient-script") > + inputs > `("PATH" ":" prefix > ,(map (lambda (dir) > (string-append dir "/bin:" I=E2=80=99m also skeptical here; =E2=80=98wrap-program=E2=80=99 needs to kn= ow the file name of =E2=80=98sh=E2=80=99 and instead we=E2=80=99re passing it the full input li= st. I would instead add #:bash (or #:sh?). The downside is that it=E2=80=99d b= e a bit more verbose, but in terms of interfaces, I=E2=80=99d find it clearer: (wrap-program (string-append libexec "/dhclient-script") `("PATH" =E2=80=A6) #:sh (string-append (assoc-ref inputs "bash") "/bin/sh")) We could introduce a helper procedure to replace (string-append =E2=80=A6) = with: (search-input-file inputs "/bin/sh") where: (define (search-input-file inputs file) (any (match-lambda ((label . directory) (let ((file (string-append directory "/" file))) (and (file-exists? file) file)))) inputs)) WDYT? > + (wrap-program (string-append out "/bin/screenfetch") > + %build-inputs As a rule of thumb we should refer to #:inputs and #:outputs instead of the global variables =E2=80=98%build-inputs=E2=80=99 etc. > From cdd45bc0aef8b6cb60d351a8fded18700804e8db Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Mon, 19 Apr 2021 19:54:53 +0200 > Subject: [PATCH 7/7] doc: Document 'wrap-program'. > > * doc/guix.texi (Wrapping Code)[wrap-program]: Copy docstring from > guix/build/utils.scm and use Texinfo markup. Neat! > doc/guix.texi | 37 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/doc/guix.texi b/doc/guix.texi > index a2ff13fe0f..6235ae9bf7 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -8703,7 +8703,42 @@ Here is an example using the @code{which} procedur= e in a build phase: > This section documents procedures that create =E2=80=98wrappers=E2=80=99= around existing > binaries, that e.g. set environment variables required during execution. >=20=20 > -@c TODO document wrap-program > +@deffn {Scheme Procedure} wrap-program @var{prog} @var{inputs} @var{vars} > +Make a wrapper for @var{prog}. @var{vars} should look like this: > + > +@lisp > + '(VARIABLE DELIMITER POSITION LIST-OF-DIRECTORIES) ^ You can remove indentation and use @var instead of capital letters. > +@lisp > + (wrap-program "foo" > + '("PATH" ":" =3D ("/gnu/.../bar/bin")) > + '("CERT_PATH" suffix ("/gnu/.../baz/certs" > + "/qux/certs"))) ^^ You can remove indentation here too. > +@end lisp > + > +will copy @file{foo} to @file{.foo-real} and create the file @file{foo} = with > +the following contents: > + > +@example > + #!location/of/bin/bash > + export PATH=3D"/gnu/.../bar/bin" > + export CERT_PATH=3D"$CERT_PATH$@{CERT_PATH:+:@}/gnu/.../baz/certs:/qux= /certs" > + exec -a $0 location/of/.foo-real "$@@" =E2=80=A6 and here. This one can even go to master. Thanks! Ludo=E2=80=99.