Re: [PATCH v4 1/2] check-uapi: Introduce check-uapi.sh

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/7/2023 12:19 PM, Masahiro Yamada wrote:
On Tue, Mar 28, 2023 at 2:42 AM John Moon <quic_johmoo@xxxxxxxxxxx> 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@xxxxxxxxxxx>
---
     - 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.



Good point. I think doing this should address:

current_ref="$(git symbolic-ref --short HEAD 2> /dev/null || git rev parse HEAD)"

Will fix in v5.



+       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="$?"




Sure, will do.





+
+
+       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.




Agreed, will do.







[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux