MIME-Version: 1.0
References: <20230217202234.32260-1-quic_johmoo@quicinc.com> <20230217202234.32260-2-quic_johmoo@quicinc.com>
In-Reply-To: <20230217202234.32260-2-quic_johmoo@quicinc.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sun, 19 Feb 2023 14:25:58 +0900
Message-ID: <CAK7LNASf_RQGM6QYwvUN+FLq-v3jK66Pk5jW9M-vM1V4K0xH8g@mail.gmail.com>
Subject: Re: [PATCH RESEND 1/1] check-uapi: Introduce check-uapi.sh
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@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        Trilok Soni <quic_tsoni@quicinc.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Todd Kjos <tkjos@google.com>,
        Matthias Maennich <maennich@google.com>,
        Giuliano Procida <gprocida@google.com>,
        kernel-team@android.com, Jordan Crouse <jorcrous@amazon.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Feb 18, 2023 at 5:23 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 patch 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 patch 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."
>
> Signed-off-by: John Moon <quic_johmoo@quicinc.com>


BTW, the patch is prefixed with 'RESEND', but it contains
several diff lines against the previous submission [1].

People would submit this as v2 with changes mentioned under '---'.


[1]: https://patchwork.kernel.org/project/linux-kbuild/patch/20230217202234.32260-2-quic_johmoo@quicinc.com/


diff --git a/scripts/check-uapi.sh b/scripts/check-uapi.sh
index 209c6793a4d0..b9cd3a2d7805 100755
--- a/scripts/check-uapi.sh
+++ b/scripts/check-uapi.sh
@@ -3,6 +3,7 @@

 # Script to check a patch for UAPI stability
 set -o errexit
+set -o pipefail

 print_usage() {
        name=$(basename "$0")
@@ -34,7 +35,7 @@ get_header() {

        if [ ! -x "${KERNEL_SRC}/scripts/unifdef" ]; then
                if ! make -C "${KERNEL_SRC}/scripts" unifdef; then
-                       echo 'error - failed to build required
dependency "scripts/unifdef"'
+                       errlog 'error - failed to build required
dependency "scripts/unifdef"'
                        exit 1
                fi
        fi
@@ -111,7 +112,7 @@ check_changed_files() {

        total=$((passed + failed))
        if [ "$total" -eq 0 ]; then
-               errlog "No changes to UAPI headers detected"
+               errlog "No changes to UAPI headers detected in most
recent commit"
        else
                errlog "${passed}/${total} UAPI header file changes
are backwards compatible"
        fi
@@ -190,16 +191,15 @@ check_deps() {
                errlog "error - abidiff not found!"
                errlog "Please install abigail-tools (version 1.7 or greater)"
                errlog "See:
https://sourceware.org/libabigail/manual/libabigail-overview.html"
-               exit 2
+               exit 1
        fi

-       local -r abidiff_maj=$("$ABIDIFF" --version | cut -d ' ' -f 2
| cut -d '.' -f 1)
-       local -r abidiff_min=$("$ABIDIFF" --version | cut -d ' ' -f 2
| cut -d '.' -f 1)
+       read -r abidiff_maj abidiff_min _ < <("$ABIDIFF" --version |
cut -d ' ' -f 2 | tr '.' ' ')
        if [ "$abidiff_maj" -lt 1 ] || ([ "$abidiff_maj" -eq 1 ] && [
"$abidiff_min" -lt 7 ]); then
-               errlog "error - installed abidiff version too old:
$("$ABIDIFF" --version)"
+               errlog "error - abidiff version too old: $("$ABIDIFF"
--version)"
                errlog "Please install abigail-tools (version 1.7 or greater)"
                errlog "See:
https://sourceware.org/libabigail/manual/libabigail-overview.html"
-               exit 2
+               exit 1
        fi
 }

@@ -220,13 +220,18 @@ main() {
        check_deps

        tmp_dir=$(mktemp -d)
-       #trap 'rm -rf $tmp_dir' EXIT
+       trap 'rm -rf $tmp_dir' EXIT

        if [ -z "$KERNEL_SRC" ]; then
                KERNEL_SRC="$(realpath "$(dirname "$0")"/..)"
        fi
        export KERNEL_SRC

+       if ! (cd "$KERNEL_SRC" && git rev-parse --is-inside-work-tree
> /dev/null 2>&1); then
+               errlog "error - this script requires the kernel tree
to be initialized with Git"
+               exit 1
+       fi
+
        export ARCH
        export CC
        export CROSS_COMPILE


-- 
Best Regards
Masahiro Yamada