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
+ 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. -- David Marchand
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.
--
Dodji
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>
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> Acked-by: Ray Kinsella <mdr@ashroe.eu> Acked-by: Neil Horman <nhorman@tuxdriver.com> --- Changes since v1: - used arithmetic test, - updated error message for generic errors, --- 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..172e934382 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)) -ne 0 ]; then + echo "ABIDIFF_ERROR|ABIDIFF_USAGE_ERROR, this could be a script or environment issue." + fi + if [ $(($abiret & 4)) -ne 0 ]; then + echo "ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged this as a potential issue)." + fi + if [ $(($abiret & 8)) -ne 0 ]; then + echo "ABIDIFF_ABI_INCOMPATIBLE_CHANGE, this change breaks the ABI." + fi + echo + } done [ -z "$error" ] || [ -n "$warnonly" ] -- 2.23.0
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>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> Changes since v1:
> - used arithmetic test,
> - updated error message for generic errors,
>
Acked-by: Aaron Conole <aconole@redhat.com>
On Wed, Jul 15, 2020 at 2:49 PM Aaron Conole <aconole@redhat.com> wrote:
> 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>
> > Acked-by: Ray Kinsella <mdr@ashroe.eu>
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Acked-by: Aaron Conole <aconole@redhat.com>
Applied.
--
David Marchand