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 12B9F45AC6; Sun, 6 Oct 2024 11:47:42 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9BA9D4025F; Sun, 6 Oct 2024 11:47:41 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 8FBC94025D for ; Sun, 6 Oct 2024 11:47:40 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id D8427C939 for ; Sun, 6 Oct 2024 11:47:39 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id CBE0BC937; Sun, 6 Oct 2024 11:47:39 +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 72594C9C1; Sun, 6 Oct 2024 11:47:36 +0200 (CEST) Message-ID: <6fd9040d-d923-42e2-8eab-b7ad5e48e28c@lysator.liu.se> Date: Sun, 6 Oct 2024 11:47:35 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v12 1/7] buildtools/chkincs: relax C linkage requirement From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= 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> Content-Language: en-US In-Reply-To: <722ddf9d-29ed-4d3b-bc54-ac2aafefd2bb@lysator.liu.se> 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 10:55, Mattias Rönnblom wrote: > On 2024-10-04 13:51, Robin Jarry wrote: >> Mattias Rönnblom, Oct 04, 2024 at 10:40: >>> Making small scripts needlessly complicated is not good example, it's >>> a bad one. >> >> I don't find adding argument checks needlessly complicated but this is >> a matter of preference. To me, Python is not shell script. If you want >> something small, shell might be more appropriate? >> > > Python can serve in many roles. I suggest you be more pragmatic and > sensitive to the context. Sometimes the non-existence/non-use of doc > strings and other language features that makes sense in larger programs > is not a sign of the author being out to "treat Python badly". > >>>>> Sure, but it would be required to invoke the script for every >>>>> header file in the tree. Not sure I think that would be a net gain. >>>> >>>> You can store a global exit status in the script and process all >>>> headers before exiting with an error if any. >>> >>> You will need to give the user a list of offending header files. >> >> I'm not suggesting to avoid printing the offending file names. I'm >> only suggesting to exit(1) if there were *any* offending file names. >> That way you don't have to check *in meson* if the script did output >> anything. Checking the exit status is simpler. >> > > What you wrote was "/../ That way you would not have to capture stdout > at all and you could leave meson do the work.". > > Can you elaborate on this? I'm not sure I follow. Is there a way to not > capture the script output, invoke the script only once per build, and > yet produce a fine-grained error message to the user? > > I agree properly setting the exit code is an improvement. I just don't > see how that materially changes anything on the meson side of things. > But then, I know nothing about meson. > 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). >> Sorry for being pedantic, but Python code in DPDK is already treated >> badly. I wish we could improve the quality a bit. >> >> Cheers. >> >