From unknown Fri Aug 08 22:15:29 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#64359 <64359@debbugs.gnu.org> To: bug#64359 <64359@debbugs.gnu.org> Subject: Status: [PATCH] [RFC] --search-paths: emit code compatible with set -u Reply-To: bug#64359 <64359@debbugs.gnu.org> Date: Sat, 09 Aug 2025 05:15:29 +0000 retitle 64359 [PATCH] [RFC] --search-paths: emit code compatible with set -u reassign 64359 guix-patches submitter 64359 Zack Weinberg severity 64359 normal tag 64359 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Thu Jun 29 19:11:10 2023 Received: (at submit) by debbugs.gnu.org; 29 Jun 2023 23:11:10 +0000 Received: from localhost ([127.0.0.1]:54250 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qF0n7-0001l8-CS for submit@debbugs.gnu.org; Thu, 29 Jun 2023 19:11:10 -0400 Received: from lists.gnu.org ([209.51.188.17]:38918) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qF0n4-0001kz-Sz for submit@debbugs.gnu.org; Thu, 29 Jun 2023 19:11:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qF0n4-00011G-F7 for guix-patches@gnu.org; Thu, 29 Jun 2023 19:11:06 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qF0n0-0001Ue-3Q for guix-patches@gnu.org; Thu, 29 Jun 2023 19:11:06 -0400 Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id BF2FC5C0378 for ; Thu, 29 Jun 2023 19:10:57 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Thu, 29 Jun 2023 19:10:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=owlfolio.org; h= cc:content-type:content-type:date:date:from:from:in-reply-to :message-id:mime-version:reply-to:sender:subject:subject:to:to; s=fm1; t=1688080257; x=1688166657; bh=qXjZT+79rx6hcGLIz7GFjmF+8 AoRoJn7ndSEYUr1gcY=; b=oRvMk/L4hYugs9fZKYZpxrDPQryeEjVGrtf/sG9J/ mnVolamMHYnpq4oWerG0L8O2X6ff5Pp0eU5kZIRRUYiJPnaJTyfB272TsZqzF9LF 1uP8WFPC9vS8EjRSHbhIf5YGpPV7sr2X/bVzq9JX5h06PAYOl9JPYN4uic66w1RW a808RdO0/sRCjj7Y1ckuyTMmXFiVqBV3qfDolnH8xAH9YK3OMjLaCKEROjdJqutR tkps/tkqLKcoi9PyHdHYp3jDz7/bhDikcVNZJuvLHdEu+LRZ3r4ZgmgfExdRKiZ4 XKRd5vJBtsCt6CRdqzOYemEkWuJSxIe4S/d7M0TQM5O5A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:message-id :mime-version:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1688080257; x=1688166657; bh=qXjZT+79rx6hcGLIz7GFjmF+8AoRoJn7ndS EYUr1gcY=; b=IGw+gOQV6qsp22vrOWyD10Wi9SkXdjralL+WBujj8fRwxIPjswk 6UZldr/mb9n5bCznG4idQIQUxdW4Il2TVqBzlQvzrWKLTIND2/p+QzBDL3RdGjZ+ jKxovj8EAKP1H9t2Aaz2MIFqIw341hQvF10slL3T7cqCWnN+2U9EJbNhujrgNE/0 p0ZoyMZEJW/Ez+QjaHPrvdH1jUYezEXpt9JTYT24i9mtbyzBnMZElsyvxnGl3tSn ImXZflOGlwv4AUdB3+pTO4YPOD032DHiG7rnhYbR75KDv7fT31d6PpVmKZAZtSPf +f7D+Ct/e3Leyure5Pq/Relu0mJNl2dEQpw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrtdehgddvtdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecuogflrghprghnqddqlffrqdfuuhhsphgvtghtrfgrth htvghrnhdqjfdtgeculddvtddmnecujfgurhepfffkhffvufgfgggtsehttdertddtrefg necuhfhrohhmpegkrggtkhcuhggvihhnsggvrhhguceoiigrtghksehofihlfhholhhioh drohhrgheqnecuggftrfgrthhtvghrnhepheeuueeikeejhfeftdfhhfeuiedtleejleet heeiveeihefhuefhleejkeevheevnecuffhomhgrihhnpeguihhrvghnvhdrnhgvthenuc evlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpeiirggtkhes ohiflhhfohhlihhordhorhhg X-ME-Proxy: Feedback-ID: i876146a2:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Thu, 29 Jun 2023 19:10:57 -0400 (EDT) Date: Thu, 29 Jun 2023 19:10:56 -0400 Message-ID: <877crlyidb.wl-zack@owlfolio.org> From: Zack Weinberg To: guix-patches@gnu.org Subject: [PATCH] [RFC] --search-paths: emit code compatible with set -u User-Agent: Wanderlust/2.15.9 (Almost Unreal) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-2022-JP Received-SPF: pass client-ip=66.111.4.25; envelope-from=zack@owlfolio.org; helo=out1-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.6 (-) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.6 (--) The shell script fragment emitted by the --search-paths option to ‘guix shell’, etc., uses this construct to prepend a directory to a search-path environment variable: export VAR="/gnu/store/...${VAR:+:}$VAR" If this is evaluated by a shell in set -u mode, and VAR was previously unset, it will error out: $ bash -c ' set -u unset PKG_CONFIG_PATH export PKG_CONFIG_PATH="/example/lib/pkgconfig${PKG_CONFIG_PATH:+:}$PKG_CONFIG_PATH" echo $PKG_CONFIG_PATH ' bash: line 4: PKG_CONFIG_PATH: unbound variable This happens in real life, for instance, if direnv[1] is being used in its “strict_env” mode[2], which the documentation says will become the default in a future release. This patch makes the code emitted by --search-paths compatible with set -u, by changing each use of bare `$VARIABLE` to `${VARIABLE:-}`, e.g. $ bash -c ' set -u unset PKG_CONFIG_PATH export PKG_CONFIG_PATH="/example/lib/pkgconfig${PKG_CONFIG_PATH:+:}${PKG_CONFIG_PATH:-}" echo $PKG_CONFIG_PATH' /example/lib/pkgconfig (Note: `${VARIABLE-}` would also be correct here. When there are no characters between the `-` and the `}`, `${VARIABLE:-}` and `${VARIABLE-}` do exactly the same thing. However, `${VARIABLE:+:}` and `${VARIABLE+:}` are *not* equivalent, and the former is what we want. Therefore I think the code is easier to understand if we consistently use the : variant of both substitution operators.) For consistency I also made the same change to the code generated by 'wrap-program'. (There's an argument for adding `set -euo pipefail` to the top of those scripts, but that should probably be a separate patch.) I tried to write tests to verify this new constraint, i.e. that the code emitted by --search-paths works correctly in set -u mode. The test suite got stuck for upwards of six hours, apparently on tests that I didn’t touch. I _suspect_ this is a problem with my development machine, which is a botched chimera of Guix and Debian at the moment, but I’d appreciate it if someone could look carefully at the test code. The code change itself is straightforward. Also, as far as I can tell, there is no existing package that causes environment-variable-definition to be invoked with #:kind 'suffix, and in fact I’m not sure it’s *possible* to write a package definition that does that. So that case is currently not being tested. [1]: https://direnv.net/ [2]: https://direnv.net/man/direnv.toml.1.html#codestrictenvcode --- guix/build/utils.scm | 8 ++++---- guix/search-paths.scm | 4 ++-- tests/guix-environment.sh | 21 +++++++++++++++++++++ tests/guix-shell.sh | 20 ++++++++++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/guix/build/utils.scm b/guix/build/utils.scm index 2352a627e9..f2a18d698d 100644 --- a/guix/build/utils.scm +++ b/guix/build/utils.scm @@ -1384,19 +1384,19 @@ (define (export-variable lst) (format #f "export ~a=\"~a\"" var (string-join rest sep))) ((var sep 'prefix rest) - (format #f "export ~a=\"~a${~a:+~a}$~a\"" + (format #f "export ~a=\"~a${~a:+~a}${~a:-}\"" var (string-join rest sep) var sep var)) ((var sep 'suffix rest) - (format #f "export ~a=\"$~a${~a+~a}~a\"" + (format #f "export ~a=\"${~a:-}${~a:+~a}~a\"" var var var sep (string-join rest sep))) ((var '= rest) (format #f "export ~a=\"~a\"" var (string-join rest ":"))) ((var 'prefix rest) - (format #f "export ~a=\"~a${~a:+:}$~a\"" + (format #f "export ~a=\"~a${~a:+:}${~a:-}\"" var (string-join rest ":") var var)) ((var 'suffix rest) - (format #f "export ~a=\"$~a${~a:+:}~a\"" + (format #f "export ~a=\"${~a:-}${~a:+:}~a\"" var var var (string-join rest ":"))))) (when (wrapped-program? prog) diff --git a/guix/search-paths.scm b/guix/search-paths.scm index fcbe7b7953..13c96ad692 100644 --- a/guix/search-paths.scm +++ b/guix/search-paths.scm @@ -225,10 +225,10 @@ (define* (environment-variable-definition variable value ('exact (format #f "export ~a=\"~a\"" variable value)) ('prefix - (format #f "export ~a=\"~a${~a:+~a}$~a\"" + (format #f "export ~a=\"~a${~a:+~a}${~a:-}\"" variable value variable separator variable)) ('suffix - (format #f "export ~a=\"$~a${~a:+~a}~a\"" + (format #f "export ~a=\"${~a:-}${~a:+~a}~a\"" variable variable variable separator value)))) (define* (search-path-definition search-path value diff --git a/tests/guix-environment.sh b/tests/guix-environment.sh index 1424ea9a88..d0ffcd91f3 100644 --- a/tests/guix-environment.sh +++ b/tests/guix-environment.sh @@ -157,6 +157,27 @@ esac # in its profile (e.g., for 'gzip'), but we have to accept them. guix environment guix --bootstrap -n +# The code emitted by '--search-paths', when adding to either end of a path +# variable that might not be set to begin with, should not cause errors, +# whether or not the variable was set, even if the shell is in `set -u` mode. +# As far as I can tell, there are currently no packages that add to the end +# of a path variable, so we cannot test that case. +printf '%s\n' \ + 'set -u' \ + 'set -e' \ + 'unset PKG_CONFIG_PATH' \ + 'export PATH=/nonexistent' \ + > "$tmpdir/c" +guix environment guix --bootstrap -n --search-paths >> "$tmpdir/c" +printf '%s\n' \ + 'echo "PATH=$PATH"' \ + 'echo "PKG_CONFIG_PATH=$PKG_CONFIG_PATH"' \ + >> "$tmpdir/c" +bash -x "$tmpdir/c" > "$tmpdir/d" +grep -E '^PKG_CONFIG_PATH=/gnu/[^:]+$' "$tmpdir/d" +grep -E '^PATH=/gnu/[^:]+:/nonexistent$' "$tmpdir/d" + + # Try program transformation options. mkdir "$tmpdir/emacs-36.8" drv="`guix environment --ad-hoc emacs -n 2>&1 | grep 'emacs.*\.drv'`" diff --git a/tests/guix-shell.sh b/tests/guix-shell.sh index ed368515eb..ec30a06288 100644 --- a/tests/guix-shell.sh +++ b/tests/guix-shell.sh @@ -41,6 +41,26 @@ guix shell --ad-hoc guile-bootstrap && false # Rejecting unsupported packages. guix shell -s armhf-linux intelmetool -n && false +# The code emitted by '--search-paths', when adding to either end of a path +# variable that might not be set to begin with, should not cause errors, +# whether or not the variable was set, even if the shell is in `set -u` mode. +# As far as I can tell, there are currently no packages that add to the end +# of a path variable, so we cannot test that case. +printf '%s\n' \ + 'set -u' \ + 'set -e' \ + 'unset PKG_CONFIG_PATH' \ + 'export PATH=/nonexistent' \ + > "$tmpdir/c" +guix shell -D guix --bootstrap -n --search-paths >> "$tmpdir/c" +printf '%s\n' \ + 'echo "PATH=$PATH"' \ + 'echo "PKG_CONFIG_PATH=$PKG_CONFIG_PATH"' \ + >> "$tmpdir/c" +bash -x "$tmpdir/c" > "$tmpdir/d" +grep -E '^PKG_CONFIG_PATH=/gnu/[^:]+$' "$tmpdir/d" +grep -E '^PATH=/gnu/[^:]+:/nonexistent$' "$tmpdir/d" + # Test approximately that the child process does not inherit extra file # descriptors. Ideally we'd check there's nothing more than 0, 1, and 2, but # we cannot do that because (1) we might be inheriting additional FDs, for -- 2.40.1 From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 11 13:07:09 2023 Received: (at 64359) by debbugs.gnu.org; 11 Oct 2023 17:07:09 +0000 Received: from localhost ([127.0.0.1]:39936 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qqcfs-0000Lo-JR for submit@debbugs.gnu.org; Wed, 11 Oct 2023 13:07:08 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59124) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qqcfn-0000LC-ES for 64359@debbugs.gnu.org; Wed, 11 Oct 2023 13:07:06 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqcfL-00045Q-Sk; Wed, 11 Oct 2023 13:06:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:References:In-Reply-To:Subject:To: From; bh=gIKwIr2ZMBae0yOJYKGlaqYOHmxwYiSnyfG1P3QTEmI=; b=mxZ1F/xKFSe28QJxHvpl GZLXjkW4GU9GZO1/bOdrd87J/rI7WKT1f60ftTXsDatRYb36R/rW3jY1z3Ap7XPBatv681gQVoYFo R+cPuvzacJN2+r9HjE0D0awr2hzSeE1vx5G9fqqgh9CuKTbrBxIhW8JuGTLIYfMJKg7t3fT27lEYj KhFHtiSVxA/qNl//Wu29wVVYkr8YRmWGbvNCW4BxJCwK7ajTkdsdYdlPwwySn5RT4mchtdpbZ+0iU G/AmD3gTt0DWXdIV5XG6ZFSqRSWI2SBfRs+wgN+lCuxNwVp8R2EgKmln+kSfknjfqyeR+GNq7JbtU JI8NnfYcGf/f6Q==; From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Zack Weinberg Subject: Re: [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u In-Reply-To: <877crlyidb.wl-zack@owlfolio.org> (Zack Weinberg's message of "Thu, 29 Jun 2023 19:10:56 -0400") References: <877crlyidb.wl-zack@owlfolio.org> Date: Wed, 11 Oct 2023 19:06:32 +0200 Message-ID: <87bkd5t7fr.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 64359 Cc: 64359@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi Zack, Zack Weinberg skribis: > The shell script fragment emitted by the --search-paths option to > =E2=80=98guix shell=E2=80=99, etc., uses this construct to prepend a dire= ctory to > a search-path environment variable: > > export VAR=3D"/gnu/store/...${VAR:+:}$VAR" > > If this is evaluated by a shell in set -u mode, and VAR was previously > unset, it will error out: > > $ bash -c ' > set -u > unset PKG_CONFIG_PATH > export PKG_CONFIG_PATH=3D"/example/lib/pkgconfig${PKG_CONFIG_PATH= :+:}$PKG_CONFIG_PATH" > echo $PKG_CONFIG_PATH > ' > bash: line 4: PKG_CONFIG_PATH: unbound variable > > This happens in real life, for instance, if direnv[1] is being used in its > =E2=80=9Cstrict_env=E2=80=9D mode[2], which the documentation says will b= ecome the default > in a future release. > > This patch makes the code emitted by --search-paths compatible with set -= u, > by changing each use of bare `$VARIABLE` to `${VARIABLE:-}`, e.g. > > $ bash -c ' > set -u > unset PKG_CONFIG_PATH > export PKG_CONFIG_PATH=3D"/example/lib/pkgconfig${PKG_CONFIG_PATH= :+:}${PKG_CONFIG_PATH:-}" > echo $PKG_CONFIG_PATH' > /example/lib/pkgconfig This change makes a lot of sense to me. [...] > +++ b/guix/build/utils.scm > @@ -1384,19 +1384,19 @@ (define (export-variable lst) > (format #f "export ~a=3D\"~a\"" > var (string-join rest sep))) > ((var sep 'prefix rest) > - (format #f "export ~a=3D\"~a${~a:+~a}$~a\"" > + (format #f "export ~a=3D\"~a${~a:+~a}${~a:-}\"" > var (string-join rest sep) var sep var)) This part is a full-rebuild change, so it=E2=80=99d have to wait. However,= it=E2=80=99s within =E2=80=98wrap-program=E2=80=99; the script generated by =E2=80=98wra= p-program=E2=80=99 does *not* use =E2=80=98set -u=E2=80=99, so I think this change is unnecessary. Am I = right? > +++ b/guix/search-paths.scm > @@ -225,10 +225,10 @@ (define* (environment-variable-definition variable = value > ('exact > (format #f "export ~a=3D\"~a\"" variable value)) > ('prefix > - (format #f "export ~a=3D\"~a${~a:+~a}$~a\"" > + (format #f "export ~a=3D\"~a${~a:+~a}${~a:-}\"" > variable value variable separator variable)) > ('suffix > - (format #f "export ~a=3D\"$~a${~a:+~a}~a\"" > + (format #f "export ~a=3D\"${~a:-}${~a:+~a}~a\"" > variable variable variable separator value)))) LGTM. > --- a/tests/guix-environment.sh > +++ b/tests/guix-environment.sh You can remove this change and keep only the =E2=80=98tests/guix-shell.sh= =E2=80=99 part. Could you send an updated patch? Thanks, and apologies for the long delay! Ludo=E2=80=99. From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 11 14:57:50 2023 Received: (at 64359) by debbugs.gnu.org; 11 Oct 2023 18:57:51 +0000 Received: from localhost ([127.0.0.1]:40078 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qqeP0-0001OR-HN for submit@debbugs.gnu.org; Wed, 11 Oct 2023 14:57:50 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:50441) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qqeOv-0001O2-5s for 64359@debbugs.gnu.org; Wed, 11 Oct 2023 14:57:48 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 13EC15C0298; Wed, 11 Oct 2023 14:57:17 -0400 (EDT) Received: from imap45 ([10.202.2.95]) by compute5.internal (MEProxy); Wed, 11 Oct 2023 14:57:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=owlfolio.org; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm1; t= 1697050637; x=1697137037; bh=OiUGeGy0KjtupERpLhE2HKvJIELuo4H2NUm e5WzTk6I=; b=S4hZzARqasHWYA587UwtG+ylXKrLvWV8Af23tCAVkcu3jYwcWyk CJeyTIWbCzY6ns/YCRq/uOVkCaMJp90AhloAIRrOPWqxzMcjghmXdeI1/ccJQtim h6Ct5t9uspiLDATDDDn5Z2j8fD6RdM4yNau6wOlpDOqvHTL+HY+gxjevCAbPn8f9 0oqcJJDurz9bNcyCz/Lt2JOt6604ux+3eZSrGgM06U1EyQveheqwVel1WaQvIWVX L5Q31MO4XbOqA9e7s0SZLQilbbkgaI78gDYqpGFDmF6yyAbyebr+2b+DbBe0ZLu5 N+TnWjEhVRHmzgdPThZl9h6abP4JszQ8NHw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1697050637; x=1697137037; bh=OiUGeGy0KjtupERpLhE2HKvJIELuo4H2NUm e5WzTk6I=; b=TbPqvn2+Yl/S5U3L7eMgMzy2oo2EYBFspzyrA3YFj1P5xt+vUKn tv6NrcyxtlxPpa+bNJi2Wx6eskoaj8MGGh52hdg7x3B8sA0ZWgH/+r7SXEmEMSHC /jjYEHQyaMaLi4VDRm2MHqJWqwiPF2+95fk7PV8CfbqOC5u+qM+FHsh1G2UFe8Wh 3cMiGEkmShwUIPpiDvY2E5baCWBZLAZ2QcyGO53BgHpqXFwfqYbmCIjsFkTUd/15 pJNy1uAkxLKkfShZX5T8cooWMxvgRc5wG47HpFs3OfLd+jW1cUopQtsbdKsOWOS/ 0OgeGFGYQjMyzA8shY5K6k1sC8d3BxnBMog== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrheekgdduvdekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvfevufgtgfesthhqredtreerjeenucfhrhhomhepfdgk rggtkhcuhggvihhnsggvrhhgfdcuoeiirggtkhesohiflhhfohhlihhordhorhhgqeenuc ggtffrrghtthgvrhhnpeduueeigeehffekiefhtdehiedvueffteevtefhudfguedtueei tdetgfetieeiieenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpeiirggtkhesohiflhhfohhlihhordhorhhg X-ME-Proxy: Feedback-ID: i876146a2:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id A99FD272007B; Wed, 11 Oct 2023 14:57:16 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-1019-ged83ad8595-fm-20231002.001-ged83ad85 MIME-Version: 1.0 Message-Id: In-Reply-To: <87bkd5t7fr.fsf@gnu.org> References: <877crlyidb.wl-zack@owlfolio.org> <87bkd5t7fr.fsf@gnu.org> Date: Wed, 11 Oct 2023 14:56:55 -0400 From: "Zack Weinberg" To: =?UTF-8?Q?Ludovic_Court=C3=A8s?= Subject: Re: [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 64359 Cc: 64359@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On Wed, Oct 11, 2023, at 1:06 PM, Ludovic Court=C3=A8s wrote: >> This patch makes the code emitted by --search-paths compatible with s= et -u, >> by changing each use of bare `$VARIABLE` to `${VARIABLE:-}`, e.g. >> >> $ bash -c ' >> set -u >> unset PKG_CONFIG_PATH >> export PKG_CONFIG_PATH=3D"/example/lib/pkgconfig${PKG_CONFIG_= PATH:+:}${PKG_CONFIG_PATH:-}" >> echo $PKG_CONFIG_PATH' >> /example/lib/pkgconfig > > This change makes a lot of sense to me. Glad to hear! >> +++ b/guix/build/utils.scm >> @@ -1384,19 +1384,19 @@ (define (export-variable lst) >> (format #f "export ~a=3D\"~a\"" >> var (string-join rest sep))) >> ((var sep 'prefix rest) >> - (format #f "export ~a=3D\"~a${~a:+~a}$~a\"" >> + (format #f "export ~a=3D\"~a${~a:+~a}${~a:-}\"" >> var (string-join rest sep) var sep var)) > > This part is a full-rebuild change, so it=E2=80=99d have to wait. How= ever, it=E2=80=99s > within =E2=80=98wrap-program=E2=80=99; the script generated by =E2=80=98= wrap-program=E2=80=99 does *not* > use =E2=80=98set -u=E2=80=99, so I think this change is unnecessary. = Am I right? It's not strictly necessary to fix the bug, no. I made this change beca= use it's the only other appearance of 'export VAR=3D"additional-value${VAR:+= :}$VAR"' in the guix source code and I thought it would be better to change both = of them the same way. If only so that years from now someone doesn't waste = any time wondering why they're not quite the same and whether it matters. Why is it a full-rebuild change? As you point out, it should not actual= ly change anything? >> --- a/tests/guix-environment.sh >> +++ b/tests/guix-environment.sh > You can remove this change and keep only the =E2=80=98tests/guix-shell= .sh=E2=80=99 part. I know "guix environment" is obsolete, but isn't it appropriate to test = it thoroughly for as long as it still exists? (and again, years from now s= omeone might waste time wondering why this is only tested for "guix shell") zw From debbugs-submit-bounces@debbugs.gnu.org Sat Oct 14 13:04:51 2023 Received: (at 64359) by debbugs.gnu.org; 14 Oct 2023 17:04:51 +0000 Received: from localhost ([127.0.0.1]:50323 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qri4I-0002X5-Lo for submit@debbugs.gnu.org; Sat, 14 Oct 2023 13:04:50 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40740) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qri4H-0002Wt-27 for 64359@debbugs.gnu.org; Sat, 14 Oct 2023 13:04:49 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qri3n-0004Dr-SL; Sat, 14 Oct 2023 13:04:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:References:In-Reply-To:Subject:To: From; bh=rDwaRCS/W6XVwCLP2oQdeaAfx7h9VlVYXUgVKxtpdvk=; b=Nql4jJQT51xw9oKNiRPZ yATf4FqWCNergvBKguASn4V918urRmY+1PQw71ik44lXyLH+tuW44nR86UJyzx/MR5VpmGbA41yO8 7+oAWOxOso4muW5XJYwYlE3uRU7IwB06sP2U9ogU3i+pyOfIGs4xz9UqkG5t9YCcm9e8g9OyVc79H xWWI+cN4utxUbzlDFTIQrCtv/Ty2m4a6p5gLmUIXqxsym4gjWcP3sul/V/d3zIy2WE3aVJyOL+NhF 37+7ZRLPpNhXEWK4qceMQpme++Y8YbS8sz4rYhyxbo/seZgDzbvXHGPrXHsB/So+5O1hOX03vjpXt T9g/1ckD5sa/AA==; From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: "Zack Weinberg" Subject: Re: [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u In-Reply-To: (Zack Weinberg's message of "Wed, 11 Oct 2023 14:56:55 -0400") References: <877crlyidb.wl-zack@owlfolio.org> <87bkd5t7fr.fsf@gnu.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: Tridi 23 =?utf-8?Q?Vend=C3=A9miaire?= an 232 de la =?utf-8?Q?R=C3=A9volution=2C?= jour du Navet X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sat, 14 Oct 2023 19:04:17 +0200 Message-ID: <87lec5dtke.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 64359 Cc: 64359@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi Zack, "Zack Weinberg" skribis: >>> +++ b/guix/build/utils.scm >>> @@ -1384,19 +1384,19 @@ (define (export-variable lst) >>> (format #f "export ~a=3D\"~a\"" >>> var (string-join rest sep))) >>> ((var sep 'prefix rest) >>> - (format #f "export ~a=3D\"~a${~a:+~a}$~a\"" >>> + (format #f "export ~a=3D\"~a${~a:+~a}${~a:-}\"" >>> var (string-join rest sep) var sep var)) >> >> This part is a full-rebuild change, so it=E2=80=99d have to wait. Howev= er, it=E2=80=99s >> within =E2=80=98wrap-program=E2=80=99; the script generated by =E2=80=98= wrap-program=E2=80=99 does *not* >> use =E2=80=98set -u=E2=80=99, so I think this change is unnecessary. Am= I right? > > It's not strictly necessary to fix the bug, no. I made this change becau= se > it's the only other appearance of 'export VAR=3D"additional-value${VAR:+:= }$VAR"' > in the guix source code and I thought it would be better to change both of > them the same way. If only so that years from now someone doesn't waste a= ny > time wondering why they're not quite the same and whether it matters. > > Why is it a full-rebuild change? As you point out, it should not actually > change anything? It=E2=80=99s a full-rebuild change because every single package depends on = (guix build utils). When we change it, we have to rebuild literally every package. >>> --- a/tests/guix-environment.sh >>> +++ b/tests/guix-environment.sh >> You can remove this change and keep only the =E2=80=98tests/guix-shell.s= h=E2=80=99 part. > > I know "guix environment" is obsolete, but isn't it appropriate to test it > thoroughly for as long as it still exists? (and again, years from now so= meone > might waste time wondering why this is only tested for "guix shell") No, I think it=E2=80=99s unnecessary because the two share the same code. Eventually we=E2=80=99ll merge the two tests (and remove =E2=80=98guix envi= ronment=E2=80=99, someday). Could you send an updated patch? Thanks, Ludo=E2=80=99. From debbugs-submit-bounces@debbugs.gnu.org Wed Oct 18 14:22:30 2023 Received: (at 64359) by debbugs.gnu.org; 18 Oct 2023 18:22:30 +0000 Received: from localhost ([127.0.0.1]:34645 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qtBBd-00074Q-Uq for submit@debbugs.gnu.org; Wed, 18 Oct 2023 14:22:30 -0400 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:54141) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qtBBb-00074B-IO for 64359@debbugs.gnu.org; Wed, 18 Oct 2023 14:22:28 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 470803200AEB; Wed, 18 Oct 2023 14:21:55 -0400 (EDT) Received: from imap45 ([10.202.2.95]) by compute5.internal (MEProxy); Wed, 18 Oct 2023 14:21:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=owlfolio.org; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm2; t= 1697653314; x=1697739714; bh=Q3SCc4C93hAk/F+zLLtWQprOmWnYgEAyaTy rIIukdmQ=; b=Q1XjADn6UpHFzH4T3y4lQmkz3Cczz/0S1o0lhOkBLqfjTRgx3c9 EzJbotHqR3WK/SBE8PtRUTvwO5hlmYF2+ZjeVBJ9wjrKZZ9+y12brhzlVlxOEDMX EbhwcuCP4EsDLvdcYqPdDdCN4IQehtLGWvUhS5t59tWFNAjSaRHXo/Gm4L10heBV xoIynmuwIq13EHyJGGj2JVd744H/XlzQEWHAQD4BuK3IHM53tebc2V6P2sDvgBI8 XfPXoJ3iDnYBsna/WbrPWXgApl5Ofskxv3eEwnKGpg1v8vvp7BfRs1kocjeBf410 0ULteUeyibsDQJ1PqcWenqK1zcVH7d1by+Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1697653314; x=1697739714; bh=Q3SCc4C93hAk/F+zLLtWQprOmWnYgEAyaTy rIIukdmQ=; b=GfHkyG/oXa6juTvmuilFsiqt82mluwkPa3QMwiQCHbp0h/ZL+Fc 6ge8fgts5bq8Ka1xdfpl2UDq3LGFn9VDWaP1RRvTomkTj0hPqRqVOwoWJRadipQw jiOK8WX3XiLI6plufj8eKOkc+qsUwEfmtWn0qb1yPwcO1e7o8LfEm9IxyD5AQ/zo Rj51qpBGOfCTkJhC9Hm7WJZN+4/xOaxMxHF7p/hrh4oklpl+PG1grkTmKJNGQduE 41kTxSy95cvwoSGjD6dukhwXYJO0DA6tl7anEgI8RKMJ6NfKuihv5BrzsQjb4qQx 9NP9nvEPTOcYAX6epSQmWA5Tj1hTrZ24soQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrjeeggdduvddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvfevufgtgfesthhqredtreerjeenucfhrhhomhepfdgk rggtkhcuhggvihhnsggvrhhgfdcuoeiirggtkhesohiflhhfohhlihhordhorhhgqeenuc ggtffrrghtthgvrhhnpeduueeigeehffekiefhtdehiedvueffteevtefhudfguedtueei tdetgfetieeiieenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpeiirggtkhesohiflhhfohhlihhordhorhhg X-ME-Proxy: Feedback-ID: i876146a2:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 704BE272007C; Wed, 18 Oct 2023 14:21:54 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-1019-ged83ad8595-fm-20231002.001-ged83ad85 MIME-Version: 1.0 Message-Id: <7ab35fdd-f495-469e-b7fb-004faae6c74b@app.fastmail.com> In-Reply-To: <87lec5dtke.fsf@gnu.org> References: <877crlyidb.wl-zack@owlfolio.org> <87bkd5t7fr.fsf@gnu.org> <87lec5dtke.fsf@gnu.org> Date: Wed, 18 Oct 2023 14:21:34 -0400 From: "Zack Weinberg" To: =?UTF-8?Q?Ludovic_Court=C3=A8s?= Subject: Re: [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 64359 Cc: 64359@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On Sat, Oct 14, 2023, at 1:04 PM, Ludovic Court=C3=A8s wrote: >> Why is it a full-rebuild change? As you point out, it should not act= ually >> change anything? > > It=E2=80=99s a full-rebuild change because every single package depend= s on (guix > build utils). When we change it, we have to rebuild literally every > package. I'm sorry, I still don't understand why a change with no functional effe= ct on the packages themselves would require a full rebuild. zw