From nobody Wed Jul 20 22:13:28 2022 X-Original-To: dev-commits-src-all@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 4Lp92L6c5Vz4X5np; Wed, 20 Jul 2022 22:13:30 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Lp92L5pjsz3hfV; Wed, 20 Jul 2022 22:13:30 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658355210; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SkHNVhYTpTA2o9B0am2ydMjDAAFY3Sazbd7rJxI1gGM=; b=uWaqIMssDZWFuUIht03B9QkKc+4RM0x0Z1TWK3Adguxb7SRF41nFoA5JvQmmFb99ruwIMI Z5I/BKRvl4Ztevs9vXIjcvN8c20g8rc0GvleFhSUPVcKx186QSWGz7h95Iv/zlf6OE/x5l S9HDNJwkNim4V25K5qXAEGo2sq74xrCKIwddqiazOLM3vRd/oor60AInhGCStM3EH9WheL DgN4F6RNTwXhTBAf1XGsYAh3Fkyw6VfJxH5gCfLpMLGI0cqcN8kPy26gn+phbW2CSSzMR8 xmBT/jxyOemBnfqgzp/OPdi2UMY0qhmNNAjGp8UCO4moBqerL2FLjKrK9dMqnQ== Received: from [IPV6:2601:648:8680:ed60:a92a:730f:681:b1f9] (unknown [IPv6:2601:648:8680:ed60:a92a:730f:681:b1f9]) (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 did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Lp92K70Jrz1DQl; Wed, 20 Jul 2022 22:13:29 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <3cea890a-3441-4ced-e449-b54494f42a64@FreeBSD.org> Date: Wed, 20 Jul 2022 15:13:28 -0700 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: git: 84bf2bbbecc3 - main - stand: constrain zlib/gzip CFLAGS better Content-Language: en-US To: Warner Losh , Toomas Soome Cc: Dmitry Chagin , Warner Losh , src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202207081750.268Ho5kZ066824@gitrepo.freebsd.org> <244CD526-C7D0-4D42-9DAB-6EA690DFD3A7@me.com> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658355210; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SkHNVhYTpTA2o9B0am2ydMjDAAFY3Sazbd7rJxI1gGM=; b=xUyniRgv3t0kUWoVoGfheJZUOM2G+Bw7PqszoCOlcLuzEIcDzGfD7qC721nRTQXE9Sa3gh zGHYKJFUCRKxL7RF+DbCXDmN8/JK7iEba1xhgf110PA2SHOAzsDX+y1rRQTHDf53cNf8RJ HZItmKtBIvjhNvXTHNkqxhHMkfMTy9DoEXNht6GXccVBhycSlTTRslHh+W3MMHhwlLXnsq YFaprN/Z3EbQji5Rw7XvOYBm23kGun61YCrlL/HzMU6oiUjQHN+BkRUZPCx9fNsLMFL/1P Kc3I+rJCqn7jGk5zqOUY+xwFLgFcLKgJgWOozbPukzm5LdBiQ8cAQGF57q76Tg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1658355210; a=rsa-sha256; cv=none; b=ZV+2TxlMocZNgK/HoY/Xf18eWapK1En1kdz6ewtpNCcHEFN+i/YP6bsWJYo845JqxLv8qW G/PgSmh6oHSLRjzrpEHVB5RnyvR7EKszvc8iXt5k9RdUm2zYiemML9ifhZ26M6h8yF6M2g pDV3eX9pEwPqLpNzrJsjagYMEZJTa4nZAZdTGAXnkRSHFiafGa9bDgCYTfdwtPR95VKDG6 Gx28D3L3020+CaYo5k7PwQYrgEV1ZDQcpRQ+DAwOv1/+DywefLjTjZpABYgthCAWLxyqgv 01zDsml/CPY3aG4y+3hBoAJYAsPCDeREyyIZtRp4NWMuydj2y8Rk+7WesGvZQQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N On 7/20/22 11:20 AM, Warner Losh wrote: > On Wed, Jul 20, 2022 at 12:12 PM Warner Losh wrote: > >> >> >> On Wed, Jul 20, 2022 at 12:06 PM Warner Losh wrote: >> >>> >>> >>> On Wed, Jul 20, 2022 at 11:44 AM Toomas Soome wrote: >>> >>>> >>>> >>>>> On 20. Jul 2022, at 20:24, Dmitry Chagin >>>> wrote: >>>>> >>>>> On Fri, Jul 08, 2022 at 05:50:05PM +0000, Warner Losh wrote: >>>>>> The branch main has been updated by imp: >>>>>> >>>>>> URL: >>>> https://cgit.FreeBSD.org/src/commit/?id=84bf2bbbecc369cea6095bed7a738674b27f8d13 >>>>>> >>>>>> commit 84bf2bbbecc369cea6095bed7a738674b27f8d13 >>>>>> Author: Warner Losh >>>>>> AuthorDate: 2022-07-08 16:29:25 +0000 >>>>>> Commit: Warner Losh >>>>>> CommitDate: 2022-07-08 17:47:37 +0000 >>>>>> >>>>>> stand: constrain zlib/gzip CFLAGS better >>>>>> >>>>>> Define ZLIB_CFLAGS and use it only for the sources that are in >>>> ZLIB or >>>>>> that include it. >>>>>> >>>>>> Sponsored by: Netflix >>>>>> --- >>>>>> stand/libsa/Makefile | 13 +++++++------ >>>>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/stand/libsa/Makefile b/stand/libsa/Makefile >>>>>> index b5d800c26295..09637bd5e9d4 100644 >>>>>> --- a/stand/libsa/Makefile >>>>>> +++ b/stand/libsa/Makefile >>>>>> @@ -96,9 +96,11 @@ SRCS+=${i} >>>>>> >>>>>> # decompression functionality from zlib >>>>>> .PATH: ${SRCTOP}/sys/contrib/zlib >>>>>> -CFLAGS+=-DHAVE_MEMCPY -I${SRCTOP}/sys/contrib/zlib >>>>>> -SRCS+= adler32.c crc32.c >>>>>> -SRCS+= infback.c inffast.c inflate.c inftrees.c zutil.c >>>>>> +ZLIB_CFLAGS=-DHAVE_MEMCPY -I${SRCTOP}/sys/contrib/zlib >>>>>> +.for i in adler32.c crc32.c infback.c inffast.c inflate.c inftrees.c >>>> zutil.c >>>>>> +CFLAGS.${i}+=${ZLIB_CFLAGS} >>>>>> +SRCS+= ${i} >>>>>> +.endfor >>>>>> >>>>>> # lz4 decompression functionality >>>>>> .PATH: ${SRCTOP}/sys/cddl/contrib/opensolaris/common/lz4 >>>>>> @@ -168,9 +170,8 @@ SRCS+= time.c >>>>>> .PATH: ${SRCTOP}/sys/ufs/ffs >>>>>> SRCS+=ffs_subr.c ffs_tables.c >>>>>> >>>>>> -CFLAGS.dosfs.c+= -I${LDRSRC} >>>>>> -CFLAGS.tftp.c+= -I${LDRSRC} >>>>>> -CFLAGS.ufs.c+= -I${LDRSRC} >>>>> ^^^^^^^^^^^^ is this correct? at least it breaks builds with >>>>> WITHOUT_LOADER_ZFS and WITHOUT_BOOT probably, see PR/260083 >>>>> >>>>> >>>> >>>> No, it is not correct. >>>> >>> >>> My change is correct, theoretically. However, there's a layering >>> violation that means they are needed so it was premature. >>> >>> I'll fix a bandaide and do it better when I return from vacation. >>> >> >> Doh! I don't have the right keys loaded in my ssh-agent, so I can't push >> the change because the port forwarding on my router is broken and I can't >> remotely login :( >> >> If someone could commit the change I suggested in >> https://reviews.freebsd.org/D35860 that would be great! >> > > The changes were needed, btw, to limit the scope of CFLAGS for the OpenZFS > blake3.c support. However, this one scope limiting shouldn't break that. The thing that doesn't make sense to me at all though is that what happens in practice is that ZFS unconditionally adds -ILDRSRC to the global CFLAGS. Given that the goal is to limit the proliferation of global options in CFLAGS, it seems odd to reject changes that add -ILDRSRC to specific files. My second review for GELI does replace a global CFLAGS change you removed with instead adding it to specific files which seems to be a step in the right direction. Further fixing ZFS to not set any global CFLAGS would seem to be a further improvement that would expose this current breakage even when ZFS is enabled. However, I don't really see why we shouldn't add the needed include just to specific files for now rather than the hack in defs.mk. If we later fix libsa to not need headers from the loader (which I agree with, and might mean that we just need to move some headers or their contents over to libsa) can remove targeted CFLAGS.foo.c bits for -ILDRSRC as part of that commit. -- John Baldwin