From nobody Thu Mar 09 19:55:31 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 4PXg046CXNz3xKbx; Thu, 9 Mar 2023 19:55:32 +0000 (UTC) (envelope-from brooks@spindle.one-eyed-alien.net) Received: from spindle.one-eyed-alien.net (spindle.one-eyed-alien.net [199.48.129.229]) (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 did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4PXg044NZCz3DZL; Thu, 9 Mar 2023 19:55:32 +0000 (UTC) (envelope-from brooks@spindle.one-eyed-alien.net) Authentication-Results: mx1.freebsd.org; none Received: by spindle.one-eyed-alien.net (Postfix, from userid 3001) id 4D1753C0199; Thu, 9 Mar 2023 19:55:31 +0000 (UTC) Date: Thu, 9 Mar 2023 19:55:31 +0000 From: Brooks Davis To: John Baldwin Cc: Ed Maste , Warner Losh , 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: References: <202303082331.328NViDn050541@gitrepo.freebsd.org> <7a33ffb1-f46f-fd57-b142-fc8641d923df@FreeBSD.org> 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 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <7a33ffb1-f46f-fd57-b142-fc8641d923df@FreeBSD.org> X-Rspamd-Queue-Id: 4PXg044NZCz3DZL X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:36236, ipnet:199.48.128.0/22, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N 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 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