Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Ray Kinsella <mdr@ashroe.eu>]

On 17/04/2020 17:10, Thomas Monjalon wrote:
> 17/04/2020 17:42, Ray Kinsella:
>> On 17/04/2020 13:10, Thomas Monjalon wrote:
>>> 17/04/2020 13:47, Ray Kinsella:
>>>> On 17/04/2020 11:20, Thomas Monjalon wrote:
>>>>> 17/04/2020 12:11, Ray Kinsella:
>>>>>> check-abi.sh appears to be backward step in terms of usability.
>>>>>
>>>>> No, check-abi.sh benefits from a nice integration in build scripts.
>>>>> See below.
>>>>>
>>>>>> With validate-abi.sh I do can do a "validate-abi.sh HEAD~1 HEAD".
>>>>>> And it will do the build, install, dump and comparison for me. 
>>>>>> And it picked up my 20.0.2 - > 21.0 changes no problem. 
>>>>>>
>>>>>> With check-abi on the other hand, I need to the build and install myself.
>>>>>> check-abi requires dump files, but I see no reference in the documentation to how these are created.
>>>>>> It silently fails when it doesn't find any ...
>>>>>>
>>>>>> Do I run abi-dumper on the so's myself, or how does it work?
>>>>>
>>>>> check-abi.sh is integrated in test-build.sh and test-meson-builds.sh.
>>>>> Probably we should document usage in these scripts.
>>>>
>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious.
>>>> Any tips or tricks would be welcome.
>>>
>>> export DPDK_ABI_REF_VERSION=v20.02
>>> or
>>> export DPDK_ABI_REF_VERSION=v19.11
>>>
>>> Depends on which compatibility you want to test...
>>>
>>
>> Few things ...
>>
>> 1. test-meson-build.sh keep barfing complaining about reference paths.
>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app
>>
>> Under the hood, ninja install is failing complaining that it needs an absolute path.
>> I fixed this in test_meson_build.sh and will send a patch in a minute. 
>> Though it's strange no-one else has seen it?
> 
> I set an absolute path in DPDK_ABI_REF_DIR.
> Not sure you can really fix it. What would be the root dir?

Figure it out at runtime, check with realpath, if DPDK_ABI_REF_DIR is not set?
Or at the very least test_meson_build.sh should barf, if DPDK_ABI_REF_DIR is not set when $DPDK_ABI_REF_VERSION is set.

root@silpixa00395806:/build/dpdk# git diff devtools/test-meson-builds.sh
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index c1ff2bb50..f24cc104d 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -103,13 +103,15 @@ compile () # <builddir>

 install_target () # <builddir> <installdir>
 {
+       destdir_fp=$(realpath $2)
+
        rm -rf $2
        if [ -n "$TEST_MESON_BUILD_VERY_VERBOSE$TEST_MESON_BUILD_VERBOSE" ]; then
                echo "DESTDIR=$2 $ninja_cmd -C $1 install"
-               DESTDIR=$2 $ninja_cmd -C $1 install
+               DESTDIR=$destdir_fp $ninja_cmd -C $1 install
        else
                echo "DESTDIR=$2 $ninja_cmd -C $1 install >/dev/null"
-               DESTDIR=$2 $ninja_cmd -C $1 install >/dev/null
+               DESTDIR=$destdir_fp $ninja_cmd -C $1 install >/dev/null
        fi
 }

@@ -147,7 +149,7 @@ build () # <directory> <target compiler> <meson options>
                $srcdir/devtools/gen-abi.sh \
                        $(readlink -f $builds_dir/$targetdir/install)
                $srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
-                       $(readlink -f $builds_dir/$targetdir/install)
+                       $(readlink -f $builds_dir/$targetdir/install) || :
        fi
 }

> 
>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. 
> 
> Yes
> 
>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally.
>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case.
>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD`
> 
> Why is it a common case? You want to compare with a tag. Why something else?
> 

I disagree, perhaps it's just me as a contributor.
My first action is to check that _my_ submission has done no harm. 
Assuming that the origin's HEAD is more than likely good, I check my changes against it.

I assume that anything else that has changed between the origin's HEAD and v19.11, someone somewhere else has approved. 
So why would I compare against v19.11,  when I only want to check that the changes I have made have caused no harm.