Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 9 Mar 2023 19:55:31 +0000
From:      Brooks Davis <brooks@freebsd.org>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Ed Maste <emaste@freebsd.org>, Warner Losh <imp@bsdimp.com>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: c581962414ed - main - src.conf.5: Add some WITH_/WITHOUT_ option descriptions
Message-ID:  <ZAo5sxHFg2LpVnmR@spindle.one-eyed-alien.net>
In-Reply-To: <7a33ffb1-f46f-fd57-b142-fc8641d923df@FreeBSD.org>
References:  <202303082331.328NViDn050541@gitrepo.freebsd.org> <dac0dcf2-a104-e326-43ae-91a084b61623@FreeBSD.org> <CAPyFy2DBaxyGefuVJou17%2BFo=Bj9eZ2YNzUy8CjY%2BmO%2BEHXDnA@mail.gmail.com> <CANCZdfrxGAOKOUGKLiz0up%2Bk1m0ZiVupXfDFP1rzF2-6gpujbA@mail.gmail.com> <CAPyFy2AmPg9781T_x8PsKGrq6OZXPEPZZF3xVu0PxmNfGWLtDA@mail.gmail.com> <7a33ffb1-f46f-fd57-b142-fc8641d923df@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Mar 09, 2023 at 09:30:05AM -0800, John Baldwin wrote:
> On 3/9/23 7:10 AM, Ed Maste wrote:
> > On Wed, 8 Mar 2023 at 23:12, Warner Losh <imp@bsdimp.com> wrote:
> >>
> >> Yea, there's no reason to have the description twice...
> >=20
> > It looks like the ones that have both WITH_ and WITHOUT_ descriptions a=
re:
> >=20
> > ATM AUTO_OBJ BIND_NOW CLANG CLANG_BOOTSTRAP CLANG_FULL CXGBETOOL
> > DEBUG_FILES EFI FDT GCC GCC_BOOTSTRAP GCOV GDB GH_BC GNU_DIFF
> > GOOGLETEST HYPERV KERNEL_RETPOLINE LIB32 LLD LLD_BOOTSTRAP LLD_IS_LD
> > LLDB LLVM_ASSERTIONS LLVM_COV LLVM_CXXFILT LLVM_TARGET_AARCH64
> > LLVM_TARGET_ALL LLVM_TARGET_ARM LLVM_TARGET_MIPS LLVM_TARGET_POWERPC
> > LLVM_TARGET_RISCV LLVM_TARGET_SPARC LLVM_TARGET_X86 LOADER_GELI
> > LOADER_KBOOT LOADER_LUA LOADER_OFW LOADER_UBOOT MALLOC_PRODUCTION
> > MLX5TOOL MODULE_DRM MODULE_DRM2 NVME OFED OPENMP OPENSSL_KTLS PIE
> > PROFILE RELRO REPRODUCIBLE_BUILD RETPOLINE SENDMAIL SHARED_TOOLCHAIN
> > SSP STATS SYSTEM_COMPILER SYSTEM_LINKER TCP_WRAPPERS UNIFIED_OBJDIR
> > USB_GADGET_EXAMPLES ZFS
> >=20
> > although not all of them are used (the ones that default on across all
> > architectures).
> >=20
> > Looking at src.conf.5 the duplicates I see are:
> >=20
> > CXGBETOOL EFI FDT HYPERV LIB32 LLDB LOADER_GELI LOADER_KBOOT
> > LOADER_LUA LOADER_OFW LOADER_UBOOT MLX5TOOL NVME OFED OPENMP =20
> > OPENSSL_KTLS PIE ZFS
> >=20
> > Perhaps for these cases we can just skip the negative sense
> > (WITHOUT_), just listing the architectures it applies to?
> >=20
> > Something like:
> >=20
> >       WITH_CXGBETOOL
> >               Build cxgbetool(8)
> >=20
> >               This is the default setting on amd64/amd64, arm64/aarch64,
> >               i386/i386, powerpc/powerpc64 and powerpc/powerpc64le.
> >=20
> >       WITHOUT_CXGBETOOL is the default setting on amd64/amd64,
> >               arm64/aarch64, i386/i386, powerpc/powerpc64 and
> >               powerpc/powerpc64le.
>=20
> My first thought was your first suggestion (a single FOO file that
> permitted a common prefix for the with/without cases).  However, your
> second suggestion above is also fine and is probably easier to
> implement?
>=20
> The other wrinkle is that we don't really handle BROKEN_OPTIONS ideally.
> We just list the FOO option as defaulting to WITHOUT without telling
> the user that actually it will fail to build if you enable it.  Not
> sure how much work that would be to fix.

Another thing I noticed is that we don't really handle dependent
options sensibly.  For example makeman wants descriptions for
WITH_LOADER_EFI_SECUREBOOT and WITH_LOADER_VERIEXEC_VECTX, but
infact they are no ops because they don't do anything without
WITH_LOADER_VERIEXEC and then they default to enabled.  Only the
WITHOUT_ forms do anything in the current configuration.  (See comments
in https://reviews.freebsd.org/D39002).

At risk of veering off topic, the overall structure of makeman is
obnoxious with all the work being done in a subshell such that it's
impractical to return useful diagnostics.  In a CI job I created
(https://reviews.freebsd.org/D38991) I found I had to store stderr and
then grep for problematic output because I couldn't figure out a good
way to return a non-zero exist status when I didn't want to completely
stop processing when I hit an issue.  makeman should probably be
rewritten in a language with better flow control.

-- Brooks



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ZAo5sxHFg2LpVnmR>