devtools: give some hints for ABI errors
Checks
Commit Message
abidiff can provide some more information about the ABI difference it
detected.
In all cases, a discussion on the mailing must happen but we can give
some hints to know if this is a problem with the script calling abidiff,
a potential ABI breakage or an unambiguous ABI breakage.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
devtools/check-abi.sh | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
Comments
+ Aaron
On 08/07/2020 11:22, David Marchand wrote:
> abidiff can provide some more information about the ABI difference it
> detected.
> In all cases, a discussion on the mailing must happen but we can give
> some hints to know if this is a problem with the script calling abidiff,
> a potential ABI breakage or an unambiguous ABI breakage.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> devtools/check-abi.sh | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
> index e17fedbd9f..521e2cce7c 100755
> --- a/devtools/check-abi.sh
> +++ b/devtools/check-abi.sh
> @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
> error=1
> continue
> fi
> - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
> + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
> + abiret=$?
> echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
> error=1
> - fi
> + echo
> + if [ $(($abiret & 3)) != 0 ]; then
> + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
> + fi
> + if [ $(($abiret & 4)) != 0 ]; then
> + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
> + fi
> + if [ $(($abiret & 8)) != 0 ]; then
> + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
> + fi
> + echo
> + }
> done
>
> [ -z "$error" ] || [ -n "$warnonly" ]
>
This look good to me, my only thought was can we do anything to help the ABI checks play nice with Travis.
At the moment it takes time to find the failure reason in the Travis log.
Ray K
On Wed, Jul 8, 2020 at 3:09 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>
> + Aaron
>
> On 08/07/2020 11:22, David Marchand wrote:
> > abidiff can provide some more information about the ABI difference it
> > detected.
> > In all cases, a discussion on the mailing must happen but we can give
> > some hints to know if this is a problem with the script calling abidiff,
> > a potential ABI breakage or an unambiguous ABI breakage.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> > devtools/check-abi.sh | 16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
> > index e17fedbd9f..521e2cce7c 100755
> > --- a/devtools/check-abi.sh
> > +++ b/devtools/check-abi.sh
> > @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
> > error=1
> > continue
> > fi
> > - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
> > + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
> > + abiret=$?
> > echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
> > error=1
> > - fi
> > + echo
> > + if [ $(($abiret & 3)) != 0 ]; then
> > + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
Forgot to --amend.
Hopefully yes, this will be reported to dev@dpdk.org... I wanted to
highlight this could be a script or env issue.
> > + fi
> > + if [ $(($abiret & 4)) != 0 ]; then
> > + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
> > + fi
> > + if [ $(($abiret & 8)) != 0 ]; then
> > + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
> > + fi
> > + echo
> > + }
> > done
> >
> > [ -z "$error" ] || [ -n "$warnonly" ]
> >
>
> This look good to me, my only thought was can we do anything to help the ABI checks play nice with Travis.
> At the moment it takes time to find the failure reason in the Travis log.
I usually look for "FILES_TO" to get to the last error.
On 08/07/2020 14:15, David Marchand wrote:
> On Wed, Jul 8, 2020 at 3:09 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>>
>> + Aaron
>>
>> On 08/07/2020 11:22, David Marchand wrote:
>>> abidiff can provide some more information about the ABI difference it
>>> detected.
>>> In all cases, a discussion on the mailing must happen but we can give
>>> some hints to know if this is a problem with the script calling abidiff,
>>> a potential ABI breakage or an unambiguous ABI breakage.
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>> ---
>>> devtools/check-abi.sh | 16 ++++++++++++++--
>>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
>>> index e17fedbd9f..521e2cce7c 100755
>>> --- a/devtools/check-abi.sh
>>> +++ b/devtools/check-abi.sh
>>> @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
>>> error=1
>>> continue
>>> fi
>>> - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
>>> + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
>>> + abiret=$?
>>> echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
>>> error=1
>>> - fi
>>> + echo
>>> + if [ $(($abiret & 3)) != 0 ]; then
>>> + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
>
> Forgot to --amend.
> Hopefully yes, this will be reported to dev@dpdk.org... I wanted to
> highlight this could be a script or env issue.
>
>
>>> + fi
>>> + if [ $(($abiret & 4)) != 0 ]; then
>>> + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
>>> + fi
>>> + if [ $(($abiret & 8)) != 0 ]; then
>>> + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
>>> + fi
>>> + echo
>>> + }
>>> done
>>>
>>> [ -z "$error" ] || [ -n "$warnonly" ]
>>>
>>
>> This look good to me, my only thought was can we do anything to help the ABI checks play nice with Travis.
>> At the moment it takes time to find the failure reason in the Travis log.
>
> I usually look for "FILES_TO" to get to the last error.
>
Right, but there is hopefully a better way to give Travis some clues ...
"Kinsella, Ray" <mdr@ashroe.eu> writes:
> + Aaron
>
> On 08/07/2020 11:22, David Marchand wrote:
>> abidiff can provide some more information about the ABI difference it
>> detected.
>> In all cases, a discussion on the mailing must happen but we can give
>> some hints to know if this is a problem with the script calling abidiff,
>> a potential ABI breakage or an unambiguous ABI breakage.
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> ---
>> devtools/check-abi.sh | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
>> index e17fedbd9f..521e2cce7c 100755
>> --- a/devtools/check-abi.sh
>> +++ b/devtools/check-abi.sh
>> @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
>> error=1
>> continue
>> fi
>> - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
>> + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
>> + abiret=$?
>> echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
>> error=1
>> - fi
>> + echo
>> + if [ $(($abiret & 3)) != 0 ]; then
>> + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
>> + fi
>> + if [ $(($abiret & 4)) != 0 ]; then
>> + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
>> + fi
>> + if [ $(($abiret & 8)) != 0 ]; then
>> + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
>> + fi
>> + echo
>> + }
>> done
>>
>> [ -z "$error" ] || [ -n "$warnonly" ]
>>
>
> This look good to me, my only thought was can we do anything to help the ABI checks play nice with Travis.
> At the moment it takes time to find the failure reason in the Travis log.
That's a problem even for non-ABI failures. I was considering pulling
the travis log for each failed build and attaching it, but even that
isn't a great solution (very large emails aren't much easier to search).
I'm open to suggestions.
> Ray K
On 08/07/2020 14:45, Aaron Conole wrote:
> "Kinsella, Ray" <mdr@ashroe.eu> writes:
>
>> + Aaron
>>
>> On 08/07/2020 11:22, David Marchand wrote:
>>> abidiff can provide some more information about the ABI difference it
>>> detected.
>>> In all cases, a discussion on the mailing must happen but we can give
>>> some hints to know if this is a problem with the script calling abidiff,
>>> a potential ABI breakage or an unambiguous ABI breakage.
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>> ---
>>> devtools/check-abi.sh | 16 ++++++++++++++--
>>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
>>> index e17fedbd9f..521e2cce7c 100755
>>> --- a/devtools/check-abi.sh
>>> +++ b/devtools/check-abi.sh
>>> @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
>>> error=1
>>> continue
>>> fi
>>> - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
>>> + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
>>> + abiret=$?
>>> echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
>>> error=1
>>> - fi
>>> + echo
>>> + if [ $(($abiret & 3)) != 0 ]; then
>>> + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
>>> + fi
>>> + if [ $(($abiret & 4)) != 0 ]; then
>>> + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
>>> + fi
>>> + if [ $(($abiret & 8)) != 0 ]; then
>>> + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
>>> + fi
>>> + echo
>>> + }
>>> done
>>>
>>> [ -z "$error" ] || [ -n "$warnonly" ]
>>>
>>
>> This look good to me, my only thought was can we do anything to help the ABI checks play nice with Travis.
>> At the moment it takes time to find the failure reason in the Travis log.
>
> That's a problem even for non-ABI failures. I was considering pulling
> the travis log for each failed build and attaching it, but even that
> isn't a great solution (very large emails aren't much easier to search).
>
> I'm open to suggestions.
For me the problem arises when you log on to the Travis interface,
you need to search for ERROR etc ... there must a better way.
>
>> Ray K
>
Hello,
David Marchand <david.marchand@redhat.com> writes:
> abidiff can provide some more information about the ABI difference it
> detected.
> In all cases, a discussion on the mailing must happen but we can give
> some hints to know if this is a problem with the script calling abidiff,
> a potential ABI breakage or an unambiguous ABI breakage.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
For what it's worth, the change looks good to me, at least from an
abidiff perspective.
Thanks.
Cheers.
On 08/07/2020 11:22, David Marchand wrote:
> abidiff can provide some more information about the ABI difference it
> detected.
> In all cases, a discussion on the mailing must happen but we can give
> some hints to know if this is a problem with the script calling abidiff,
> a potential ABI breakage or an unambiguous ABI breakage.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> devtools/check-abi.sh | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
> index e17fedbd9f..521e2cce7c 100755
> --- a/devtools/check-abi.sh
> +++ b/devtools/check-abi.sh
> @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
> error=1
> continue
> fi
> - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
> + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
> + abiret=$?
> echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
> error=1
> - fi
> + echo
> + if [ $(($abiret & 3)) != 0 ]; then
> + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
> + fi
> + if [ $(($abiret & 4)) != 0 ]; then
> + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
> + fi
> + if [ $(($abiret & 8)) != 0 ]; then
> + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
> + fi
> + echo
> + }
> done
>
> [ -z "$error" ] || [ -n "$warnonly" ]
>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
On Wed, Jul 08, 2020 at 12:22:12PM +0200, David Marchand wrote:
> abidiff can provide some more information about the ABI difference it
> detected.
> In all cases, a discussion on the mailing must happen but we can give
> some hints to know if this is a problem with the script calling abidiff,
> a potential ABI breakage or an unambiguous ABI breakage.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> devtools/check-abi.sh | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
> index e17fedbd9f..521e2cce7c 100755
> --- a/devtools/check-abi.sh
> +++ b/devtools/check-abi.sh
> @@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
> error=1
> continue
> fi
> - if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
> + abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
> + abiret=$?
> echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
> error=1
> - fi
> + echo
> + if [ $(($abiret & 3)) != 0 ]; then
> + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
> + fi
> + if [ $(($abiret & 4)) != 0 ]; then
> + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
> + fi
> + if [ $(($abiret & 8)) != 0 ]; then
> + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
> + fi
> + echo
> + }
> done
>
> [ -z "$error" ] || [ -n "$warnonly" ]
> --
> 2.23.0
>
>
this looks pretty reasonable to me, sure.
Acked-by: Neil Horman <nhorman@tuxdriver.com>
@@ -50,10 +50,22 @@ for dump in $(find $refdir -name "*.dump"); do
error=1
continue
fi
- if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then
+ abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
+ abiret=$?
echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'"
error=1
- fi
+ echo
+ if [ $(($abiret & 3)) != 0 ]; then
+ echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, please report this to dev@dpdk.org."
+ fi
+ if [ $(($abiret & 4)) != 0 ]; then
+ echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)."
+ fi
+ if [ $(($abiret & 8)) != 0 ]; then
+ echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI."
+ fi
+ echo
+ }
done
[ -z "$error" ] || [ -n "$warnonly" ]