Re: git: 84bf2bbbecc3 - main - stand: constrain zlib/gzip CFLAGS better

From: Warner Losh <imp_at_bsdimp.com>
Date: Wed, 20 Jul 2022 18:20:08 UTC
On Wed, Jul 20, 2022 at 12:12 PM Warner Losh <imp@bsdimp.com> wrote:

>
>
> On Wed, Jul 20, 2022 at 12:06 PM Warner Losh <imp@bsdimp.com> wrote:
>
>>
>>
>> On Wed, Jul 20, 2022 at 11:44 AM Toomas Soome <tsoome@me.com> wrote:
>>
>>>
>>>
>>> > On 20. Jul 2022, at 20:24, Dmitry Chagin <dchagin@heemeyer.club>
>>> 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 <imp@FreeBSD.org>
>>> >> AuthorDate: 2022-07-08 16:29:25 +0000
>>> >> Commit:     Warner Losh <imp@FreeBSD.org>
>>> >> 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.

Warner


> Warner
>
>
>> Warner
>>
>>
>>> rgds,
>>> toomas
>>>
>>>
>>> >
>>> >
>>> >> +CFLAGS.gzipfs.c+= ${ZLIB_CFLAGS}
>>> >> +CFLAGS.pkgfs.c+= ${ZLIB_CFLAGS}
>>> >> CFLAGS.bzipfs.c+= -I${SRCTOP}/contrib/bzip2  -DBZ_NO_STDIO
>>> -DBZ_NO_COMPRESS
>>> >>
>>> >> # explicit_bzero and calculate_crc32c
>>>
>>>