From nobody Sun Feb 04 16:41:34 2024 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 4TSZzS278nz58LWF; Sun, 4 Feb 2024 16:41:56 +0000 (UTC) (envelope-from brd@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 4TSZzS1bhYz4lYG; Sun, 4 Feb 2024 16:41:56 +0000 (UTC) (envelope-from brd@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707064916; 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=AMdSdyrbKfzBFJqKn/AvcfzthHDPc44OSb6U5MqJLhk=; b=LSp7/BLOQzcbfy91tsvyZBf27icR+X6mI0qCDPi3wvQPnOy7dsRfPf3+8l9Z6KAPeAFmQL DPPwGw897tQFCqK3sqN28nql3eYTqkqd4atsWzOFEfrYcJxnRKRVcOthyOAKv2ky27c4in XrmyjksF8KVxyWN4V5C5C4Y7qzLvyONWUkpi0snhUNjwjr0a/Uo/02K4qcpfYKyHhkL2Z0 KRact3fFQ0Vf2NHsECgvvaMf0XuQuLpQiNGSr3mN2XNGfF21M9kIYWJERUJ5oCDyKuUENb dojReBlRmV38TLzYK8FfeRqjMgcm9EO0BYZ7sjE6M5wUS5txsosxK1FV1HA12Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707064916; 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=AMdSdyrbKfzBFJqKn/AvcfzthHDPc44OSb6U5MqJLhk=; b=Tufg0CaRT1DU25brJakrhlxau33TR5MlN1agx6jxokdUII4x5MQ7sqFh4Bk77zQqQum3NU hlTD/2u+BZKeNuVKB2yQtAA314YaYn6SpQtaAGD2LkLtRxrijNWhJRqbMv28VLnTZ9BbbL RxHyqgYJvz6ZOvhtzKJBDYFQi6sg5Pp9SncKjAaXF+ZjOkMT62GDO2e2bdnCDlOKIi+h27 pCg+YVimt8t4L/+MEMVMC7QLU+VNroI8LSeid2tNqe0psieMGsvstT4gzYyJsa/VfN1TOi oiPR6+HDslgWsWcgs+PSlhtH4OJ859s5eXbsn7BGER02zU9NhzFNx7fWBHIAGQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1707064916; a=rsa-sha256; cv=none; b=Rl1CET2XIy2U4BBW5vm4uOd0t9tLkAUPgmqTZNnbi+42MI1Jmk7FoDdDPhtVMShGDobw1q ZKTKqRhpM1sX/ezHhqwUPItE6psQ+7fp9MCfsnNx8P1DG+MwECmHpfByNe13xs13c+wbBQ dQCCtqb2tQtbhSTLLM9VDQ/lNdQcBwG498nus6gHDs2Ccc9xN9OVIUOImzlYRhUbJvRm1l CIqAzsfYOkpnSyLDsrAQj8xlcHFHUuBy6Sa/4zVr18D39hl4NVcrxP20EA3+HaqeF6kepo kWaajyUAQ+ydOshn7VRI+VNaAFLLXKj4JKs6YCokTsGjB0gW/9JnvK9x1Fo9ng== Received: from auth1-smtp.messagingengine.com (auth1-smtp.messagingengine.com [66.111.4.227]) (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) (Authenticated sender: brd/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4TSZzR5sJTzbx8; Sun, 4 Feb 2024 16:41:55 +0000 (UTC) (envelope-from brd@FreeBSD.org) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailauth.nyi.internal (Postfix) with ESMTP id F1D9F27C005B; Sun, 4 Feb 2024 11:41:54 -0500 (EST) Received: from imap48 ([10.202.2.98]) by compute1.internal (MEProxy); Sun, 04 Feb 2024 11:41:54 -0500 X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedukedgledtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpefofgggkfgjfhffhffvvefutgfgse htqhertderreejnecuhfhrohhmpedfuehrrgguucffrghvihhsfdcuoegsrhgusefhrhgv vgeuufffrdhorhhgqeenucggtffrrghtthgvrhhnpeegjeeikeehgfffteelveefgfdtke ehleffieduffevheevveegjedtkedtleetveenucffohhmrghinhepfhhrvggvsghsugdr ohhrghenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpe gsrhgrugdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihhthidqjedtjeeifedvfedv qddukedtieelieekkedqsghrugeppefhrhgvvgeuufffrdhorhhgsegsrhgruggurghvih hsrdhioh X-ME-Proxy: Feedback-ID: if7394599:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id AF15D31A0066; Sun, 4 Feb 2024 11:41:54 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.11.0-alpha0-144-ge5821d614e-fm-20240125.002-ge5821d61 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 Message-Id: <49467837-dadd-4252-bfa7-169b0630bb41@app.fastmail.com> In-Reply-To: References: <202401312205.40VM5dQS018685@gitrepo.freebsd.org> <1D1F0A7A-C735-4D6F-B333-39920E84CD5D@freebsd.org> Date: Sun, 04 Feb 2024 09:41:34 -0700 From: "Brad Davis" To: "Jessica Clarke" Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: 009d3f66cb5f - main - bsdinstall: separate out dist selection in prep for pkgbase support Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable On Fri, Feb 2, 2024, at 6:27 PM, Jessica Clarke wrote: > On 31 Jan 2024, at 22:15, Jessica Clarke wrote: >> On 31 Jan 2024, at 22:05, Brad Davis wrote: >>>=20 >>> The branch main has been updated by brd: >>>=20 >>> URL: https://cgit.FreeBSD.org/src/commit/?id=3D009d3f66cb5f0cf3f1d35= 3f311d3a6878b2a534e >>>=20 >>> commit 009d3f66cb5f0cf3f1d353f311d3a6878b2a534e >>> Author: Brad Davis >>> AuthorDate: 2024-01-26 17:46:46 +0000 >>> Commit: Brad Davis >>> CommitDate: 2024-01-31 22:05:27 +0000 >>>=20 >>> bsdinstall: separate out dist selection in prep for pkgbase support >>>=20 >>> No functional change intended. >>>=20 >>> Approved by: asiciliano >>> Sponsored by: Rubicon Communications, LLC ("Netgate") >>> Differential Revision: https://reviews.freebsd.org/D43621 >>> --- >>> usr.sbin/bsdinstall/scripts/auto | 40 ++++-------------- >>> usr.sbin/bsdinstall/scripts/selectdists | 73 +++++++++++++++++++++++= ++++++++++ >>> usr.sbin/bsdinstall/startbsdinstall | 1 + >>> 3 files changed, 82 insertions(+), 32 deletions(-) >>>=20 >>> diff --git a/usr.sbin/bsdinstall/scripts/auto b/usr.sbin/bsdinstall/= scripts/auto >>> index 9f4b5b52fe5d..c651d654d62e 100755 >>> --- a/usr.sbin/bsdinstall/scripts/auto >>> +++ b/usr.sbin/bsdinstall/scripts/auto >>> @@ -153,36 +153,10 @@ trap true SIGINT # This section is optional >>> trap error SIGINT # Catch cntrl-C here >>> if [ -z "$BSDINSTALL_SKIP_HOSTNAME" ]; then bsdinstall hostname || e= rror "Set hostname failed"; fi >>>=20 >>> -export DISTRIBUTIONS=3D"${DISTRIBUTIONS:-base.txz kernel.txz}" >>> -if [ -f $BSDINSTALL_DISTDIR/MANIFEST ]; then >>> - DISTMENU=3D`awk -F'\t' '!/^(kernel\.txz|base\.txz)/{print $1,$5,$6= }' $BSDINSTALL_DISTDIR/MANIFEST` >>> - DISTMENU=3D"$(echo ${DISTMENU} | sed -E 's/\.txz//g')" >>> - >>> - if [ -n "$DISTMENU" ]; then >>> - exec 5>&1 >>> - EXTRA_DISTS=3D$( eval bsddialog \ >>> - --backtitle \"$OSNAME Installer\" \ >>> - --title \"Distribution Select\" --nocancel --separate-output \ >>> - --checklist \"Choose optional system components to install:\" \ >>> - 0 0 0 $DISTMENU \ >>> - 2>&1 1>&5 ) >>> - for dist in $EXTRA_DISTS; do >>> - export DISTRIBUTIONS=3D"$DISTRIBUTIONS $dist.txz" >>> - done >>> - fi >>> -fi >>> - >>> -FETCH_DISTRIBUTIONS=3D"" >>> -for dist in $DISTRIBUTIONS; do >>> - if [ ! -f $BSDINSTALL_DISTDIR/$dist ]; then >>> - FETCH_DISTRIBUTIONS=3D"$FETCH_DISTRIBUTIONS $dist" >>> - fi >>> -done >>> - >>> -if [ -n "$FETCH_DISTRIBUTIONS" -a -n "$BSDINSTALL_CONFIGCURRENT" ];= then >>> - bsddialog --backtitle "$OSNAME Installer" --title "Network Install= ation" --msgbox "Some installation files were not found on the boot volu= me. The next few screens will allow you to configure networking so that = they can be downloaded from the Internet." 0 0 >>> - bsdinstall netconfig || error >>> - NETCONFIG_DONE=3Dyes >>> +if [ -n "$BSDINSTALL_USE_DISTRIBUTIONS" ]; then >>> + exec 5>&1 >>> + export DISTRIBUTIONS=3D$( `dirname $0`/selectdists 2>&1 1>&5 ) >>> + exec 5>&- >>> fi >>>=20 >>> rm -f $PATH_FSTAB >>> @@ -347,8 +321,10 @@ if [ -n "$FETCH_DISTRIBUTIONS" ]; then >>>=20 >>> [ $FETCH_RESULT -ne 0 ] && error "Could not fetch remote distributio= ns" >>> fi >>> -bsdinstall checksum || error "Distribution checksum failed" >>> -bsdinstall distextract || error "Distribution extract failed" >>> +if [ -n "$BSDINSTALL_USE_DISTRIBUTIONS" ]; then >>> + bsdinstall checksum || error "Distribution checksum failed" >>> + bsdinstall distextract || error "Distribution extract failed" >>> +fi >>>=20 >>> # Set up boot loader >>> bsdinstall bootconfig || error "Failed to configure bootloader" >>> diff --git a/usr.sbin/bsdinstall/scripts/selectdists b/usr.sbin/bsdi= nstall/scripts/selectdists >>> new file mode 100644 >>> index 000000000000..b548e82a95f8 >>> --- /dev/null >>> +++ b/usr.sbin/bsdinstall/scripts/selectdists >>> @@ -0,0 +1,73 @@ >>> +#!/bin/sh >>> +#- >>> +# Copyright (c) 2011 Nathan Whitehorn >>> +# Copyright (c) 2013-2018 Devin Teske >>> +# All rights reserved. >>> +# >>> +# Redistribution and use in source and binary forms, with or without >>> +# modification, are permitted provided that the following conditions >>> +# are met: >>> +# 1. Redistributions of source code must retain the above copyright >>> +# notice, this list of conditions and the following disclaimer. >>> +# 2. Redistributions in binary form must reproduce the above copyri= ght >>> +# notice, this list of conditions and the following disclaimer i= n the >>> +# documentation and/or other materials provided with the distrib= ution. >>> +# >>> +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'= ' AND >>> +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,= THE >>> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULA= R PURPOSE >>> +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE = LIABLE >>> +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONS= EQUENTIAL >>> +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE= GOODS >>> +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPT= ION) >>> +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRAC= T, STRICT >>> +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN= ANY WAY >>> +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILI= TY OF >>> +# SUCH DAMAGE. >>> +# >>> +# >>> +############################################################ INCLUD= ES >>> + >>> +BSDCFG_SHARE=3D"/usr/share/bsdconfig" >>> +. $BSDCFG_SHARE/common.subr || exit 1 >>> + >>> +############################################################ CONFIG= URATION >>> + >>> +# >>> +# Default distributions >>> +# >>> +DISTRIBUTIONS=3D"${DISTRIBUTIONS:-base.txz kernel.txz}" >>> + >>> +############################################################ MAIN >>> + >>> +if [ -f $BSDINSTALL_DISTDIR/MANIFEST ]; then >>> + DISTMENU=3D`awk -F'\t' '!/^(kernel\.txz|base\.txz)/{print $1,$5,$6= }' $BSDINSTALL_DISTDIR/MANIFEST` >>> + DISTMENU=3D"$(echo ${DISTMENU} | sed -E 's/\.txz//g')" >>> + >>> + if [ -n "$DISTMENU" ]; then >>> + EXTRA_DISTS=3D$( eval bsddialog \ >>> + --backtitle \"$OSNAME Installer\" \ >>> + --title \"Distribution Select\" --nocancel --separate-output \ >>> + --checklist \"Choose optional system components to install:\" \ >>> + 0 0 0 $DISTMENU \ >>> + 2>&1 >&3 ) >>> + for dist in $EXTRA_DISTS; do >>> + DISTRIBUTIONS=3D"$DISTRIBUTIONS $dist.txz" >>> + done >>> + fi >>> +fi >>> + >>> +FETCH_DISTRIBUTIONS=3D"" >>> +for dist in $DISTRIBUTIONS; do >>> + if [ ! -f $BSDINSTALL_DISTDIR/$dist ]; then >>> + FETCH_DISTRIBUTIONS=3D"$FETCH_DISTRIBUTIONS $dist" >>> + fi >>> +done >>> + >>> +if [ -n "$FETCH_DISTRIBUTIONS" -a -n "$BSDINSTALL_CONFIGCURRENT" ];= then >>> + bsddialog --backtitle "$OSNAME Installer" --title "Network Install= ation" --msgbox "Some installation files were not found on the boot volu= me. The next few screens will allow you to configure networking so that = they can be downloaded from the Internet." 0 0 >>> + bsdinstall netconfig || error >>> + NETCONFIG_DONE=3Dyes >>> +fi >>> + >>> +echo $DISTRIBUTIONS >&2 >>> diff --git a/usr.sbin/bsdinstall/startbsdinstall b/usr.sbin/bsdinsta= ll/startbsdinstall >>> index 63239c969ac6..8d9fb981c11d 100644 >>> --- a/usr.sbin/bsdinstall/startbsdinstall >>> +++ b/usr.sbin/bsdinstall/startbsdinstall >>> @@ -6,6 +6,7 @@ >>> : ${BSDDIALOG_EXTRA=3D3} >>> : ${BSDDIALOG_ESC=3D5} >>> : ${BSDDIALOG_ERROR=3D255} >>> +export BSDINSTALL_USE_DISTRIBUTIONS=3Dy >>=20 >> I said it in the review and I=E2=80=99ll say it again here since you = decided to >> just ignore me: this does not belong here. Please remove it and fix t= he >> default in selectdists. > > Firstly, ping on this. I still object to the approach taken here. > > Secondly, this commit was not at all tested. You do not install the new > selectdists script, and so the installer falls over (in a rather > cryptic way*). I am extremely unimpressed at ignoring reviewer comments > and completely breaking the installer, and thus am immediately > reverting this commit. It probably works if you install the script, but > it=E2=80=99s your job to test that, not mine, so when you have a patch= that > actually works please re-seek review and actually engage with the > comments. I appreciate your feedback, but I explained why I did it that way and th= at wasn't good enough for you. Sorry for the breakage. I tested by rebuilding the memstick image over = and over and installing a bhyve VM: sudo make -C release clean && sudo make -C release memstick NOPORTS=3Dy = NOPKG=3Dy NOSRC=3Dy WITHOUT_DEBUG=3Dy # ls -al /usr/libexec/bsdinstall/selectdists=20 -r-xr-xr-x 1 root wheel 2882 Jan 31 21:37 /usr/libexec/bsdinstall/selec= tdists So I am not sure how it worked for me. Regards, Brad Davis