From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DB84A45AC8; Sun, 6 Oct 2024 16:17:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 694614025F; Sun, 6 Oct 2024 16:17:12 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 698C44025D for ; Sun, 6 Oct 2024 16:17:10 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id F28CED5AE for ; Sun, 6 Oct 2024 16:17:09 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id E5C31D542; Sun, 6 Oct 2024 16:17:09 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.2 Received: from [192.168.1.85] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 23622D540; Sun, 6 Oct 2024 16:17:06 +0200 (CEST) Message-ID: <2ee279b5-910f-4428-84e0-6a14dd4dfd5c@lysator.liu.se> Date: Sun, 6 Oct 2024 16:17:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v12 1/7] buildtools/chkincs: relax C linkage requirement To: Robin Jarry , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , dev@dpdk.org Cc: Heng Wang , Stephen Hemminger , Tyler Retzlaff , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Jack Bond-Preston , David Marchand , Chengwen Feng References: <20240920062437.738706-2-mattias.ronnblom@ericsson.com> <20240920104754.739033-1-mattias.ronnblom@ericsson.com> <20240920104754.739033-2-mattias.ronnblom@ericsson.com> <74e86a66-4341-465d-bcba-d76db66035cd@lysator.liu.se> <722ddf9d-29ed-4d3b-bc54-ac2aafefd2bb@lysator.liu.se> <6fd9040d-d923-42e2-8eab-b7ad5e48e28c@lysator.liu.se> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2024-10-06 14:25, Robin Jarry wrote: > Mattias Rönnblom, Oct 06, 2024 at 11:47: >> OK, so it does change something, but not for the better, seemingly. >> >> meson.build now somehow has to distinguished between two different >> "errors": >> 1) One or more offending header files were found. >> 2) Any other kind of error (file not found, access control issues, >> internal script error, etc). >> >> For 1), it should print a list of the offending header files. >> >> If you just set "check: true" in run_command(), and have the >> chkextern.py script return non-zero on error, the offending header >> files will not be printed on stdout. >> >> My guess would be that this complicating factor is the reason for this >> pattern of not (in/from meson) depending on the script delegate exit >> code, but rather the output. >> >> May I should rename the script to "scanextern.py" to better reflect >> that a failure to find an issue is not an error (=non-zero exit code). > > I think you need to change run_command() to custom_target(). I was > thinking of this patch to be much simpler as follows: I started off with a shell script, but I switched to Python when I wanted to strip off comments. (Not that that can't be done in bourne shell.) The below script doesn't strip off comments, and provides no usage information. Earlier I got the impression you wanted to improve command-line usage. I think we need to decide if this thing is an integral part of the build system, custom-made for its needs, or if its something that should also be friendly to a command-line human user. I could go either way on that. > > diff --git a/buildtools/chkincs/check_extern_c.sh b/buildtools/chkincs/ > check_extern_c.sh > new file mode 100755 > index 000000000000..96ace5ffd248 > --- /dev/null > +++ b/buildtools/chkincs/check_extern_c.sh > @@ -0,0 +1,49 @@ > +#!/bin/sh > +# SPDX-License-Identifier: BSD-3-Clause > + > +has_symbols() { > +    grep -Eq \ > +        -e 'rte_[a-z0-9_]+\(' \ > +        -e 'cmdline_[a-z0-9_]+\(' \ > +        -e 'vt100_[a-z0-9_]+\(' \ > +        -e 'rdline_[a-z0-9_]+\(' \ > +        -e 'cirbuf_[a-z0-9_]+\(' \ > +        -e 'pthread_[a-z0-9_]+\(' \ > +        -e 'regcomp\(' \ > +        -e 'count_cpu\(' \ > +        -e '^extern[[:space:]]+[a-z0-9_]+[[:space:]]' \ > +        "$1" > +} > + > +has_extern_c() { > +    grep -q 'extern "C"' "$1" > +} > + > +mode=$1 > +shift > +ret=0 > + > +case "$mode" in Here the shell script purist notes that getopt(1) is not used. :) > +--missing) > +    for header in "$@"; do > +        if has_symbols "$header" && ! has_extern_c "$header"; then > +            echo "error: missing extern \"C\": $header" >&2 > +            ret=1 > +        fi > +    done > +    ;; > +--redundant) > +    for header in "$@"; do > +        if ! has_symbols "$header" && has_extern_c "$header"; then > +            echo "error: missing extern \"C\": $header" >&2 > +            exit 1 > +        fi > +    done > +    ;; > +*) > +    echo "error: unsupported mode '$mode'" >&2 > +    ret=1 > +esac > + > +exit $ret > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/ > meson.build > index f2dadcae18ef..b463845083eb 100644 > --- a/buildtools/chkincs/meson.build > +++ b/buildtools/chkincs/meson.build > @@ -38,14 +38,11 @@ if not add_languages('cpp', required: false) > endif > > # check for extern C in files, since this is not detected as an error by > the compiler > -grep = find_program('grep', required: false) > -if grep.found() > -    errlist = run_command([grep, '--files-without-match', '^extern > "C"', dpdk_chkinc_headers], > -            check: false, capture: true).stdout().split() > -    if errlist != [] > -        error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- > '.join(errlist)) > -    endif > -endif > +custom_target('check-extern-c', > +        command: files('check_extern_c.sh') + ['--missing', '@INPUT@'], > +        input: dpdk_chkinc_headers, > +        output: 'check-extern-c', > +        build_by_default: true) > > gen_cpp_files = generator(gen_c_file_for_header, >         output: '@BASENAME@.cpp', >