From d3ddf609d96f7387f7692b565c49cdf5aeeec94a Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Mon, 18 Sep 2017 16:00:23 +0200 Subject: [PATCH] Add a --leaf-changes-only option to abipkgdiff This patch adds the --leaf-changes-only option to abipkgdiff, just like what we have for abidiff. The patch also emit leaf changes report by default when comparing two Linux Kernel packages. The patch also adds the --impacted-interfaces and --full-impact options. * doc/manuals/abipkgdiff.rst: Add documentation for the new --leaf-change-only, --impacted-interfaces and --full-impact options. * tools/abipkgdiff.cc (options::{leaf_changes_only, show_impacted_interfaces, show_full_impact_report): Add new data members. (options::options): Initialize them. (display_usage): Add help strings for the new --leaf-change-only, --impacted-interfaces and --full-impact|-f options. (set_diff_context_from_opts): Set the diff context for the 'leaf-changes-only' and 'show-impacted-interfaces' flags. (parse_command_line): Parse the --leaf-change-only, --impacted-interfaces and --full-impact options. Handle the case where the --linux-kernel-abi-whitelist|-w option is given a whitelist *package*. * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt: New test output reference. * tests/test-diff-pkg.cc (in_out_spec): Compare data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64.rpm and data/test-diff-pkg/spice-server-0.12.8-1.el7.x86_64.rpm with the new --leaf-changes-only and --impacted-interfaces options, using the new output reference above. * tests/data/Makefile.am: Add the new test material to source distribution. Signed-off-by: Dodji Seketeli --- doc/manuals/abipkgdiff.rst | 99 +++++++++++++++++++ tests/data/Makefile.am | 1 + ...l7.x86_64-0.12.8-1.el7.x86_64-report-3.txt | 53 ++++++++++ tests/test-diff-pkg.cc | 12 +++ tools/abipkgdiff.cc | 43 +++++++- 5 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt diff --git a/doc/manuals/abipkgdiff.rst b/doc/manuals/abipkgdiff.rst index 606b6ae6..86d614a5 100644 --- a/doc/manuals/abipkgdiff.rst +++ b/doc/manuals/abipkgdiff.rst @@ -123,6 +123,101 @@ Options Compare ELF files that are shared libraries, only. Do not compare executable files, for instance. + * ``--leaf-changes-only|-l`` only show leaf changes, so don't show + impact analysis report. + + The typical output of ``abipkgdiff`` and ``abidiff`` when + comparing two binaries, that we shall call ``full impact report``, + looks like this :: + + $ abidiff libtest-v0.so libtest-v1.so + Functions changes summary: 0 Removed, 1 Changed, 0 Added function + Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + + 1 function with some indirect sub-type change: + + [C]'function void fn(C&)' at test-v1.cc:13:1 has some indirect sub-type changes: + parameter 1 of type 'C&' has sub-type changes: + in referenced type 'struct C' at test-v1.cc:7:1: + type size hasn't changed + 1 data member change: + type of 'leaf* C::m0' changed: + in pointed to type 'struct leaf' at test-v1.cc:1:1: + type size changed from 32 to 64 bits + 1 data member insertion: + 'char leaf::m1', at offset 32 (in bits) at test-v1.cc:4:1 + + $ + + So in that example the report emits information about how the data + member insertion change of "struct leaf" is reachable from + function "void fn(C&)". In other words, the report not only shows + the data member change on "struct leaf", but it also shows the + impact of that change on the function "void fn(C&)". + + In abidiff (and abipkgdiff) parlance, the change on "struct leaf" + is called a leaf change. So the ``--leaf-changes-only + --impacted-interfaces`` options show, well, only the leaf change. + And it goes like this: :: + + $ abidiff -l libtest-v0.so libtest-v1.so + 'struct leaf' changed: + type size changed from 32 to 64 bits + 1 data member insertion: + 'char leaf::m1', at offset 32 (in bits) at test-v1.cc:4:1 + + one impacted interface: + function void fn(C&) + $ + + Note how the report ends up by showing the list of interfaces + impacted by the leaf change. That's the effect of the additional + ``--impacted-interfaces`` option. + + Now if you don't want to see that list of impacted interfaces, + then you can just avoid using the ``--impacted-interface`` option. + You can learn about that option below, in any case. + + Please note that when comparing two Linux Kernel packages, it's + this ``leaf changes report`` that is emitted, by default. The + normal so-called ``full impact report`` can be emitted with the + option ``--full-impact`` which is documented later below. + + + * ``--impacted-interfaces`` + + When showing leaf changes, this option instructs abipkgdiff to + show the list of impacted interfaces. This option is thus to be + used in addition to the ``--leaf-changes-only`` option, or, when + comparing two Linux Kernel packages. Otherwise, it's simply + ignored. + + * ``--full-impact|-f`` + + When comparing two Linux Kernel packages, this function instructs + ``abipkgdiff`` to emit the so-called ``full impact report``, which + is the default report kind emitted by the ``abidiff`` tool: :: + + $ abidiff libtest-v0.so libtest-v1.so + Functions changes summary: 0 Removed, 1 Changed, 0 Added function + Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + + 1 function with some indirect sub-type change: + + [C]'function void fn(C&)' at test-v1.cc:13:1 has some indirect sub-type changes: + parameter 1 of type 'C&' has sub-type changes: + in referenced type 'struct C' at test-v1.cc:7:1: + type size hasn't changed + 1 data member change: + type of 'leaf* C::m0' changed: + in pointed to type 'struct leaf' at test-v1.cc:1:1: + type size changed from 32 to 64 bits + 1 data member insertion: + 'char leaf::m1', at offset 32 (in bits) at test-v1.cc:4:1 + + $ + + * ``--redundant`` In the diff reports, do display redundant changes. A redundant @@ -200,6 +295,10 @@ Options functions and global variables by the Linux Kernel binaries are compared. + Please note that if a white list package is given in parameter, + this option handles it just fine, like if the --wp option was + used. + * ``--wp`` <*path-to-whitelist-package*> When comparing two Linux kernel RPM packages, this option points diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 70da2968..72add0f2 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -1339,6 +1339,7 @@ test-diff-pkg/spice-server-devel-0.12.8-1.el7.x86_64.rpm \ test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-0.txt \ test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-1.txt \ test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt \ +test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt \ test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt \ test-diff-pkg/libcdio-0.94-1.fc26.x86_64.rpm \ test-diff-pkg/libcdio-0.94-2.fc26.x86_64.rpm \ diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt new file mode 100644 index 00000000..36cf212f --- /dev/null +++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt @@ -0,0 +1,53 @@ +================ changes of 'libspice-server.so.1.8.0'=============== +Leaf changes summary: 2 artifacts changed (8 filtered out) + Added/removed functions summary: 1 Removed, 8 Added functions + Added/removed variables summary: 0 Removed, 0 Added variable + + 1 Removed function: + + [D] 'function int spice_server_migrate_client_state(SpiceServer*)' {spice_server_migrate_client_state@@SPICE_SERVER_0.6.0} + + 8 Added functions: + + [A] 'function void spice_replay_free(SpiceReplay*)' {spice_replay_free@@SPICE_SERVER_0.12.6} + [A] 'function void spice_replay_free_cmd(SpiceReplay*, QXLCommandExt*)' {spice_replay_free_cmd@@SPICE_SERVER_0.12.6} + [A] 'function SpiceReplay* spice_replay_new(FILE*, int)' {spice_replay_new@@SPICE_SERVER_0.12.6} + [A] 'function QXLCommandExt* spice_replay_next_cmd(SpiceReplay*, QXLWorker*)' {spice_replay_next_cmd@@SPICE_SERVER_0.12.6} + [A] 'function uint32_t spice_server_get_best_playback_rate(SpicePlaybackInstance*)' {spice_server_get_best_playback_rate@@SPICE_SERVER_0.12.5} + [A] 'function uint32_t spice_server_get_best_record_rate(SpiceRecordInstance*)' {spice_server_get_best_record_rate@@SPICE_SERVER_0.12.5} + [A] 'function void spice_server_set_playback_rate(SpicePlaybackInstance*, uint32_t)' {spice_server_set_playback_rate@@SPICE_SERVER_0.12.5} + [A] 'function void spice_server_set_record_rate(SpiceRecordInstance*, uint32_t)' {spice_server_set_record_rate@@SPICE_SERVER_0.12.5} + + 'enum __anonymous_enum__' changed: + type size hasn't changed + 7 enumerator deletions: + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_INVALID' value '0' + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_OFF' value '1' + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_GLZ' value '2' + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_AUTO_LZ' value '3' + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_QUIC' value '4' + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_GLZ' value '5' + '__anonymous_enum__::SPICE_IMAGE_COMPRESS_LZ' value '6' + + 9 enumerator insertions: + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_INVALID' value '0' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_OFF' value '1' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_GLZ' value '2' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_AUTO_LZ' value '3' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_QUIC' value '4' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_GLZ' value '5' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ' value '6' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_LZ4' value '7' + 'SpiceImageCompression::SPICE_IMAGE_COMPRESSION_ENUM_END' value '8' + + 2 impacted interfaces: + function spice_image_compression_t spice_server_get_image_compression(SpiceServer*) + function int spice_server_set_image_compression(SpiceServer*, spice_image_compression_t) + 'typedef spice_image_compression_t' changed: + typedef name changed from spice_image_compression_t to SpiceImageCompression at enums.h:197:1 + + 2 impacted interfaces: + function spice_image_compression_t spice_server_get_image_compression(SpiceServer*) + function int spice_server_set_image_compression(SpiceServer*, spice_image_compression_t) +================ end of changes of 'libspice-server.so.1.8.0'=============== + diff --git a/tests/test-diff-pkg.cc b/tests/test-diff-pkg.cc index 4db8f23a..74dbcb66 100644 --- a/tests/test-diff-pkg.cc +++ b/tests/test-diff-pkg.cc @@ -498,6 +498,18 @@ static InOutSpec in_out_specs[] = "data/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt", "output/test-diff-pkg/libcdio-0.94-1.fc26.x86_64--libcdio-0.94-2.fc26.x86_64-report.1.txt" }, + { + "data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64.rpm", + "data/test-diff-pkg/spice-server-0.12.8-1.el7.x86_64.rpm", + "--no-default-suppression --leaf-changes-only --impacted-interfaces", + "", + "data/test-diff-pkg/spice-debuginfo-0.12.4-19.el7.x86_64.rpm", + "data/test-diff-pkg/spice-debuginfo-0.12.8-1.el7.x86_64.rpm", + "data/test-diff-pkg/spice-server-devel-0.12.4-19.el7.x86_64.rpm", + "data/test-diff-pkg/spice-server-devel-0.12.4-19.el7.x86_64.rpm", + "data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt", + "output/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-3.txt" + }, #endif //WITH_RPM #ifdef WITH_DEB diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc index f20c46cc..079844f8 100644 --- a/tools/abipkgdiff.cc +++ b/tools/abipkgdiff.cc @@ -159,6 +159,9 @@ public: bool no_default_suppression; bool keep_tmp_files; bool compare_dso_only; + bool leaf_changes_only; + bool show_impacted_interfaces; + bool show_full_impact_report; bool show_linkage_names; bool show_redundant_changes; bool show_harmless_changes; @@ -187,6 +190,9 @@ public: no_default_suppression(), keep_tmp_files(), compare_dso_only(), + leaf_changes_only(), + show_impacted_interfaces(), + show_full_impact_report(), show_linkage_names(true), show_redundant_changes(), show_harmless_changes(), @@ -646,6 +652,12 @@ display_usage(const string& prog_name, ostream& out) << " --wp path to a linux kernel abi whitelist package\n" << " --keep-tmp-files don't erase created temporary files\n" << " --dso-only compare shared libraries only\n" + << " --leaf-changes-only|-l only show leaf changes, " + "so no change impact analysis\n" + << " --impacted-interfaces|-i when in leaf mode, show " + "interfaces impacted by ABI changes\n" + << " --full-impact|-f when comparing kernel packages, show the " + "full impact analysis report rather than the default leaf changes reports\n" << " --no-linkage-name do not display linkage names of " "added/removed/changed\n" << " --redundant display redundant changes\n" @@ -978,6 +990,8 @@ set_diff_context_from_opts(diff_context_sptr ctxt, { ctxt->default_output_stream(&cout); ctxt->error_output_stream(&cerr); + ctxt->show_leaf_changes_only(opts.leaf_changes_only); + ctxt->show_impacted_interfaces(opts.show_impacted_interfaces); ctxt->show_relative_offset_changes(opts.show_relative_offset_changes); ctxt->show_redundant_changes(opts.show_redundant_changes); ctxt->show_locs(opts.show_locs); @@ -2406,6 +2420,15 @@ parse_command_line(int argc, char* argv[], options& opts) opts.keep_tmp_files = true; else if (!strcmp(argv[i], "--dso-only")) opts.compare_dso_only = true; + else if (!strcmp(argv[i], "--leaf-changes-only") + ||!strcmp(argv[i], "-l")) + opts.leaf_changes_only = true; + else if (!strcmp(argv[i], "--impacted-interfaces") + ||!strcmp(argv[i], "-i")) + opts.show_impacted_interfaces = true; + else if (!strcmp(argv[i], "--full-impact") + ||!strcmp(argv[i], "-f")) + opts.show_full_impact_report = true; else if (!strcmp(argv[i], "--no-linkage-name")) opts.show_linkage_names = false; else if (!strcmp(argv[i], "--redundant")) @@ -2451,7 +2474,15 @@ parse_command_line(int argc, char* argv[], options& opts) opts.wrong_option = argv[i]; return true; } - opts.kabi_whitelist_paths.push_back(argv[j]); + if (guess_file_type(argv[j]) == abigail::tools_utils::FILE_TYPE_RPM) + // The kernel abi whitelist is actually a whitelist + // *package*. Take that into account. + opts.kabi_whitelist_packages.push_back + (make_path_absolute(argv[j]).get()); + else + // We assume the kernel abi whitelist is a white list + // file. + opts.kabi_whitelist_paths.push_back(argv[j]); ++i; } else if (!strcmp(argv[i], "--wp")) @@ -2661,6 +2692,16 @@ main(int argc, char* argv[]) return (abigail::tools_utils::ABIDIFF_USAGE_ERROR | abigail::tools_utils::ABIDIFF_ERROR); } + // We are looking at kernel packages. If the user provided + // the --full-impact option then it means we want to display + // the default libabigail report format where a full impact + // analysis is done for each ABI change. + // + // Otherwise, let's just emit the leaf change report. + if (opts.show_full_impact_report) + opts.leaf_changes_only = false; + else + opts.leaf_changes_only = true; } break; -- 2.43.5