From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms9.migadu.com with LMTPS id UC82CU4hnmSzqwAASxT56A (envelope-from ) for ; Fri, 30 Jun 2023 02:26:54 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id ODQhCU4hnmQt0wAAauVa8A (envelope-from ) for ; Fri, 30 Jun 2023 02:26:54 +0200 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 4DFCC12BE6 for ; Fri, 30 Jun 2023 02:26:52 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qF1y4-0002MU-EX; Thu, 29 Jun 2023 20:26:33 -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 1qF0nz-000154-8w for guix-patches@gnu.org; Thu, 29 Jun 2023 19:12:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qF0ny-0001jo-2O for guix-patches@gnu.org; Thu, 29 Jun 2023 19:12:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qF0nx-0001mS-Nx for guix-patches@gnu.org; Thu, 29 Jun 2023 19:12:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u Resent-From: Zack Weinberg Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 29 Jun 2023 23:12:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 64359 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 64359@debbugs.gnu.org X-Debbugs-Original-To: guix-patches@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.16880802706770 (code B ref -1); Thu, 29 Jun 2023 23:12:01 +0000 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 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-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Mailman-Approved-At: Thu, 29 Jun 2023 20:26:25 -0400 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-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1688084813; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:resent-cc:resent-from:resent-sender: resent-message-id:list-id:list-help:list-unsubscribe:list-subscribe: list-post:dkim-signature; bh=qXjZT+79rx6hcGLIz7GFjmF+8AoRoJn7ndSEYUr1gcY=; b=X+OZHfWdV+G8q02U+foi/63n4T4AOw148KcfIExGzOUfnwCyT3tXaNY1oenkaXaoxd+x+S cSn4VwRa+gEeKPTuDsorlDRqx8IllqTbGAGQlWHzVa6y2ZtNQQ+9ZItuNUmBlTPlJ+/mRx JUCyctazzAzU3Mzr0bMUJ9W1HUXj3DZGoRVgQedQrShKUJD6QNXGBddcD/W4sqxEND5OWS SsNpOUP0wrSJOOCgp2Cj0EgDRGfRe1k+pZvxo3z01qlY64A8jcNhcjgIn5zk3vdqCisoFk QySHM2DvkKGxM2vfUo9SEWYvh0DdgKqhCe1DgjWNR8jKLa/xEkF5Y3wG4LExkw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=owlfolio.org header.s=fm1 header.b="oRvMk/L4"; dkim=fail ("headers rsa verify failed") header.d=messagingengine.com header.s=fm2 header.b=IGw+gOQV; dmarc=fail reason="SPF not aligned (relaxed)" header.from=owlfolio.org (policy=none); spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" ARC-Seal: i=1; s=key1; d=yhetil.org; t=1688084813; a=rsa-sha256; cv=none; b=lS9hz+UfYoQrQy7gZW44dZ7reXJ9DZoZvsCSop1TrB3ZiCcnXFYVOMTl/2aWCshCXA6zrZ RhcMi/5s5WazrATZHGtO+V/8SGJx2RPWwoWH7b/CavGITWm5APiprxR256CcUldxizPJ68 7toDmUTVcDkrdfmwm++R/tv+mSAHxRzDoPXRU/cuwnPuKv4K15631hVn8hg30Y6OwG5VEm j3Bpl5NP+4DbpS/k051gIk6oCW7QqAYHoYvLwldZ8EMh5iAnFWwUA9Jssc6FnnwijyhdwN x4tkwhwerITQ1am8ZC6B7rrCJnHEfBf8kOwAiyzfPvqi9xRLU4bqY6vLJrh1Wg== X-Migadu-Scanner: scn1.migadu.com X-Migadu-Spam-Score: -1.46 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=owlfolio.org header.s=fm1 header.b="oRvMk/L4"; dkim=fail ("headers rsa verify failed") header.d=messagingengine.com header.s=fm2 header.b=IGw+gOQV; dmarc=fail reason="SPF not aligned (relaxed)" header.from=owlfolio.org (policy=none); spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 4DFCC12BE6 X-Spam-Score: -1.46 X-TUID: wJm2mOBzz0J6 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