From patchwork Fri Mar 15 19:53:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 87260 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B94C1385E449 for ; Fri, 15 Mar 2024 19:54:04 +0000 (GMT) X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 5EB46385E445 for ; Fri, 15 Mar 2024 19:53:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5EB46385E445 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5EB46385E445 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710532439; cv=none; b=CHCBH799cR2QZrz3ucPqsMnd2xRuPJLGVGNLIHY915alE87vcKLuxTFL90taFWx5FLCUOchVxq1mcfUe69kR2FZS/RaaBXCGBC11+Of+P5dGTqBikYcsUXNxbp8tS8rMI2lTXzx3DntzR6OLEzA7ockxVjSFYJiNnGEGDUYGYRY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710532439; c=relaxed/simple; bh=C0qKPq2QqAzFpdi3cx/C4EnAAMEyOS1oNX3hnxdN4Hw=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ADFkcdZQtFSgKtUYrYViykGKyXXAPWwqn9UKlNpHdGg4d6d/+SGk9bjiwXAHt3U2kZ3la/YvE9iAxXhYib+3iMEpnmisp3oGrt2ir4UGIj7oPOIUUtnBLQCQtlwDUXvjfmv9mavtwQqiJg8hM7vXKTBAAGlYFdpwZm9MAZqV/1U= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710532429; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=HRauK+2qCjvUiyG0jQqvu8pM7LxYZAwG+JjCy66WeQ0=; b=YbnYFVat2cHsOaUkVXf1kfv/UgU9qT6FpwFRFYzkgaf0AdpPJqGMrboeE5SajbDLghU8e3 nBWjy3lqcFNDF5ftpcaO/XJUxfJRyVs/Z8fhjerpJ1YEb8wpdlVFw1oUZsDTziiD+aINVJ N53yfUwfvtSbSspOh0eNtw5iH7h7OoE= Received: from mail-vs1-f71.google.com (mail-vs1-f71.google.com [209.85.217.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-45-95HfuBUePZyAdJRarNz-mg-1; Fri, 15 Mar 2024 15:53:47 -0400 X-MC-Unique: 95HfuBUePZyAdJRarNz-mg-1 Received: by mail-vs1-f71.google.com with SMTP id ada2fe7eead31-473084a6ce8so651923137.3 for ; Fri, 15 Mar 2024 12:53:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710532426; x=1711137226; h=mime-version:user-agent:message-id:date:organization:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=HRauK+2qCjvUiyG0jQqvu8pM7LxYZAwG+JjCy66WeQ0=; b=pYlGaGty3C2Ao1YzxyZR8h09Y1by3nsyypgaewI9jgIy1iq4k4YsNd1xst3yps+ooS 0052Js4PYBI+zYyrMzsB/8w8ZV/5J76+KH/BYH8xfU3vHfPpp4va65fj5fmT9wrM69Ny ochS76YsxzEgriO6A+MC5vg+Fbm+2Qlaw6T8tMo7cY5oqWotdOFYvK4gMQIuR0aslqe7 taxXKZWGBphAX+56ThK7mIf5pSAe6lo6/2QmVAuWx6e5jg2LWQpUYoevnmm/1nbiNzQL +kwLBBL9rGWIstJfFq4dekck7Qet1p3Dg3sO7zetY5tnxtUPgX93CBaVk+4BzSGjjtk0 Ugkg== X-Gm-Message-State: AOJu0YzWvpw9dlchCZB883KoqT41nf0AViH513p2gPXudd8LjBf1liar f7ZOkxfqdUNQPHQHSK0LReZ2jQu2dltdkPGp4ohQ7ij/ahcNJGGOO4gs2P+7v1r8Xa8lu+81C/J fv5vF8geFy8+0hCaW4wl1m+lqyoV8dfFuvswVXY3OnZWP2m3XewLoFIJrdXHBvqaQjQW9RfJOtr ku6uYzZtrlOmIuvT4P/6VS9a+OJ/xT1gE1H7vAWw== X-Received: by 2002:a67:fad1:0:b0:472:eca1:46c with SMTP id g17-20020a67fad1000000b00472eca1046cmr5883227vsq.30.1710532426020; Fri, 15 Mar 2024 12:53:46 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFml9+LqklpvC0DqRTtztVmelD3yeqWyOon5fHfG/Lcb9oVknvD4QoXJBjYm8KE8FcnZazk1g== X-Received: by 2002:a67:fad1:0:b0:472:eca1:46c with SMTP id g17-20020a67fad1000000b00472eca1046cmr5883201vsq.30.1710532425450; Fri, 15 Mar 2024 12:53:45 -0700 (PDT) Received: from localhost (88-120-130-27.subs.proxad.net. [88.120.130.27]) by smtp.gmail.com with ESMTPSA id iu11-20020ad45ccb000000b0068c8be959a0sm2319068qvb.111.2024.03.15.12.53.44 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Mar 2024 12:53:44 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 74DFC5071022; Fri, 15 Mar 2024 20:53:42 +0100 (CET) From: Dodji Seketeli To: libabigail@sourceware.org Subject: [PATCH, applied to mainline] abicompat: Fix exit code in weak mode Organization: Red Hat / France X-Operating-System: AlmaLinux 9.3 X-URL: http://www.redhat.com Date: Fri, 15 Mar 2024 20:53:42 +0100 Message-ID: <87plvv1dbt.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, RCVD_IN_SORBS_WEB, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org 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 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(system(cmd.c_str())); - if (abigail::tools_utils::abidiff_status_has_error(status)) + int code = system(cmd.c_str()); + abidiff_status status = static_cast(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_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_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_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); }