fedabipkgdiff: make --self-compare use abipkgdiff --self-check

  Hello,

Now that the abipkgdiff program supports the --self-check option to
make it compare the binaries in an RPM against their own ABIXML
representation, make the --self-compare option of fedabipkgdiff use
the abipkgdiff --self-check.

	* tools/fedabipkgdiff (abipkgdiff): If the user provides the
	--self-compare options, generate the abipkgdiff command by using
	the --self-check option.
	(run_abipkgdiff): Each return value of the abipkgidiff runs can be
	negative because they are unsigned values in essence, but as
	python doesn't seem to have a unsigned integer type.  So we need
	to consider the max of the absolute value of the return codes
	here.
	* tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt:
	Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 ...7-self-compare-from-fc23-dbus-glib-report-0.txt |  6 ++++
 tools/fedabipkgdiff                                | 39 ++++++++++++++--------
 2 files changed, 31 insertions(+), 14 deletions(-)
  

I like the concept of fedabipkgdiff and abipkgdiff —self-check but I think that it really should NOT suppress harmless changes when doing a self-check. When comparing a library to itself “harmless” changes are indicative of changes not captured faithfully enough in either the abixml or the intermediate representation to represent the full ABI. The fact that in the context of comparing a library to itself libabigail concludes that these changes happen to be harmless is just luck.

-ben

> On Nov 24, 2020, at 2:34 AM, Dodji Seketeli via Libabigail <libabigail@sourceware.org> wrote:
> 
> Hello,
> 
> Now that the abipkgdiff program supports the --self-check option to
> make it compare the binaries in an RPM against their own ABIXML
> representation, make the --self-compare option of fedabipkgdiff use
> the abipkgdiff --self-check.
> 
> 	* tools/fedabipkgdiff (abipkgdiff): If the user provides the
> 	--self-compare options, generate the abipkgdiff command by using
> 	the --self-check option.
> 	(run_abipkgdiff): Each return value of the abipkgidiff runs can be
> 	negative because they are unsigned values in essence, but as
> 	python doesn't seem to have a unsigned integer type.  So we need
> 	to consider the max of the absolute value of the return codes
> 	here.
> 	* tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt:
> 	Adjust.
> 
> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
> ---
> ...7-self-compare-from-fc23-dbus-glib-report-0.txt |  6 ++++
> tools/fedabipkgdiff                                | 39 ++++++++++++++--------
> 2 files changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt b/tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt
> index 63a6dca..e037e7e 100644
> --- a/tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt
> +++ b/tests/data/test-fedabipkgdiff/test7-self-compare-from-fc23-dbus-glib-report-0.txt
> @@ -1,12 +1,18 @@
> Comparing the ABI of binaries between dbus-glib-0.106-1.fc23.i686.rpm and dbus-glib-0.106-1.fc23.i686.rpm:
> 
> +==== SELF CHECK SUCCEEDED for 'dbus-binding-tool' ====
> +==== SELF CHECK SUCCEEDED for 'libdbus-glib-1.so.2.3.3' ====
> 
> Comparing the ABI of binaries between dbus-glib-devel-0.106-1.fc23.i686.rpm and dbus-glib-devel-0.106-1.fc23.i686.rpm:
> 
> +==== SELF CHECK SUCCEEDED for 'dbus-bash-completion-helper' ====
> 
> Comparing the ABI of binaries between dbus-glib-0.106-1.fc23.x86_64.rpm and dbus-glib-0.106-1.fc23.x86_64.rpm:
> 
> +==== SELF CHECK SUCCEEDED for 'dbus-binding-tool' ====
> +==== SELF CHECK SUCCEEDED for 'libdbus-glib-1.so.2.3.3' ====
> 
> Comparing the ABI of binaries between dbus-glib-devel-0.106-1.fc23.x86_64.rpm and dbus-glib-devel-0.106-1.fc23.x86_64.rpm:
> 
> +==== SELF CHECK SUCCEEDED for 'dbus-bash-completion-helper' ====
> 
> diff --git a/tools/fedabipkgdiff b/tools/fedabipkgdiff
> index 2191613..06ab573 100755
> --- a/tools/fedabipkgdiff
> +++ b/tools/fedabipkgdiff
> @@ -1118,19 +1118,30 @@ def abipkgdiff(cmp_half1, cmp_half2):
>     else:
>         debuginfo_pkg2 = format_debug_info_pkg_options("--d2", cmp_half2.ancillary_debug);
> 
> -    cmd = [
> -        abipkgdiff_tool,
> -        suppressions,
> -        '--show-identical-binaries' if global_config.show_identical_binaries else '',
> -        '--no-default-suppression' if global_config.no_default_suppr else '',
> -        '--dso-only' if global_config.dso_only else '',
> -        debuginfo_pkg1,
> -        debuginfo_pkg2,
> -        devel_pkg1,
> -        devel_pkg2,
> -        cmp_half1.subject.downloaded_file,
> -        cmp_half2.subject.downloaded_file,
> -    ]
> +    cmd = []
> +
> +    if global_config.self_compare:
> +        cmd = [
> +            abipkgdiff_tool,
> +            '--dso-only' if global_config.dso_only else '',
> +            '--self-check',
> +            debuginfo_pkg1,
> +            cmp_half1.subject.downloaded_file,
> +        ]
> +    else:
> +        cmd = [
> +            abipkgdiff_tool,
> +            suppressions,
> +            '--show-identical-binaries' if global_config.show_identical_binaries else '',
> +            '--no-default-suppression' if global_config.no_default_suppr else '',
> +            '--dso-only' if global_config.dso_only else '',
> +            debuginfo_pkg1,
> +            debuginfo_pkg2,
> +            devel_pkg1,
> +            devel_pkg2,
> +            cmp_half1.subject.downloaded_file,
> +            cmp_half2.subject.downloaded_file,
> +        ]
>     cmd = [s for s in cmd if s != '']
> 
>     if global_config.dry_run:
> @@ -1193,7 +1204,7 @@ def run_abipkgdiff(rpm_col1, rpm_col2):
>     return_codes = [
>         abipkgdiff(cmp_half1, cmp_half2) for cmp_half1, cmp_half2
>         in generate_comparison_halves(rpm_col1, rpm_col2)]
> -    return max(return_codes) if return_codes else 0
> +    return max(return_codes, key=abs) if return_codes else 0
> 
> 
> @log_call
> -- 
> 1.8.3.1
> 
> 
> -- 
> 		Dodji
>