From owner-svn-src-head@freebsd.org Tue Sep 22 22:02:10 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 ECC5F3E1692 for ; Tue, 22 Sep 2020 22:02:10 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-lf1-x144.google.com (mail-lf1-x144.google.com [IPv6:2a00:1450:4864:20::144]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BwwJd4PMgz3Vyg for ; Tue, 22 Sep 2020 22:02:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-lf1-x144.google.com with SMTP id w11so19758208lfn.2 for ; Tue, 22 Sep 2020 15:02:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=AgXugTZS9/kCC60ZcqOXC/mJufzYm5WE0B36eTIyvLQ=; b=ql3UWQ0wYPRdug1d4iiqQgAIRJJ3d3ecYny+KFWhVRbSAIhQJclQVDkex+uCE/nJJ0 DX3ajRB60Fa7YQpNvJ1ln/sHyJ3OaOG143Psrhu5ANa3FthIpEVvK2gh2vkZxzKeFj3B FAaGxj5HPUnnQtt2Djgfa3/m9WQBl8qyJknQBDA/gGbZEFRdW2pwQzbSLlIombCsOJ7O QjHuuMexnIplAdbkmyHd8FqVF1RcMVI8LLtf3cYVgRRwpYwWaD9xQQNjKDB80o48lOYC cOe8Tn2c8gmBZuh9i6bDLWUMPI4GwRCZn0SzDGuAGeX4vMxzvx/DuDTM+d721pEDJTmk sNQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=AgXugTZS9/kCC60ZcqOXC/mJufzYm5WE0B36eTIyvLQ=; b=He8LN4RfR6hrXbZMJ89dUrN2anuEB8zdxg2ehHGZtiX6lQOl/48yd/Mwdq9NqeGsU0 AYf7YgL0+3CES2Mcw1AOnhWJL2QjoZBwU6EG9/JYYgrbcqczkbfimpYaw0OFEm8blv66 mABHiXoShSzwFnCkdAinejqEPThlQLYEePsg18kOBibvNPACh+SnIn+EWzVd8XQLBalz ZaRBcQNVzoOM+RyQntZaiimxhi5bW1a33JI0BQcO/NLuXXOd+8kMuqmiSojOzk75L80I SgXAvnQKU/zcVCiBvhPdLfIbatmlnPaAqGeSgiXycvb0Bt3mHSaJdFBxVLrVENtoAZd2 aMzg== X-Gm-Message-State: AOAM531tmYbDprSee/HenkO6QOZ5A6gQ1st3ZONZkNQMdM/Y87ydfLjq mtlnb6N3XuJRPqeG5GFVDVwdGuWteyC7huDapfYgdw== X-Google-Smtp-Source: ABdhPJwd8bwtchbJAyOANBjRxjDtTpfUJTw6rCiLPCuvrM5mPkbuxW18r7Kwjf0ZKM0Wf3eL4cQSbt/qiVjUwwHishw= X-Received: by 2002:ac2:4891:: with SMTP id x17mr2216860lfc.28.1600812127553; Tue, 22 Sep 2020 15:02:07 -0700 (PDT) MIME-Version: 1.0 References: <202009112049.08BKnavL032212@repo.freebsd.org> In-Reply-To: From: Warner Losh Date: Tue, 22 Sep 2020 16:01:56 -0600 Message-ID: Subject: Re: svn commit: r365643 - head/bin/cp To: Kyle Evans Cc: Ian Lepore , Alan Somers , Mateusz Guzik , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 4BwwJd4PMgz3Vyg X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=ql3UWQ0w; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2a00:1450:4864:20::144) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-1.50 / 15.00]; RCVD_TLS_ALL(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; NEURAL_HAM_MEDIUM(-0.87)[-0.872]; FROM_HAS_DN(0.00)[]; FREEMAIL_CC(0.00)[freebsd.org,gmail.com]; NEURAL_HAM_LONG(-0.94)[-0.942]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; NEURAL_SPAM_SHORT(0.31)[0.311]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; RCPT_COUNT_SEVEN(0.00)[7]; RCVD_IN_DNSWL_NONE(0.00)[2a00:1450:4864:20::144:from]; R_SPF_NA(0.00)[no SPF record]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; RCVD_COUNT_TWO(0.00)[2]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; MAILMAN_DEST(0.00)[svn-src-head] Content-Type: text/plain; charset="UTF-8" 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 22:02:11 -0000 On Tue, Sep 22, 2020 at 3:55 PM Kyle Evans wrote: > On Tue, Sep 22, 2020 at 4:53 PM Ian Lepore wrote: > > > > On Tue, 2020-09-22 at 15:50 -0600, Warner Losh wrote: > > > 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. > > > > > > 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. > > > > > > Warner > > > > > > > For me, :> is idiomatic (but ugly). > > > > On the other hand, the cp /dev/null had a nice dogfooding aspect to > > it... when we broke cp by accident, its use in the build system was the > > first alarm to go off. > > > > --Ian > > > > To be honest, this is a case that really should be covered by > regression tests somewhere. > It should (but isn't yet). Ian is right for old-school FreeBSD thinking. In that thinking the build system should use an eclectic mix of tools to act as a fire-wall against accidental breakage. Complete, effective, test suites give much better coverage... if they are run... So until we run tests frequently, with loud regression squawking that's as effective as build breakage, I tend to fall in the 'all of the above' camp until that's in place... :) Warner P.S. though not, if I suppose, if it means that we're slowing down the regression coverage uptake... > > 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. > > > > > > > > On Tue, Sep 22, 2020 at 3:45 PM Warner Losh wrote: > > > > > > > > > > > > > > > > > > > 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? > > > > > > > > > > > > > > > > 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. > > > > > > > > > > Warner > > > > > > > > > > > > > > > > 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 < > > > > > > > asomers@freebsd.org> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > +1. > > > > > > > > > > > > > > > > On Tue, Sep 22, 2020 at 3:27 PM Kyle Evans < > > > > > > > > kevans@freebsd.org> > > > > > > > > > > > > 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 < > > > > > > > > > asomers@freebsd.org> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > LGTM. > > > > > > > > > > > > > > > > > > > > On Tue, Sep 22, 2020 at 2:59 PM Kyle Evans < > > > > > > > > > > kevans@freebsd.org> > > > > > > > > > > > > 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/t > > > > > > > > > > > ftp.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 < > > > > > > > > > > > asomers@freebsd.org> > > > > > > > > > > > > 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 < > > > > > > > > > > > > mjguzik@gmail.com> 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 < > > > > > > > > > > > > kevans@freebsd.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Sep 11, 2020 at 3:49 PM Alan Somers < > > > > > > > > > > > > asomers@freebsd.org> 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 > > >