From: Masahiro Yamada <masahiroy@kernel.org> To: John Moon <quic_johmoo@quicinc.com> Cc: Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, Nicolas Schier <nicolas@fjasle.eu>, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Randy Dunlap <rdunlap@infradead.org>, Arnd Bergmann <arnd@arndb.de>, Bjorn Andersson <andersson@kernel.org>, Todd Kjos <tkjos@google.com>, Matthias Maennich <maennich@google.com>, Giuliano Procida <gprocida@google.com>, kernel-team@android.com, libabigail@sourceware.org, Jordan Crouse <jorcrous@amazon.com>, Trilok Soni <quic_tsoni@quicinc.com>, Satya Durga Srinivasu Prabhala <quic_satyap@quicinc.com>, Elliot Berman <quic_eberman@quicinc.com>, Guru Das Srinagesh <quic_gurus@quicinc.com> Subject: Re: [PATCH v4 1/2] check-uapi: Introduce check-uapi.sh Date: Sat, 8 Apr 2023 04:19:48 +0900 [thread overview] Message-ID: <CAK7LNAQ26HVA-oMwOqAg-diZ7dRa_41hjmRb88Vcv-GcQsFfqg@mail.gmail.com> (raw) In-Reply-To: <20230327174140.8169-2-quic_johmoo@quicinc.com> On Tue, Mar 28, 2023 at 2:42 AM John Moon <quic_johmoo@quicinc.com> wrote: > > While the kernel community has been good at maintaining backwards > compatibility with kernel UAPIs, it would be helpful to have a tool > to check if a commit introduces changes that break backwards > compatibility. > > To that end, introduce check-uapi.sh: a simple shell script that > checks for changes to UAPI headers using libabigail. > > libabigail is "a framework which aims at helping developers and > software distributors to spot some ABI-related issues like interface > incompatibility in ELF shared libraries by performing a static > analysis of the ELF binaries at hand." > > The script uses one of libabigail's tools, "abidiff", to compile the > changed header before and after the commit to detect any changes. > > abidiff "compares the ABI of two shared libraries in ELF format. It > emits a meaningful report describing the differences between the two > ABIs." > > The script also includes the ability to check the compatibility of > all UAPI headers across commits. This allows developers to inspect > the stability of the UAPIs over time. > > Signed-off-by: John Moon <quic_johmoo@quicinc.com> > --- > - Refactored to exclusively check headers installed by make > headers_install. This simplified the code dramatically and removed > the need to perform complex git diffs. > - Removed the "-m" flag. Since we're checking all installed headers > every time, a flag to check only modified files didn't make sense. > - Added info message when usr/include/Makefile is not present that > it's likely because that file was only introduced in v5.3. > - Changed default behavior of log file. Now, the script will not > create a log file unless you pass "-l <file>". > - Simplified exit handler. > - Added -j $MAX_THREADS to make headers_install to improve speed. > - Cleaned up variable references. > > scripts/check-uapi.sh | 488 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 488 insertions(+) > create mode 100755 scripts/check-uapi.sh > > + > +# Save the current git tree state, stashing if needed > +save_tree_state() { > + printf "Saving current tree state... " > + current_ref="$(git rev-parse HEAD)" > + readonly current_ref > + if tree_is_dirty; then > + unstash="true" > + git stash push --quiet > + fi > + printf "OK\n" > +} > + > +# Restore the git tree state, unstashing if needed > +restore_tree_state() { > + if [ -z "$current_ref" ]; then > + return 0 > + fi > + > + printf "Restoring current tree state... " > + git checkout --quiet "$current_ref" This does not restore the original state. I was on a branch before running this script. After everything is finished, I am on a detached commit because $current_ref is not a branch. > + if ! do_compile "$(get_header_tree "$past_ref")/include" "$past_header" "${past_header}.bin" 2> "$log"; then > + eprintf "error - couldn't compile version of UAPI header %s at %s\n" "$file" "$past_ref" > + cat "$log" >&2 > + exit "$FAIL_COMPILE" > + fi > + > + "$ABIDIFF" --non-reachable-types "${past_header}.bin" "${base_header}.bin" > "$log" && ret="$?" || ret="$?" [bikeshed] I might want to write like this: ret=0 "$ABIDIFF" --non-reachable-types "${past_header}.bin" "${base_header}.bin" > "$log" || ret="$?" > + > + > + if [ "$quiet" = "true" ]; then > + run "$base_ref" "$past_ref" "$abi_error_log" "$@" > /dev/null > + else > + run "$base_ref" "$past_ref" "$abi_error_log" "$@" > + fi if [ "$quiet" = "true" ]; then exec > /dev/null fi run "$base_ref" "$past_ref" "$abi_error_log" "$@" is more elegant because this is the last line of main() and exit_handler() does not print anything. -- Best Regards Masahiro Yamada
WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <masahiroy@kernel.org> To: John Moon <quic_johmoo@quicinc.com> Cc: Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, Nicolas Schier <nicolas@fjasle.eu>, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Randy Dunlap <rdunlap@infradead.org>, Arnd Bergmann <arnd@arndb.de>, Bjorn Andersson <andersson@kernel.org>, Todd Kjos <tkjos@google.com>, Matthias Maennich <maennich@google.com>, Giuliano Procida <gprocida@google.com>, kernel-team@android.com, libabigail@sourceware.org, Jordan Crouse <jorcrous@amazon.com>, Trilok Soni <quic_tsoni@quicinc.com>, Satya Durga Srinivasu Prabhala <quic_satyap@quicinc.com>, Elliot Berman <quic_eberman@quicinc.com>, Guru Das Srinagesh <quic_gurus@quicinc.com> Subject: Re: [PATCH v4 1/2] check-uapi: Introduce check-uapi.sh Date: Sat, 8 Apr 2023 04:19:48 +0900 [thread overview] Message-ID: <CAK7LNAQ26HVA-oMwOqAg-diZ7dRa_41hjmRb88Vcv-GcQsFfqg@mail.gmail.com> (raw) In-Reply-To: <20230327174140.8169-2-quic_johmoo@quicinc.com> On Tue, Mar 28, 2023 at 2:42 AM John Moon <quic_johmoo@quicinc.com> wrote: > > While the kernel community has been good at maintaining backwards > compatibility with kernel UAPIs, it would be helpful to have a tool > to check if a commit introduces changes that break backwards > compatibility. > > To that end, introduce check-uapi.sh: a simple shell script that > checks for changes to UAPI headers using libabigail. > > libabigail is "a framework which aims at helping developers and > software distributors to spot some ABI-related issues like interface > incompatibility in ELF shared libraries by performing a static > analysis of the ELF binaries at hand." > > The script uses one of libabigail's tools, "abidiff", to compile the > changed header before and after the commit to detect any changes. > > abidiff "compares the ABI of two shared libraries in ELF format. It > emits a meaningful report describing the differences between the two > ABIs." > > The script also includes the ability to check the compatibility of > all UAPI headers across commits. This allows developers to inspect > the stability of the UAPIs over time. > > Signed-off-by: John Moon <quic_johmoo@quicinc.com> > --- > - Refactored to exclusively check headers installed by make > headers_install. This simplified the code dramatically and removed > the need to perform complex git diffs. > - Removed the "-m" flag. Since we're checking all installed headers > every time, a flag to check only modified files didn't make sense. > - Added info message when usr/include/Makefile is not present that > it's likely because that file was only introduced in v5.3. > - Changed default behavior of log file. Now, the script will not > create a log file unless you pass "-l <file>". > - Simplified exit handler. > - Added -j $MAX_THREADS to make headers_install to improve speed. > - Cleaned up variable references. > > scripts/check-uapi.sh | 488 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 488 insertions(+) > create mode 100755 scripts/check-uapi.sh > > + > +# Save the current git tree state, stashing if needed > +save_tree_state() { > + printf "Saving current tree state... " > + current_ref="$(git rev-parse HEAD)" > + readonly current_ref > + if tree_is_dirty; then > + unstash="true" > + git stash push --quiet > + fi > + printf "OK\n" > +} > + > +# Restore the git tree state, unstashing if needed > +restore_tree_state() { > + if [ -z "$current_ref" ]; then > + return 0 > + fi > + > + printf "Restoring current tree state... " > + git checkout --quiet "$current_ref" This does not restore the original state. I was on a branch before running this script. After everything is finished, I am on a detached commit because $current_ref is not a branch. > + if ! do_compile "$(get_header_tree "$past_ref")/include" "$past_header" "${past_header}.bin" 2> "$log"; then > + eprintf "error - couldn't compile version of UAPI header %s at %s\n" "$file" "$past_ref" > + cat "$log" >&2 > + exit "$FAIL_COMPILE" > + fi > + > + "$ABIDIFF" --non-reachable-types "${past_header}.bin" "${base_header}.bin" > "$log" && ret="$?" || ret="$?" [bikeshed] I might want to write like this: ret=0 "$ABIDIFF" --non-reachable-types "${past_header}.bin" "${base_header}.bin" > "$log" || ret="$?" > + > + > + if [ "$quiet" = "true" ]; then > + run "$base_ref" "$past_ref" "$abi_error_log" "$@" > /dev/null > + else > + run "$base_ref" "$past_ref" "$abi_error_log" "$@" > + fi if [ "$quiet" = "true" ]; then exec > /dev/null fi run "$base_ref" "$past_ref" "$abi_error_log" "$@" is more elegant because this is the last line of main() and exit_handler() does not print anything. -- Best Regards Masahiro Yamada _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-04-07 19:20 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-27 17:41 [PATCH v4 0/2] Validating UAPI backwards compatibility John Moon 2023-03-27 17:41 ` John Moon 2023-03-27 17:41 ` [PATCH v4 1/2] check-uapi: Introduce check-uapi.sh John Moon 2023-03-27 17:41 ` John Moon 2023-04-03 19:53 ` John Moon 2023-04-03 19:53 ` John Moon 2023-04-07 19:19 ` Masahiro Yamada [this message] 2023-04-07 19:19 ` Masahiro Yamada 2023-04-07 20:08 ` John Moon 2023-04-07 20:08 ` John Moon 2023-04-07 19:27 ` Masahiro Yamada 2023-04-07 19:27 ` Masahiro Yamada 2023-04-07 20:09 ` John Moon 2023-04-07 20:09 ` John Moon 2023-03-27 17:41 ` [PATCH v4 2/2] docs: dev-tools: Add UAPI checker documentation John Moon 2023-03-27 17:41 ` John Moon 2023-03-28 18:10 ` Nick Desaulniers 2023-03-28 18:10 ` Nick Desaulniers
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=CAK7LNAQ26HVA-oMwOqAg-diZ7dRa_41hjmRb88Vcv-GcQsFfqg@mail.gmail.com \ --to=masahiroy@kernel.org \ --cc=andersson@kernel.org \ --cc=arnd@arndb.de \ --cc=gprocida@google.com \ --cc=gregkh@linuxfoundation.org \ --cc=jorcrous@amazon.com \ --cc=kernel-team@android.com \ --cc=libabigail@sourceware.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maennich@google.com \ --cc=nathan@kernel.org \ --cc=ndesaulniers@google.com \ --cc=nicolas@fjasle.eu \ --cc=quic_eberman@quicinc.com \ --cc=quic_gurus@quicinc.com \ --cc=quic_johmoo@quicinc.com \ --cc=quic_satyap@quicinc.com \ --cc=quic_tsoni@quicinc.com \ --cc=rdunlap@infradead.org \ --cc=tkjos@google.com \ /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: linkBe 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.