From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id xprnM1KgtmB4FwEAgWs5BA (envelope-from ) for ; Tue, 01 Jun 2021 23:02:10 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id wKDALlKgtmABFQAAB5/wlQ (envelope-from ) for ; Tue, 01 Jun 2021 21:02:10 +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 02B4A228ED for ; Tue, 1 Jun 2021 23:02:10 +0200 (CEST) Received: from localhost ([::1]:55956 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1loBWb-00047l-0Z for larch@yhetil.org; Tue, 01 Jun 2021 17:02:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52468) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1loBWU-00046N-Kd for guix-patches@gnu.org; Tue, 01 Jun 2021 17:02:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:54718) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1loBWU-00004s-Cy for guix-patches@gnu.org; Tue, 01 Jun 2021 17:02:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1loBWU-0001IW-AR for guix-patches@gnu.org; Tue, 01 Jun 2021 17:02:02 -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, 01 Jun 2021 21:02:02 +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.16225812943652 (code B ref 47869); Tue, 01 Jun 2021 21:02:02 +0000 Received: (at 47869) by debbugs.gnu.org; 1 Jun 2021 21:01:34 +0000 Received: from localhost ([127.0.0.1]:38031 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1loBW1-0000wG-Hr for submit@debbugs.gnu.org; Tue, 01 Jun 2021 17:01:34 -0400 Received: from eggs.gnu.org ([209.51.188.92]:58752) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1loBVz-0000pu-Bo for 47869@debbugs.gnu.org; Tue, 01 Jun 2021 17:01:32 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:46982) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1loBVt-0008At-LZ; Tue, 01 Jun 2021 17:01:25 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=57802 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1loBVt-0002al-DC; Tue, 01 Jun 2021 17:01:25 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <0892bdfbc097b07631190c8526a41d57b456d343.camel@telenet.be> <3319cbc48171ae821c3297f9e5cbb8e9011b87ed.camel@telenet.be> Date: Tue, 01 Jun 2021 23:01:22 +0200 In-Reply-To: <3319cbc48171ae821c3297f9e5cbb8e9011b87ed.camel@telenet.be> (Maxime Devos's message of "Tue, 01 Jun 2021 21:53:17 +0200") Message-ID: <87sg21z4d9.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=1622581330; 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=E/M5l32H7RlEVW8lifxzDjop5IVaytqEgSV57Mzs4Go=; b=ZGKsQrlVT3WibJvLYO2w/QKPyF2SMNRTpUQhM2aARqlELruEk9LvWQXJP/4pQ/xFAAl6hD HyakGVwiPf7BY83o1bAwKZLOkWngasusaBFuIp4tg9Sf8qXy/cIyekAGBeClh/dMjgSlrj Dg42YAps9VfNcVf3QeVNDrcb+RAMSJRyBO07/feSrQSa2dZB4j8EW2JKlbNfO+VBs/XtHY xUJU/DPN/peTPR7ERxeJybChWVcqrwursY4y46+x4Kb4zQHIDXjVBJn+nUaIqupF5iF0M4 zi7EzdC+2skXsb4NrVD70/FKVSInpp7xOyKFosRV71JD1WaolV1RPGg+gVT8VQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1622581330; a=rsa-sha256; cv=none; b=aVxUuAj/hlVmxVkx0NXCCEhwY2+9J/yT9UULdCMkvzzYo/aX+HLThkSEIssrW35YzCFz1W zQc2EAbkU9Ua9ScpXsnMfkQyCESlos+GY/89hIdAXZH0HjJpT3L5Xoxr54Ja3gHuNZGJoP /jF9aC1JI9WJ0UED9+DLCngCGSSrC4SNFuEb8xNw5ARUCq2MUWXppNtAOkCvpDa5onBaWs USHsylx8M2vTuLr/MecCm31uGXhCNrgS8jYhtveuaoBES+tgbygvOJlT5zDalej/MAU2nB +u/JZ76WiktwB72h5OvfhZcjVY+QIffNxjFUuPOFQ500CnsE8WBVzTjuyPSSsA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; 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.93 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: 02B4A228ED X-Spam-Score: -2.93 X-Migadu-Scanner: scn1.migadu.com X-TUID: /FPpvA3p7is1 Hi Maxime, Maxime Devos skribis: > Some differences to v2: > > * The #:sh and #:guile arguments are optional. > The default value should be good when compiling natively, > but not when cross-compiling. > > Eventually, we may look into making them required, > but let's pun for later. > > * I left 'wrap-qt-program' alone for now. > > * I left documenting 'wrap-program' and 'wrap-script' for later. > > * I didn't adjust all uses of wrap-program to set #:sh, > only a few. [...] > This patch series is on top of commit 9ba35475ede5eb61bfeead096bc6b73f123= ac891 > on core-updates. Woow, nice! I=E2=80=99ll first focus on the first few patches, those that trigger a wor= ld rebuild. Subsequent patches look good and are less =E2=80=9Ccritical=E2=80= =9D. > From 02d2b52458fae1c391e79f89a89696f3b07fdb2b Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Mon, 31 May 2021 18:22:31 +0200 > Subject: [PATCH 01/18] =3D?UTF-8?q?build:=3D20Allow=3D20overriding=3D20th= e=3D20shell?=3D > =3D?UTF-8?q?=3D20interpreter=3D20in=3D20=3DE2=3D80=3D98wrap-program=3DE2= =3D80=3D99.?=3D > MIME-Version: 1.0 > Content-Type: text/plain; charset=3DUTF-8 > Content-Transfer-Encoding: 8bit > > Previously, when creating new wrappers, 'wrap-program' would search > for an interpreter to use in PATH. However, this is incorrect when > cross-compiling. Allow overriding the shell interpreter to use, > via an optional keyword argument #:sh. > > In time, when all users of 'wrap-program' have been corrected, > this keyword argument can be made mandatory. > > * guix/build/utils.scm (wrap-program): Introduce a #:sh keyword > argument, defaulting to (which "sh"). Use this keyword argument. > > Partially-Fixes: LGTM (will apply together with the other world-rebuild changes). > From f598c0168bfcb75f718cc8edf990b7a560334405 Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Mon, 31 May 2021 18:36:09 +0200 > Subject: [PATCH 02/18] =3D?UTF-8?q?build:=3D20Define=3D20=3DE2=3D80=3D98s= earch-input-f?=3D > =3D?UTF-8?q?ile=3DE2=3D80=3D99=3D20procedure.?=3D > 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. > > When compiling natively (target=3D#f in Guix parlance), > this is perfectly fine. > > However, when cross-compiling, there is a problem. > "which" looks in $PATH for binaries. That's good for purpose (1), > but incorrect for (2), as the $PATH contains binaries from native-inputs > instead of inputs. > > This commit defines a =E2=80=98search-input-file=E2=80=99 procedure. It f= unctions > like 'which', but instead of searching in $PATH, it searches in > the 'inputs' of the build phase, which must be passed to > =E2=80=98search-input-file=E2=80=99 as an argument. Also, the file name m= ust > include "bin/" or "sbin/" as appropriate. > > * guix/build/utils.scm (search-input-file): New procedure. > * tests/build-utils.scm > ("search-input-file: exception if not found") > ("search-input-file: can find if existent"): Test it. > * doc/guix.texi (File Search): Document it. > > Partially-Fixes: I don=E2=80=99t think we need the whole story here :-) though it doesn=E2= =80=99t hurt. =E2=80=98search-input-file=E2=80=99 is useful on its own IMO. > +@deffn {Scheme Procedure} search-input-file @var{inputs} @var{name} > +Return the complete file name for @var{name} as found in @var{inputs}. > +If @var{name} could not be found, an exception is raised instead. > +Here, @var{inputs} is an association list like @var{inputs} and > +@var{native-inputs} as available to build phases. > + > +This procedure can be used for telling @code{wrap-script} and > +@code{wrap-program} (currently undocumented) where the Guile > +binary or shell binary are located. In fact, that's the > +purpose for which @code{search-input-file} has been created > +in the first place. > +@end deffn I=E2=80=99d remove the second paragraph: IMO it=E2=80=99s not the right pla= ce to document the motivation. However, an @lisp example would be nice. BTW, please remember to leave two spaces after end-of-sentence periods. > +(define (search-input-file inputs file) > + "Find a file named FILE among the INPUTS and return its absolute file = name. > + > +FILE must be a string like \"bin/sh\". If FILE is not found, an exceptio= n is > +raised." > + (or (search-path (map cdr inputs) file) > + (error "could not find ~a among the inputs" file))) Rather: (match inputs (((_ . directories) ...) (or (search-path directories file) (raise (condition (&search-error (path directories) (file file))))= ))) =E2=80=A6 so you=E2=80=99d need to define a new error condition type. It=E2=80=99s better to make this extra effort; =E2=80=98error=E2=80=99 thro= ws to 'misc-error and cannot be meaningfully handled by callers. > +(test-assert "search-input-file: exception if not found" > + (not (false-if-exception > + (search-input-file '() "does-not-exist")))) Here you=E2=80=99d use =E2=80=98guard=E2=80=99 to check you got the right e= xception. > From 98856ca64218bd98c0d066a25ac93038a98c7ff5 Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Tue, 1 Jun 2021 21:47:01 +0200 > Subject: [PATCH 03/18] glib-or-gtk-build-system: Look up the interpreter = in > 'inputs'. > > * guix/build/glib-or-gtk-build-system.scm (wrap-all-programs): Pass > the shell interpreter from 'inputs' to 'wrap-program' using > 'search-input-file'. > > Partially-Fixes: [...] > + ;; Do not require bash to be present in the package inputs > + ;; even when there is nothing to wrap. > + ;; Also, calculate (sh) only once to prevent some I/O. > + (define %sh (delay (search-input-file inputs "bin/bash"))) > + (define (sh) (force %sh)) I=E2=80=99d be tempted for clarity to simply write: (define (sh) (search-input-file inputs "bin/bash")) The extra =E2=80=98stat=E2=80=99 calls may be okay in practice but yeah, du= nno. > From bc0085b79dd42e586cc5fcffa6f4972db9f42563 Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Tue, 1 Jun 2021 21:48:44 +0200 > Subject: [PATCH 04/18] python-build-system: Look up the interpreter in > 'inputs'. > > * guix/build/python-build-system.scm (wrap): Pass the shell > interpreter from 'inputs' to 'wrap-program' using 'search-input-file'. > > Partially-Fixes: [...] > From 0370ad982e90c3e4def9cd5245cbd6769fda2830 Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Mon, 31 May 2021 19:20:12 +0200 > Subject: [PATCH 05/18] qt-build-system: Look up the interpreter in 'input= s'. > > * guix/build/qt-build-system.scm (wrap-all-programs): Pass > the shell interpreter from 'inputs' to 'wrap-program' using > 'search-input-file'. > > Partially-Fixes: [...] > From 92278afdc58430e8e9f6887d481964e1d73e551c Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Mon, 31 May 2021 19:21:16 +0200 > Subject: [PATCH 06/18] rakudo-build-system: Look up the interpreter in > 'inputs'. > > * guix/build/rakudo-build-system.scm (wrap): Pass > the shell interpreter from 'inputs' to 'wrap-program' using > 'search-input-file'. > > Partially-Fixes: LGTM! So in the end, I=E2=80=99m suggesting modifications to #2 and the rest LGTM. Thank you! Ludo=E2=80=99.