From owner-svn-src-head@freebsd.org Tue Sep 22 21:53:28 2020 Return-Path: Delivered-To: svn-src-head@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 02F4F3E1190 for ; Tue, 22 Sep 2020 21:53:28 +0000 (UTC) (envelope-from tsoome@me.com) Received: from pv50p00im-zteg10011501.me.com (pv50p00im-zteg10011501.me.com [17.58.6.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4Bww6b0tPvz3V6h for ; Tue, 22 Sep 2020 21:53:26 +0000 (UTC) (envelope-from tsoome@me.com) Received: from nazgul.lan (148-52-235-80.sta.estpak.ee [80.235.52.148]) by pv50p00im-zteg10011501.me.com (Postfix) with ESMTPSA id 8DA41B0056D; Tue, 22 Sep 2020 21:53:21 +0000 (UTC) From: Toomas Soome Message-Id: <84F5E4BD-8492-4297-97D6-88E0CE9E9546@me.com> Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\)) Subject: Re: svn commit: r365643 - head/bin/cp Date: Wed, 23 Sep 2020 00:53:19 +0300 In-Reply-To: Cc: Alan Somers , Kyle Evans , Mateusz Guzik , src-committers , svn-src-all , svn-src-head To: Warner Losh References: <202009112049.08BKnavL032212@repo.freebsd.org> X-Mailer: Apple Mail (2.3608.120.23.2.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-09-22_21:2020-09-21, 2020-09-22 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-2006250000 definitions=main-2009220168 X-Rspamd-Queue-Id: 4Bww6b0tPvz3V6h X-Spamd-Bar: / X-Spamd-Result: default: False [-0.92 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; RWL_MAILSPIKE_GOOD(0.00)[17.58.6.42:from]; FREEMAIL_FROM(0.00)[me.com]; MV_CASE(0.50)[]; R_SPF_ALLOW(-0.20)[+ip4:17.58.0.0/16]; URI_COUNT_ODD(1.00)[3]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[me.com:+]; DMARC_POLICY_ALLOW(-0.50)[me.com,quarantine]; RCPT_COUNT_SEVEN(0.00)[7]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; FREEMAIL_ENVFROM(0.00)[me.com]; ASN(0.00)[asn:714, ipnet:17.58.0.0/20, country:US]; MID_RHS_MATCH_FROM(0.00)[]; DWL_DNSWL_NONE(0.00)[me.com:dkim]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.97)[-0.974]; R_DKIM_ALLOW(-0.20)[me.com:s=1a1hai]; FREEFALL_USER(0.00)[tsoome]; FROM_HAS_DN(0.00)[]; NEURAL_SPAM_SHORT(0.60)[0.596]; NEURAL_HAM_LONG(-1.04)[-1.038]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; RECEIVED_SPAMHAUS_PBL(0.00)[80.235.52.148:received]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[17.58.6.42:from]; FREEMAIL_CC(0.00)[freebsd.org,gmail.com]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[svn-src-head]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.33 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Sep 2020 21:53:28 -0000 > On 23. Sep 2020, at 00:50, Warner Losh wrote: >=20 > I think it's a great leap sideways, but I've done cp /dev/null foo to = clear it out for 35 years now... It's why it feels like a workaround. >=20 > Though it is a legit optimization, no matter the feelings. As for = clearer, I'm less sure since then I have to remember what the : operator = does. >=20 > Warner you can always use /bin/true instead (unless its gnu;) or just echo. rgds, toomas >=20 > On Tue, Sep 22, 2020 at 3:48 PM Alan Somers > wrote: > It doesn't feel like a workaround to me. I think Kyle's version is = clearer than the original. >=20 > On Tue, Sep 22, 2020 at 3:45 PM Warner Losh > wrote: >=20 >=20 > On Tue, Sep 22, 2020 at 3:42 PM Kyle Evans > wrote: > cp is already fixed, people are still feeling the fallout of being > within those revisions and needing to bootstrap their own cp. We can > reduce the number of components these invocations rely on trivially to > shell built-in mechanics, why not do so? >=20 > Fair point. I just bristle at putting workarounds in for valid /bin/sh = syntax because we opposed for a few days. so long as it's an = unconditional clearing of the file to be zero length, I'm OK with that. >=20 > Warner > =20 > On Tue, Sep 22, 2020 at 4:40 PM Warner Losh > wrote: > > > > So why do we need a workaround at all? cp /dev/null has been fixed, = and that's way more important to get right. > > > > I don't want to paper-over issues with this at all, though if we use = the host's (now broken) cp, I suppose that might be OK in the short = term. If that's the case, then maybe this is OK. > > > > Otherwise, I'd strongly prefer we fix cp. > > > > Warner > > > > On Tue, Sep 22, 2020 at 3:31 PM Alan Somers > wrote: > >> > >> +1. > >> > >> On Tue, Sep 22, 2020 at 3:27 PM Kyle Evans > wrote: > >>> > >>> I'm running a build at the suggestion of mjg to confirm there = aren't > >>> any others hiding that can be converted, and I will commit after = I've > >>> verified that this is it. > >>> > >>> On Tue, Sep 22, 2020 at 4:02 PM Alan Somers > wrote: > >>> > > >>> > LGTM. > >>> > > >>> > On Tue, Sep 22, 2020 at 2:59 PM Kyle Evans > wrote: > >>> >> > >>> >> Perhaps: > >>> >> > >>> >> diff --git a/stand/i386/zfsboot/Makefile = b/stand/i386/zfsboot/Makefile > >>> >> index ff315abc0ef..7e362b43a39 100644 > >>> >> --- a/stand/i386/zfsboot/Makefile > >>> >> +++ b/stand/i386/zfsboot/Makefile > >>> >> @@ -81,7 +81,7 @@ zfsboot.ld: zfsboot.ldr zfsboot.bin = ${BTXKERN} > >>> >> -o ${.TARGET} -P 1 zfsboot.bin > >>> >> > >>> >> zfsboot.ldr: > >>> >> - cp /dev/null ${.TARGET} > >>> >> + :> ${.TARGET} > >>> >> > >>> >> zfsboot.bin: zfsboot.out > >>> >> ${OBJCOPY} -S -O binary zfsboot.out ${.TARGET} > >>> >> diff --git a/stand/libsa/Makefile b/stand/libsa/Makefile > >>> >> index effece9e01b..63cd46a9c54 100644 > >>> >> --- a/stand/libsa/Makefile > >>> >> +++ b/stand/libsa/Makefile > >>> >> @@ -122,7 +122,7 @@ beforedepend: > >>> >> ln -sf ${SRCTOP}/include/arpa/inet.h arpa/inet.h; \ > >>> >> ln -sf ${SRCTOP}/include/arpa/tftp.h arpa/tftp.h; \ > >>> >> for i in _time.h _strings.h _string.h; do \ > >>> >> - [ -f xlocale/$$i ] || cp /dev/null xlocale/$$i; = \ > >>> >> + [ -f xlocale/$$i ] || :> xlocale/$$i; \ > >>> >> done; \ > >>> >> for i in ${STAND_H_INC}; do \ > >>> >> ln -sf ${SASRC}/stand.h $$i; \ > >>> >> > >>> >> > >>> >> On Tue, Sep 22, 2020 at 3:58 PM Alan Somers = > wrote: > >>> >> > > >>> >> > Looks like two places in stand. Is there any reason why = Mateusz's suggestion wouldn't work? > >>> >> > > >>> >> > > rg -g Makefile 'cp.*/dev/null' > >>> >> > stand/libsa/Makefile > >>> >> > 125: [ -f xlocale/$$i ] || cp /dev/null xlocale/$$i; \ > >>> >> > > >>> >> > stand/i386/zfsboot/Makefile > >>> >> > 82: cp /dev/null ${.TARGET} > >>> >> > > >>> >> > On Tue, Sep 22, 2020 at 2:54 PM Mateusz Guzik = > wrote: > >>> >> >> > >>> >> >> Can we instead add a workaround to the build tree? > >>> >> >> > >>> >> >> Where is cp /dev/null coming from anyway? Perhaps this can = be patched > >>> >> >> to touch the target file. > >>> >> >> > >>> >> >> On 9/22/20, Alan Somers > wrote: > >>> >> >> > On Tue, Sep 22, 2020 at 2:48 PM Kyle Evans = > wrote: > >>> >> >> > > >>> >> >> >> On Fri, Sep 11, 2020 at 3:49 PM Alan Somers = > wrote: > >>> >> >> >> > > >>> >> >> >> > Author: asomers > >>> >> >> >> > Date: Fri Sep 11 20:49:36 2020 > >>> >> >> >> > New Revision: 365643 > >>> >> >> >> > URL: https://svnweb.freebsd.org/changeset/base/365643 = > >>> >> >> >> > > >>> >> >> >> > Log: > >>> >> >> >> > cp: fall back to read/write if copy_file_range fails > >>> >> >> >> > > >>> >> >> >> > Even though copy_file_range has a file-system = agnostic version, it > >>> >> >> >> still > >>> >> >> >> > fails on devfs (perhaps because the file descriptor = is non-seekable?) > >>> >> >> >> In > >>> >> >> >> > that case, fallback to old-fashioned read/write. = Fixes > >>> >> >> >> > "cp /dev/null /tmp/null" > >>> >> >> >> > > >>> >> >> >> > >>> >> >> >> Hi, > >>> >> >> >> > >>> >> >> >> Any objection to adding a quick UPDATING entry for this? = I'm seeing > >>> >> >> >> occasional reports of this breakage as recent as today on = IRC from > >>> >> >> >> folks that were a little bit thrown off by this because = it throws up > >>> >> >> >> fairly far into the build and looks like a stand build = regression > >>> >> >> >> instead of a cp regression. > >>> >> >> >> > >>> >> >> >> Thanks, > >>> >> >> >> > >>> >> >> >> Kyle Evans > >>> >> >> >> > >>> >> >> > > >>> >> >> > No objection. Can you suggest the proper wording? > >>> >> >> > _______________________________________________ > >>> >> >> > svn-src-all@freebsd.org = mailing list > >>> >> >> > https://lists.freebsd.org/mailman/listinfo/svn-src-all = > >>> >> >> > To unsubscribe, send any mail to = "svn-src-all-unsubscribe@freebsd.org = " > >>> >> >> > > >>> >> >> > >>> >> >> > >>> >> >> -- > >>> >> >> Mateusz Guzik >