From owner-svn-ports-all@freebsd.org Thu Nov 19 23:03:21 2020 Return-Path: Delivered-To: svn-ports-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 376A9476B9E; Thu, 19 Nov 2020 23:03:21 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mail04.stack.nl (blade.stack.nl [51.15.111.152]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "*.stack.nl", Issuer "Sectigo RSA Domain Validation Secure Server CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CcZwR4tHrz3lj2; Thu, 19 Nov 2020 23:03:19 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from localhost (localhost.localdomain [127.0.0.1]) by mail04.stack.nl (Postfix) with ESMTP id A2B8DA8D; Thu, 19 Nov 2020 23:03:12 +0000 (UTC) Received: from mail04.stack.nl ([127.0.0.1]) by localhost (mail04.stack.nl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cLRw0oiP7O-a; Thu, 19 Nov 2020 23:03:10 +0000 (UTC) Received: from blade.stack.nl (blade.stack.nl [192.168.122.130]) by mail04.stack.nl (Postfix) with ESMTP id 006801C9; Thu, 19 Nov 2020 23:03:09 +0000 (UTC) Received: by blade.stack.nl (Postfix, from userid 1677) id CB04820765; Fri, 20 Nov 2020 00:03:09 +0100 (CET) Date: Fri, 20 Nov 2020 00:03:09 +0100 From: Jilles Tjoelker To: Bryan Drewery Cc: Mathieu Arnold , ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org, Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?= Subject: Re: svn commit: r554893 - head/Mk/Scripts Message-ID: <20201119230309.GA10938@stack.nl> References: <202011111329.0ABDTqUD035770@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-Rspamd-Queue-Id: 4CcZwR4tHrz3lj2 X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of jilles@stack.nl designates 51.15.111.152 as permitted sender) smtp.mailfrom=jilles@stack.nl X-Spamd-Result: default: False [-3.30 / 15.00]; MID_RHS_MATCH_FROM(0.00)[]; RBL_DBL_DONT_QUERY_IPS(0.00)[51.15.111.152:from]; FREEFALL_USER(0.00)[jilles]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:51.15.111.152]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[stack.nl]; ARC_NA(0.00)[]; RCPT_COUNT_FIVE(0.00)[6]; SPAMHAUS_ZRD(0.00)[51.15.111.152:from:127.0.2.255]; RCVD_COUNT_THREE(0.00)[4]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-0.999]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:12876, ipnet:51.15.0.0/17, country:FR]; RCVD_TLS_LAST(0.00)[]; MAILMAN_DEST(0.00)[svn-ports-all,svn-ports-head] X-BeenThere: svn-ports-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the ports tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2020 23:03:21 -0000 On Thu, Nov 19, 2020 at 11:41:57AM -0800, Bryan Drewery wrote: > On 11/11/2020 5:29 AM, Mathieu Arnold wrote: > > Author: mat > > Date: Wed Nov 11 13:29:52 2020 > > New Revision: 554893 > > URL: https://svnweb.freebsd.org/changeset/ports/554893 > > Log: > > Add set pipefail in most framework scripts. > > set pipefail changes the pipeline return status from being the return > > status of the last command to the last non 0 exit status of any command > > in the pipeline. This is needed to make sure all the commands in a > > pipeline did actually return a non 0 status and not only the last one. > [snip] > > Modified: head/Mk/Scripts/check-desktop-entries.sh > > ============================================================================== > > --- head/Mk/Scripts/check-desktop-entries.sh Wed Nov 11 13:24:31 2020 (r554892) > > +++ head/Mk/Scripts/check-desktop-entries.sh Wed Nov 11 13:29:52 2020 (r554893) > > @@ -4,6 +4,7 @@ > > # MAINTAINER: portmgr@FreeBSD.org > > > > set -e > > +set -o pipefail > > > > . "${dp_SCRIPTSDIR}/functions.sh" > > > > > This can prevent someone from upgrading from an unsupported release. The > workaround is simple enough so I think we should take it rather than > create burdens for people. > command set -o pipefail 2>/dev/null || : Hmm, does an upgrade require building ports on an old release? When scripts are written for use with 'set -o pipefail', allowing them to run without it seems unwise. Apart from that, I notice that the affected scripts contain various pipelines where the sink terminates early so that a source might receive SIGPIPE. For example in Mk/Scripts/qa.sh if ! readelf -d "${dep_file}" | grep -q SONAME; then If ${dep_file} is a really bloated library with lots of dependencies and system load is high, it is possible that the pipe will not accept all output at once, grep exits before reading all output and readelf receives SIGPIPE as it attempts to write the rest. In this case, the ${dep_file} will be incorrectly considered as not having a SONAME. In this particular case, not enabling pipefail happens to lead to the expected result, since the expected error cases of readelf (no such file or directory, invalid ELF file) do not cause any output to be written to stdout. However, reasoning like this may not be a tenable approach at scale. An alternative is to use extra code to change a SIGPIPE exit to 0, like what I included in the commit message when I added set -o pipefail to sh: https://svnweb.freebsd.org/base?view=revision&revision=344502 Some of this code could be wrapped in a function so the calling code is not cluttered too much. There will be one additional fork for the process that modifies the exit status but not more ($(kill -l "$r") does not fork). Another alternative is to avoid commands that read a pipe partially. Most uses of 'grep -q' that read from pipes can be replaced by 'grep >/dev/null', and more generically 'cmd1' can be replaced by '{ cmd1 && cat >dev/null; }'. I expect the performance impact of the additional useless processing to be low. If the example I picked cannot actually cause trouble, there may be other cases. There are various uses of 'grep -q' that read from pipes, and I didn't search for things like 'head', 'grep -Fq' or 'while read' loops that read from pipes and may break early. -- Jilles Tjoelker