From: Miguel Ojeda <ojeda@kernel.org>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>,
Nicolas Schier <nicolas@fjasle.eu>,
linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
patches@lists.linux.dev, Miguel Ojeda <ojeda@kernel.org>
Subject: [PATCH v2] kbuild: pahole-version: improve overall checking and error messages
Date: Mon, 2 Sep 2024 18:08:28 +0200 [thread overview]
Message-ID: <20240902160828.1092891-1-ojeda@kernel.org> (raw)
Like patch "rust: suppress error messages from
CONFIG_{RUSTC,BINDGEN}_VERSION_TEXT" [1], do not assume the file existing
and being executable implies executing it will succeed.
Instead, bail out if executing it fails for any reason, as well as if
the program does not print to standard output what we are expecting from
`pahole --version`. The script needs to ensure that it always returns
an integer, since its output will go into a Kconfig `int`-type symbol.
In addition, check whether the program exists first, and provide
error messages for each error condition, similar to how it is done in
e.g. `scripts/rust_is_available.sh`.
For instance, currently `pahole` may be built for another architecture
or may be a program we do not expect that fails:
$ echo 'bad' > bad-pahole
$ chmod u+x bad-pahole
$ make PAHOLE=./bad-pahole defconfig
...
./bad-pahole: 1: bad: not found
init/Kconfig:112: syntax error
init/Kconfig:112: invalid statement
With this applied, we get instead:
***
*** Running './bad-pahole' to check the pahole version failed with
*** code 127. pahole will not be used.
***
...
$ grep CONFIG_PAHOLE_VERSION .config
CONFIG_PAHOLE_VERSION=0
Similarly, `pahole` currently may be a program that returns successfully,
but that does not print the version that we would expect:
$ echo 'echo' > bad-pahole
$ chmod u+x bad-pahole
$ make PAHOLE=./bad-pahole defconfig
...
init/Kconfig:114: syntax error
init/Kconfig:114: invalid statement
With this applied, we get instead:
***
*** pahole './bad-pahole' returned an unexpected version output.
*** pahole will not be used.
***
...
$ grep CONFIG_PAHOLE_VERSION .config
CONFIG_PAHOLE_VERSION=0
Finally, with this applied, if the program does not exist, we get:
$ make PAHOLE=./bad-pahole defconfig
...
***
*** pahole './bad-pahole' could not be found. pahole will not be used.
***
...
$ grep CONFIG_PAHOLE_VERSION .config
CONFIG_PAHOLE_VERSION=0
Link: https://lore.kernel.org/rust-for-linux/20240727140302.1806011-1-masahiroy@kernel.org/ [1]
Co-developed-by: Nicolas Schier <nicolas@fjasle.eu>
Signed-off-by: Nicolas Schier <nicolas@fjasle.eu>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
v1: https://lore.kernel.org/all/20240728125527.690726-1-ojeda@kernel.org/
v2:
Reworked to catch successful programs that may not output what we expect from
pahole as well as to do the checking step-by-step (for better error messages).
I didn't change the regular expression to reduce the changes (except adding
`p`) -- it can be improved separately.
Also, please note that I added Nicolas as co-author since he proposed part of
the solution, but he has not formally signed off yet.
scripts/pahole-version.sh | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/scripts/pahole-version.sh b/scripts/pahole-version.sh
index f8a32ab93ad1..cdb80a3d6e4f 100755
--- a/scripts/pahole-version.sh
+++ b/scripts/pahole-version.sh
@@ -5,9 +5,33 @@
#
# Prints pahole's version in a 3-digit form, such as 119 for v1.19.
-if [ ! -x "$(command -v "$@")" ]; then
- echo 0
+set -e
+trap "echo 0; exit 1" EXIT
+
+if ! command -v "$@" >/dev/null; then
+ echo >&2 "***"
+ echo >&2 "*** pahole '$@' could not be found. pahole will not be used."
+ echo >&2 "***"
+ exit 1
+fi
+
+output=$("$@" --version 2>/dev/null) || code=$?
+if [ -n "$code" ]; then
+ echo >&2 "***"
+ echo >&2 "*** Running '$@' to check the pahole version failed with"
+ echo >&2 "*** code $code. pahole will not be used."
+ echo >&2 "***"
+ exit 1
+fi
+
+output=$(echo "$output" | sed -nE 's/v([0-9]+)\.([0-9]+)/\1\2/p')
+if [ -z "${output}" ]; then
+ echo >&2 "***"
+ echo >&2 "*** pahole '$@' returned an unexpected version output."
+ echo >&2 "*** pahole will not be used."
+ echo >&2 "***"
exit 1
fi
-"$@" --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'
+echo "${output}"
+trap EXIT
base-commit: 431c1646e1f86b949fa3685efc50b660a364c2b6
--
2.46.0
next reply other threads:[~2024-09-02 16:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-02 16:08 Miguel Ojeda [this message]
2024-09-02 16:12 ` [PATCH v2] kbuild: pahole-version: improve overall checking and error messages Miguel Ojeda
2024-09-03 19:52 ` Nicolas Schier
2024-09-04 2:15 ` Masahiro Yamada
2024-09-03 19:48 ` Nicolas Schier
2024-09-04 0:15 ` Miguel Ojeda
2024-09-04 2:23 ` Masahiro Yamada
2024-09-04 2:04 ` Masahiro Yamada
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240902160828.1092891-1-ojeda@kernel.org \
--to=ojeda@kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=nathan@kernel.org \
--cc=nicolas@fjasle.eu \
--cc=patches@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.