svn commit: r365643 - head/bin/cp

Warner Losh imp at bsdimp.com
Wed Sep 23 17:24:04 UTC 2020


On Wed, Sep 23, 2020, 10:56 AM Rodney W. Grimes <freebsd at gndrsh.dnsmgr.net>
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?
>
> I would even go further, I would like to see the dependency on
> /dev/null removed from the build, and the boot process.
>

A worthy goal, but one I'm afraid is out of our reach. A quick grep shows
just over 200 instances of /dev/null in the Makefile and mk file (800 if
you don't filter Makefile.in and Makefile.am). Maybe a third of those are
due to tests and other false positives. It would be quite the effort to
eliminate them all. And /dev/tty and /dev/zero likely will be troublesome
too, as they are used by running programs.

and how would you throw away output you know is bad / bogus without
/dev/null?

>From the build because it means I would no longer have to
> mount /dev in my chroots, and from the boot because I
> hate to say it, but we often scribble in /dev before
> devfs is mounted and if you look at root file systems
> mounted on other systems you well often find a /dev/null
> FILE that got created during the boot process from a >/dev/null
> before devfs was mounted.
>

But for this issue, we're not mounting devfs early enough.  We should fix
that. Removing /dev/null from the boot process likely is never going to
happen because we use it all over the place to discard output... There's
~200 instances of it in the boot rc scripts, so getting rid of it there
would also be quite the effort, with the same question.

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>
> >
>
> --
> Rod Grimes
> rgrimes at freebsd.org
>


More information about the svn-src-all mailing list