From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id E0CB23861005 for ; Sat, 17 Aug 2024 21:20:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E0CB23861005 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E0CB23861005 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=45.83.234.184 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723929619; cv=none; b=TqKKjKPk1th+B0e8mm/YUoklXZx3x0wINcTa3/dxrRam7Mij9d6cTr9L3u5tShYHoiqvEiCOHKLE9/u54uPNkT6f83VeOlPiYco1fFmPAHVm15y6CaraP+ENdk6nrhDU73WVhLIMhDGBaQPgHMyJaCLhVlTuHe3tRAsQSl26oRI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723929619; c=relaxed/simple; bh=Sogl02n3VBSzTUPyO5bfumtbYEUa4eufhkBOkyCl23s=; h=Date:From:To:Subject:Message-ID:MIME-Version; b=HyRouxJlOyUE2lYCvlWuc+QxqAfdtOlgByIZWRk2dSHvYRSvx09T59UT40R6LQX9VUYulRgWkbqnDYFj6ejOceuDC1nCgtcLMnsUVa9JjnwRSsPn66S4Io2eM+zZ0vbNVeT430CBPr+X0pk03zdVFlbMSMmclsCTONmm87I5z7A= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by gnu.wildebeest.org (Postfix, from userid 1000) id EBEFF303AA16; Sat, 17 Aug 2024 23:20:15 +0200 (CEST) Date: Sat, 17 Aug 2024 23:20:15 +0200 From: Mark Wielaard To: Keith Seitz Cc: debugedit@sourceware.org Subject: Re: [PATCH] find-debuginfo.sh: Exit with real exit status in parallel jobs Message-ID: <20240817212015.GA17272@gnu.wildebeest.org> References: <4120f4aea4b7f2cb5c30f55c19ae63a1e710df34.1723834460.git.keiths@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4120f4aea4b7f2cb5c30f55c19ae63a1e710df34.1723834460.git.keiths@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,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 List-Id: Hi Keith, On Fri, Aug 16, 2024 at 11:54:20AM -0700, Keith Seitz wrote: > Currently, when the script is executed in parallel (-jN), the > resulting exit status will always be 0. > > The script execs an appropriate number of clones of itself, calling > run_job to run the actual workload. This then calls do_file(), saving > the exit status into "res.$jobid". > > In do_file(), though, if an error occurs, exit is called. This causes > the entire exec'd shell to exit with status 0 (since there are almost > always echo calls as the last executed statement). The real exit > status is therefor never written to the "res.$jobid" files by run_job(). > > The simple solution is to use 'return' instead of 'exit'. A number > of minor adjustments are also made to propagate this properly so that > it is reported as the correct exit status. Good find. Code looks correct. Except don't we now also need to check the result of do_file () in the non-parallel case? Something like: diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in index f40e566f86c3..5998b9d1fa6c 100755 --- a/scripts/find-debuginfo.in +++ b/scripts/find-debuginfo.in @@ -579,6 +579,10 @@ fi if [ $n_jobs -le 1 ]; then while read nlinks inum f; do do_file "$nlinks" "$inum" "$f" + res=$? + if [ "$res" != "0" ]; then + exit $res + fi done <"$temp/primary" else for ((i = 1; i <= n_files; i++)); do > While at it, I've incorporated a patch for find-debuginfo/30505. > Using this patch and another patch to the RPM package (submitted as > github issue #3215), failures of gdb-add-index.sh will now properly fail > the build instead of being swallowed. It should be much easier for > developers to figure out why their builds have failed should gdb crash. Thanks, that has been a long standing issue. Smart to introduce a new _find_debuginfo_exit_on_error for rpm-config. The reason errors weren't reported in the past was because people would then disable creating debuginfo packages completely. But these days we really should report (and fix) any errors. There are a couple of other places, the main debugedit invocation, strip_to_debug, add_minidebug, which really should report errors too. But that can wait. Thanks, Mark