From: Dodji Seketeli <dodji@redhat.com>
To: libabigail@sourceware.org
Subject: [PATCH, applied to mainline] abicompat: Fix exit code in weak mode
Date: Fri, 15 Mar 2024 20:53:42 +0100 [thread overview]
Message-ID: <87plvv1dbt.fsf@redhat.com> (raw)
Hello,
It turns out the tool is almost always returning ABIDIFF_OK in weak
mode. Oops. Fixed thus. Also, update the test-abicompat.cc to test
for expected exit codes to catch this kind of regressions in the
future.
* tools/abicompat.cc (perform_compat_check_in_weak_mode): Do not
override the status code when doing the comparison in the reverse
direction.
(compare_expected_against_provided_functions)
(compare_expected_against_provided_variables): Set the status code
close to the detected diff. In the future, this might help us
provide finer grained status.
* tests/test-abicompat.cc (InOutSpec::status): Add a new data
member to represent the expected exit code.
(in_out_specs): Adjust the array of tests.
(main): If the actual exit code is different from the expected
one, then the test failed so let's report it.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to mainline.
---
tests/test-abicompat.cc | 42 ++++++++++++++++++++++++++++++++++++++---
tools/abicompat.cc | 40 +++++++++++++++++++--------------------
2 files changed, 59 insertions(+), 23 deletions(-)
diff --git a/tests/test-abicompat.cc b/tests/test-abicompat.cc
index a7fcc520..e9f00050 100644
--- a/tests/test-abicompat.cc
+++ b/tests/test-abicompat.cc
@@ -28,6 +28,7 @@
using std::string;
using std::cerr;
using std::cout;
+using abigail::tools_utils::abidiff_status;
struct InOutSpec
{
@@ -36,6 +37,7 @@ struct InOutSpec
const char* in_lib2_path;
const char* suppressions;
const char* options;
+ abidiff_status status;
const char* in_report_path;
const char* out_report_path;
};
@@ -48,6 +50,7 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest0-fn-changed-libapp-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test0-fn-changed-report-0.txt",
"output/test-abicompat/test0-fn-changed-report-0.txt",
},
@@ -57,6 +60,7 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest0-fn-changed-libapp-v1.so",
"data/test-abicompat/test0-fn-changed-0.suppr",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_OK,
"data/test-abicompat/test0-fn-changed-report-1.txt",
"output/test-abicompat/test0-fn-changed-report-1.txt",
},
@@ -66,6 +70,7 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest0-fn-changed-libapp-v1.so",
"",
"--show-base-names --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test0-fn-changed-report-2.txt",
"output/test-abicompat/test0-fn-changed-report-2.txt",
},
@@ -75,6 +80,7 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest0-fn-changed-libapp-v1.so",
"data/test-abicompat/test0-fn-changed-1.suppr",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_OK,
"data/test-abicompat/test0-fn-changed-report-3.txt",
"output/test-abicompat/test0-fn-changed-report-3.txt",
},
@@ -84,6 +90,8 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest1-fn-removed-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE
+ | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
"data/test-abicompat/test1-fn-removed-report-0.txt",
"output/test-abicompat/test1-fn-removed-report-0.txt",
},
@@ -93,6 +101,8 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest2-var-removed-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE
+ | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
"data/test-abicompat/test2-var-removed-report-0.txt",
"output/test-abicompat/test2-var-removed-report-0.txt",
},
@@ -102,6 +112,8 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest3-fn-removed-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE
+ | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
"data/test-abicompat/test3-fn-removed-report-0.txt",
"output/test-abicompat/test3-fn-removed-report-0.txt",
},
@@ -111,6 +123,8 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest4-soname-changed-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE
+ | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
"data/test-abicompat/test4-soname-changed-report-0.txt",
"output/test-abicompat/test4-soname-changed-report-0.txt",
},
@@ -120,6 +134,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --no-show-locs --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test5-fn-changed-report-0.txt",
"output/test-abicompat/test5-fn-changed-report-0.txt",
},
@@ -129,6 +144,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test5-fn-changed-report-1.txt",
"output/test-abicompat/test5-fn-changed-report-1.txt",
},
@@ -138,6 +154,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --no-show-locs --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test6-var-changed-report-0.txt",
"output/test-abicompat/test6-var-changed-report-0.txt",
},
@@ -147,6 +164,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test6-var-changed-report-1.txt",
"output/test-abicompat/test6-var-changed-report-1.txt",
},
@@ -156,6 +174,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test6-var-changed-report-2.txt",
"output/test-abicompat/test6-var-changed-report-2.txt",
},
@@ -165,6 +184,7 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest7-fn-changed-libapp-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test7-fn-changed-report-0.txt",
"output/test-abicompat/test7-fn-changed-report-0.txt",
},
@@ -175,6 +195,7 @@ InOutSpec in_out_specs[] =
"data/test-abicompat/libtest7-fn-changed-libapp-btf-v1.so",
"",
"--show-base-names --no-show-locs --no-redundant --btf",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test7-fn-changed-report-0.1.txt",
"output/test-abicompat/test7-fn-changed-report-0.1.txt",
},
@@ -185,6 +206,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --no-show-locs --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test7-fn-changed-report-1.txt",
"output/test-abicompat/test7-fn-changed-report-1.txt",
},
@@ -194,6 +216,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test7-fn-changed-report-2.txt",
"output/test-abicompat/test7-fn-changed-report-2.txt",
},
@@ -204,6 +227,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --no-show-locs --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test7-fn-changed-report-2.1.txt",
"output/test-abicompat/test7-fn-changed-report-2.1.txt",
},
@@ -214,6 +238,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test8-fn-changed-report-0.txt",
"output/test-abicompat/test8-fn-changed-report-0.txt",
},
@@ -223,6 +248,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names --weak-mode",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test9-fn-changed-report-0.txt",
"output/test-abicompat/test9-fn-changed-report-0.txt",
},
@@ -232,6 +258,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names",
+ abigail::tools_utils::ABIDIFF_OK,
"data/test-abicompat/test10/test10-fn-changed-report-0.txt",
"output/test-abicompat/test10/test10-fn-changed-report-0.txt",
},
@@ -241,6 +268,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names",
+ abigail::tools_utils::ABIDIFF_OK,
"data/test-abicompat/test10/test10-fn-changed-report-0.txt",
"output/test-abicompat/test10/test10-fn-changed-report-0.txt",
},
@@ -250,6 +278,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test10/test10-fn-changed-report-1.txt",
"output/test-abicompat/test10/test10-fn-changed-report-1.txt",
},
@@ -259,6 +288,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test10/test10-fn-changed-report-2.txt",
"output/test-abicompat/test10/test10-fn-changed-report-2.txt",
},
@@ -268,6 +298,7 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test10/test10-fn-changed-report-3.txt",
"output/test-abicompat/test10/test10-fn-changed-report-3.txt",
},
@@ -277,11 +308,12 @@ InOutSpec in_out_specs[] =
"",
"",
"--show-base-names",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
"data/test-abicompat/test10/test10-fn-changed-report-4.txt",
"output/test-abicompat/test10/test10-fn-changed-report-4.txt",
},
// This entry must be the last one.
- {0, 0, 0, 0, 0, 0, 0}
+ {0, 0, 0, 0, 0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};
int
@@ -334,8 +366,9 @@ main()
cmd += " > " + out_report_path;
bool abicompat_ok = true;
- abidiff_status status = static_cast<abidiff_status>(system(cmd.c_str()));
- if (abigail::tools_utils::abidiff_status_has_error(status))
+ int code = system(cmd.c_str());
+ abidiff_status status = static_cast<abidiff_status>(WEXITSTATUS(code));
+ if (status != s->status)
abicompat_ok = false;
if (abicompat_ok)
@@ -356,6 +389,9 @@ main()
<< DEFAULT_TERMINAL_COLOR
<< cmd
<< std::endl;
+ if (status != s->status)
+ cout << BRIGHT_RED_COLOR
+ << "expected abicompat exit code: " << s->status << ", got: " << status << std::endl;
cnt_failed++;
}
cnt_total++;
diff --git a/tools/abicompat.cc b/tools/abicompat.cc
index 9c4771c9..2860b24a 100644
--- a/tools/abicompat.cc
+++ b/tools/abicompat.cc
@@ -460,6 +460,7 @@ compare_expected_against_provided_functions(diff_context_sptr& ctxt,
vector<fn_change>& fn_changes,
bool reverse_direction)
{
+ abidiff_status status = abigail::tools_utils::ABIDIFF_OK;
for (auto expected_fn :
reverse_direction
? lib_corpus->get_sorted_undefined_functions()
@@ -483,19 +484,18 @@ compare_expected_against_provided_functions(diff_context_sptr& ctxt,
exported_fn->get_type(),
ctxt);
if (fn_type_diff && fn_type_diff->to_be_reported())
- // So there is a type change between the function
- // expected by the application and the function
- // exported by the library. Let's record that
- // change so that we can report about it later.
- fn_changes.push_back(fn_change(expected_fn, fn_type_diff, reverse_direction));
+ {
+ // So there is a type change between the function
+ // expected by the application and the function
+ // exported by the library. Let's record that
+ // change so that we can report about it later.
+ fn_changes.push_back(fn_change(expected_fn, fn_type_diff, reverse_direction));
+ status |= abigail::tools_utils::ABIDIFF_ABI_CHANGE;
+ }
}
}
}
- abidiff_status status = abigail::tools_utils::ABIDIFF_OK;
- if (!fn_changes.empty())
- status |= abigail::tools_utils::ABIDIFF_ABI_CHANGE;
-
return status;
}
@@ -534,6 +534,7 @@ compare_expected_against_provided_variables(diff_context_sptr& ctxt,
vector<var_change>& var_changes,
bool reverse_direction)
{
+ abidiff_status status = abigail::tools_utils::ABIDIFF_OK;
for (auto expected_var :
reverse_direction
? lib_corpus->get_sorted_undefined_variables()
@@ -555,18 +556,17 @@ compare_expected_against_provided_variables(diff_context_sptr& ctxt,
exported_var->get_type(),
ctxt);
if (type_diff && type_diff->to_be_reported())
- // So there is a type change between the variable
- // expected by the application and the variable
- // exported by the library. Let's record that
- // change so that we can report about it later.
- var_changes.push_back(var_change(expected_var, type_diff, reverse_direction));
+ {
+ // So there is a type change between the variable
+ // expected by the application and the variable
+ // exported by the library. Let's record that
+ // change so that we can report about it later.
+ var_changes.push_back(var_change(expected_var, type_diff, reverse_direction));
+ status |= abigail::tools_utils::ABIDIFF_ABI_CHANGE;
+ }
}
}
- abidiff_status status = abigail::tools_utils::ABIDIFF_OK;
- if (!var_changes.empty())
- status |= abigail::tools_utils::ABIDIFF_ABI_CHANGE;
-
return status;
}
@@ -807,8 +807,8 @@ perform_compat_check_in_weak_mode(options& opts,
{
vector<fn_change> fn_changes;
- status = compare_expected_against_provided_functions(ctxt, app_corpus, lib_corpus,
- fn_changes, /*reverse_direction=*/true);
+ status |= compare_expected_against_provided_functions(ctxt, app_corpus, lib_corpus,
+ fn_changes, /*reverse_direction=*/true);
report_function_changes(opts, fn_changes);
}
--
2.39.3
--
Dodji
reply other threads:[~2024-03-15 19:53 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87plvv1dbt.fsf@redhat.com \
--to=dodji@redhat.com \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).