svn commit: r365643 - head/bin/cp

Alan Somers asomers at freebsd.org
Tue Sep 22 21:48:24 UTC 2020


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 <imp at bsdimp.com> wrote:

>
>
> On Tue, Sep 22, 2020 at 3:42 PM Kyle Evans <kevans at freebsd.org> 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 <imp at bsdimp.com> 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 at freebsd.org>
>> wrote:
>> >>
>> >> +1.
>> >>
>> >> On Tue, Sep 22, 2020 at 3:27 PM Kyle Evans <kevans at 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 at freebsd.org>
>> wrote:
>> >>> >
>> >>> > LGTM.
>> >>> >
>> >>> > On Tue, Sep 22, 2020 at 2:59 PM Kyle Evans <kevans at 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/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 <asomers at 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 at 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 <asomers at freebsd.org> wrote:
>> >>> >> >> > On Tue, Sep 22, 2020 at 2:48 PM Kyle Evans <
>> kevans at freebsd.org> wrote:
>> >>> >> >> >
>> >>> >> >> >> On Fri, Sep 11, 2020 at 3:49 PM Alan Somers <
>> asomers at 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 at freebsd.org mailing list
>> >>> >> >> > https://lists.freebsd.org/mailman/listinfo/svn-src-all
>> >>> >> >> > To unsubscribe, send any mail to "
>> svn-src-all-unsubscribe at freebsd.org"
>> >>> >> >> >
>> >>> >> >>
>> >>> >> >>
>> >>> >> >> --
>> >>> >> >> Mateusz Guzik <mjguzik gmail.com>
>>
>


More information about the svn-src-all mailing list