* [PATCH] fedabipkgdiff: make --self-compare use abipkgdiff --self-check
@ 2020-11-24 10:34 Dodji Seketeli
2020-11-24 21:43 ` Ben Woodard
0 siblings, 1 reply; 2+ messages in thread
From: Dodji Seketeli @ 2020-11-24 10:34 UTC (permalink / raw)
To: libabigail
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] fedabipkgdiff: make --self-compare use abipkgdiff --self-check
2020-11-24 10:34 [PATCH] fedabipkgdiff: make --self-compare use abipkgdiff --self-check Dodji Seketeli
@ 2020-11-24 21:43 ` Ben Woodard
0 siblings, 0 replies; 2+ messages in thread
From: Ben Woodard @ 2020-11-24 21:43 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Matthias Maennich via libabigail
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-24 21:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-24 10:34 [PATCH] fedabipkgdiff: make --self-compare use abipkgdiff --self-check Dodji Seketeli
2020-11-24 21:43 ` Ben Woodard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).