From nobody Sun Jul 9 00:29:40 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Qz7Ln3Zrhz4mHMg; Sun, 9 Jul 2023 00:29:53 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: from mail-ua1-f53.google.com (mail-ua1-f53.google.com [209.85.222.53]) (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 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Qz7Ln1y60z4KqW; Sun, 9 Jul 2023 00:29:53 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ua1-f53.google.com with SMTP id a1e0cc1a2514c-783eef15004so894644241.3; Sat, 08 Jul 2023 17:29:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688862592; x=1691454592; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=SEofZSY1SqM4g0GJuFpWDRnQR2ZvGeZt0RlSkyzOwrM=; b=a0JMN3Q97GJT5daK7+6Oj0FUFEICMGNY2u6P1Mf+EdUOQ5EvDWoQe5E0b3Ihm7NK6b /z0XfYh7VDQbwPTalpCts0DXj/DuMAyeAdF6XSS2dpgnSC7lMNbDE8RdoSmpVhd75vFd k+52j0+Y1/n7Bu3zQn27IYnKKY6oJ/ARwb1TSdcl85VZLCu1JJNtykbJRm0Pl5Ebnuxs KK+nOXxDI6jhrDsdql4xV9Y0XZgtxLLiwQAKdr1GKtiKHHUr5o56yURHfqoOjImlw88g +LzXLjVDEACT24LswvlttXMzRFyzSG3brkP+jyHsmBgseQ2j5GVML/YGE0e0pyXLFDj1 e9Pg== X-Gm-Message-State: ABy/qLZMTVkSYqB6cZlwgrwPmn8yx7FUvnWzvZhJVu9YNZGL0wlmkvbU p5O3OiraIKuVNQByBvTgzOuhg+bpCOA= X-Google-Smtp-Source: APBJJlH+FyIsbTS9l79DQUCdTMTLRlzcujWpvB2ekTXbcLr/NKbV3/13WBx4FROXzbS/1l0uGgg41w== X-Received: by 2002:a1f:5e4c:0:b0:47e:5744:21c5 with SMTP id s73-20020a1f5e4c000000b0047e574421c5mr2788495vkb.16.1688862592148; Sat, 08 Jul 2023 17:29:52 -0700 (PDT) Received: from mail-ua1-f41.google.com (mail-ua1-f41.google.com. [209.85.222.41]) by smtp.gmail.com with ESMTPSA id q22-20020a056122005600b00477429dda11sm694025vkn.55.2023.07.08.17.29.51 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 08 Jul 2023 17:29:51 -0700 (PDT) Received: by mail-ua1-f41.google.com with SMTP id a1e0cc1a2514c-7919342c456so894887241.2; Sat, 08 Jul 2023 17:29:51 -0700 (PDT) X-Received: by 2002:a1f:d0c2:0:b0:471:7d9d:99d5 with SMTP id h185-20020a1fd0c2000000b004717d9d99d5mr3253820vkg.5.1688862591522; Sat, 08 Jul 2023 17:29:51 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 References: <202307082348.368Nm3TN074666@gitrepo.freebsd.org> <471FD215-B307-4116-9E92-4D84D6FB2B66@freebsd.org> In-Reply-To: <471FD215-B307-4116-9E92-4D84D6FB2B66@freebsd.org> From: Alexander Richardson Date: Sat, 8 Jul 2023 17:29:40 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: git: e64780fbc394 - main - xinstall: do not use copy_file_range(2) when BOOTSTRAPPING To: Jessica Clarke Cc: Martin Matuska , src-committers , "" , dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000d7a004060002f6da" X-Rspamd-Queue-Id: 4Qz7Ln1y60z4KqW X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; TAGGED_FROM(0.00)[] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N --000000000000d7a004060002f6da Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks for the quick fix. I'm not sure if it matters for performance but it is a bit too restrictive: modern Linux also has the function so it works just fine there (GitHub CI was happy with this commit on Linux just not MacOS). And as Jess pointed out we should probably use it on FreeBSD as well. Alex On Sat, 8 Jul 2023, 17:18 Jessica Clarke, wrote: > On 9 Jul 2023, at 00:48, Martin Matuska wrote: > > > > The branch main has been updated by mm: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3De64780fbc394b10581e50a850cc06c1= c12a8e4f9 > > > > commit e64780fbc394b10581e50a850cc06c1c12a8e4f9 > > Author: Martin Matuska > > AuthorDate: 2023-07-08 23:24:38 +0000 > > Commit: Martin Matuska > > CommitDate: 2023-07-08 23:25:23 +0000 > > > > xinstall: do not use copy_file_range(2) when BOOTSTRAPPING > > > > Reported by: arichardson > > --- > > usr.bin/xinstall/Makefile | 5 +++++ > > usr.bin/xinstall/xinstall.c | 4 ++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/usr.bin/xinstall/Makefile b/usr.bin/xinstall/Makefile > > index 9969ef104e98..3b49cb39d27a 100644 > > --- a/usr.bin/xinstall/Makefile > > +++ b/usr.bin/xinstall/Makefile > > @@ -17,6 +17,11 @@ CFLAGS+=3D -I${SRCTOP}/lib/libnetbsd > > LIBADD=3D md > > CFLAGS+=3D -DWITH_MD5 -DWITH_RIPEMD160 > > > > +.ifdef BOOTSTRAPPING > > +# For the bootstrap we disable copy_file_range() > > +CFLAGS+=3D -DBOOTSTRAP_XINSTALL > > If the bootstrap xinstall is allowed to use copy_file_range on FreeBSD > (i.e. it=E2=80=99s guaranteed to exist) this is overly restrictive and sh= ould > be: > > .if defined(BOOTSTRAPPING) && ${.MAKE.OS} !=3D "FreeBSD" > > > +.endif > > + > > HAS_TESTS=3D > > SUBDIR.${MK_TESTS}+=3D tests > > > > diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c > > index 8dace862ef1e..6c269bbb5d91 100644 > > --- a/usr.bin/xinstall/xinstall.c > > +++ b/usr.bin/xinstall/xinstall.c > > @@ -1300,7 +1300,9 @@ copy(int from_fd, const char *from_name, int > to_fd, const char *to_name, > > static size_t bufsize; > > int nr, nw; > > int serrno; > > +#ifndef BOOTSTRAP_XINSTALL > > ssize_t ret; > > +#endif > > char *p; > > int done_copy; > > DIGEST_CTX ctx; > > @@ -1311,6 +1313,7 @@ copy(int from_fd, const char *from_name, int > to_fd, const char *to_name, > > if (lseek(to_fd, (off_t)0, SEEK_SET) =3D=3D (off_t)-1) > > err(EX_OSERR, "lseek: %s", to_name); > > > > +#ifndef BOOTSTRAP_XINSTALL > > /* Try copy_file_range() if no digest is requested */ > > if (digesttype =3D=3D DIGEST_NONE) { > > ret =3D 1; > > @@ -1331,6 +1334,7 @@ copy(int from_fd, const char *from_name, int > to_fd, const char *to_name, > > /* Fall back */ > > } > > > > +#endif > > This looks to be in the wrong place wrt whitespace? > > Jess > > > digest_init(&ctx); > > > > done_copy =3D 0; > > --000000000000d7a004060002f6da Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks for the quick fix. I'm not sure if it mat= ters for performance but it is a bit too restrictive: modern Linux also has= the function so it works just fine there (GitHub CI was happy with this co= mmit on Linux just not MacOS). And as Jess pointed out we should probably u= se it on FreeBSD as well.

Alex=


On Sat, 8 Jul 2023, 17:18 Jessica Clarke, <jrtc27@freebsd.org> wrote:
On 9 Jul 2023, at 00:48, Martin Matuska <mm@FreeBSD.org>= ; wrote:
>
> The branch main has been updated by mm:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=3De64780fbc394b10581e50a850cc06c= 1c12a8e4f9
>
> commit e64780fbc394b10581e50a850cc06c1c12a8e4f9
> Author:=C2=A0 =C2=A0 =C2=A0Martin Matuska <mm@FreeBSD.org>
> AuthorDate: 2023-07-08 23:24:38 +0000
> Commit:=C2=A0 =C2=A0 =C2=A0Martin Matuska <mm@FreeBSD.org>
> CommitDate: 2023-07-08 23:25:23 +0000
>
>=C2=A0 =C2=A0 xinstall: do not use copy_file_range(2) when BOOTSTRAPPIN= G
>
>=C2=A0 =C2=A0 Reported by:=C2=A0 =C2=A0 arichardson
> ---
> usr.bin/xinstall/Makefile=C2=A0 =C2=A0| 5 +++++
> usr.bin/xinstall/xinstall.c | 4 ++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/usr.bin/xinstall/Makefile b/usr.bin/xinstall/Makefile
> index 9969ef104e98..3b49cb39d27a 100644
> --- a/usr.bin/xinstall/Makefile
> +++ b/usr.bin/xinstall/Makefile
> @@ -17,6 +17,11 @@ CFLAGS+=3D -I${SRCTOP}/lib/libnetbsd
> LIBADD=3D md
> CFLAGS+=3D -DWITH_MD5 -DWITH_RIPEMD160
>
> +.ifdef BOOTSTRAPPING
> +# For the bootstrap we disable copy_file_range()
> +CFLAGS+=3D -DBOOTSTRAP_XINSTALL

If the bootstrap xinstall is allowed to use copy_file_range on FreeBSD
(i.e. it=E2=80=99s guaranteed to exist) this is overly restrictive and shou= ld
be:

.if defined(BOOTSTRAPPING) && ${.MAKE.OS} !=3D "FreeBSD"<= br>
> +.endif
> +
> HAS_TESTS=3D
> SUBDIR.${MK_TESTS}+=3D tests
>
> diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c=
> index 8dace862ef1e..6c269bbb5d91 100644
> --- a/usr.bin/xinstall/xinstall.c
> +++ b/usr.bin/xinstall/xinstall.c
> @@ -1300,7 +1300,9 @@ copy(int from_fd, const char *from_name, int to_= fd, const char *to_name,
> static size_t bufsize;
> int nr, nw;
> int serrno;
> +#ifndef BOOTSTRAP_XINSTALL
> ssize_t ret;
> +#endif
> char *p;
> int done_copy;
> DIGEST_CTX ctx;
> @@ -1311,6 +1313,7 @@ copy(int from_fd, const char *from_name, int to_= fd, const char *to_name,
> if (lseek(to_fd, (off_t)0, SEEK_SET) =3D=3D (off_t)-1)
> err(EX_OSERR, "lseek: %s", to_name);
>
> +#ifndef BOOTSTRAP_XINSTALL
> /* Try copy_file_range() if no digest is requested */
> if (digesttype =3D=3D DIGEST_NONE) {
> ret =3D 1;
> @@ -1331,6 +1334,7 @@ copy(int from_fd, const char *from_name, int to_= fd, const char *to_name,
> /* Fall back */
> }
>
> +#endif

This looks to be in the wrong place wrt whitespace?

Jess

> digest_init(&ctx);
>
> done_copy =3D 0;

--000000000000d7a004060002f6da--