Date: Thu, 14 May 2020 21:02:24 +0000 From: Brooks Davis <brooks@freebsd.org> To: Kyle Evans <kevans@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r360833 - head Message-ID: <20200514210224.GE60902@spindle.one-eyed-alien.net> In-Reply-To: <CACNAnaEJs7x=Rmf5K%2BX6JyLAJF8mnTApFCr=ErM4LCP554rVRQ@mail.gmail.com> References: <202005090201.04921Tpf028388@repo.freebsd.org> <20200511181027.GA60902@spindle.one-eyed-alien.net> <CACNAnaFiHx8YoQQe9pW5WQz65iViZk54aR9=cF42DyhWBjJnAw@mail.gmail.com> <20200512221603.GB60902@spindle.one-eyed-alien.net> <CACNAnaEJs7x=Rmf5K%2BX6JyLAJF8mnTApFCr=ErM4LCP554rVRQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--VUDLurXRWRKrGuMn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, May 12, 2020 at 09:01:33PM -0500, Kyle Evans wrote: > On Tue, May 12, 2020 at 5:16 PM Brooks Davis <brooks@freebsd.org> wrote: > > > > On Mon, May 11, 2020 at 01:45:14PM -0500, Kyle Evans wrote: > > > On Mon, May 11, 2020 at 1:10 PM Brooks Davis <brooks@freebsd.org> wro= te: > > > > > > > > On Sat, May 09, 2020 at 02:01:29AM +0000, Kyle Evans wrote: > > > > > Author: kevans > > > > > Date: Sat May 9 02:01:29 2020 > > > > > New Revision: 360833 > > > > > URL: https://svnweb.freebsd.org/changeset/base/360833 > > > > > > > > > > Log: > > > > > installworld: attempt a certctl rehash at the tail end > > > > > > > > > > This can be run as root or normal user with no problem; if they= hadn't > > > > > twisted the WITHOUT_CAROOT knob, we'll attempt to use the host = certctl to > > > > > rehash the DESTDIR. This would allow one to build systems WITHO= UT_OPENSSL + > > > > > WITH_CAROOT with a populated /etc/ssl that they can then use wi= th an > > > > > appropriate *ssl from somewhere else. > > > > > > > > > > Cross-builds are fine because this will always use the host cer= tctl, or just > > > > > nag if it's missing and it wasn't a WITHOUT_CAROOT build. > > > > > > > > > > MFC after: 1 week > > > > > Differential Revision: https://reviews.freebsd.org/D24641 > > > > > > > > > > Modified: > > > > > head/Makefile.inc1 > > > > > > > > > > Modified: head/Makefile.inc1 > > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D > > > > > --- head/Makefile.inc1 Sat May 9 01:48:08 2020 (r3= 60832) > > > > > +++ head/Makefile.inc1 Sat May 9 02:01:29 2020 (r3= 60833) > > > > > @@ -1403,6 +1403,16 @@ distributeworld installworld stageworld: _= installcheck > > > > > ${DESTDIR}/${DISTDIR}/${dist}.debug.meta > > > > > .endfor > > > > > .endif > > > > > +.elif make(installworld) && ${MK_CAROOT} !=3D "no" > > > > > + # We could make certctl a bootstrap tool, but it requires O= penSSL and > > > > > + # friends, which we likely don't want. We'll rehash on a b= est-effort > > > > > + # basis, otherwise we'll just mention that we're not doing = it to raise > > > > > + # awareness. > > > > > + @if which certctl>/dev/null; then \ > > > > > + certctl rehash \ > > > > > > > > Does this update METALOG with the added links? > > > > > > > > It seems a little weird to rely on DESTDIR from the environment. > > > > > > > > In general I'm not enthusiastic about additions to installworld tha= t do > > > > anything other than copying files, creating links, etc in simple wa= ys. > > > > > > I will happily back this out if I can get some qualified eyes to > > > review/improve it. It does not update METALOG, and it probably should. > > > Agreed on DESTDIR. As for point #3, I guess we can continue spreading > > > `certctl rehash` all over the tree in various points that may need it; > > > the release(7) scripts will need to be done if we don't do it here at > > > a minimum, and I haven't put much thought into it beyond that. > > > > I'm not in a rush to back this out given that it's solving a real > > problem, but lets talk about improvements. > > > > I kind of feel like this belongs in distribution (which I think would > > deal with release scripts) or in etcupdate/mergemaster, but I'm not > > sure either of those are correct. I'd be happy to review changes to > > update the METALOG (I guess we'd extend certctl with an option to do > > that?) I think that's the most important things because we really > > should be routinely validating that DESTDIR only contains things in the > > METALOG. A quick and dirty fix for the DESTDIR weirdness might be addi= ng > > "env DESTDIR=3D${DESTDIR}" so it's explicit. > > >=20 > Got it- not an egregious enough violation to promptly back out, but we > will work towards a better solution. >=20 > For the larger picture, distribution is probably the correct spot for > this; we currently rehash at the following points: >=20 > - mergemaster/etcupdate > - freebsd-update > - post-install of the caroot pkg (pkgbase) >=20 > This leaves two primary final blind spots, which this commit was > attempting to resolve: >=20 > 1.) Install media (including VM images) > 2.) Initial installs (e.g. from bsdinstall) >=20 > #1 could be solved by directly adding it to the release(7) scripts and > #2 could be solved by having bsdinstall do it at config time, but > there seem to be multiple potential points that could instead hit both > (with a single stone) while also making covering the array of other > images that re@ produces (e.g. AWS images) and perhaps other points > that we've not immediately considered. >=20 > To this end, distribution or installworld are probably equally > sufficient, but I'm incredibly unfamiliar with the former. My > understanding from glancing at it over the years is that it may be > executed into a tempdir and merged with mergemaster/etcupdate, or > directly into the new root if we're looking at a new install that > doesn't have potential local changes to merge in. My understanding of distribution is that it's the things that don't make sense to update automatically (e.g. config files that have to be merged. For #1 above distribution clearly solves the problem. For #2 I kind of think this is the installers job. > Part of the problem that I see in my head is probably solved by just > having a var that mergemaster/etcupdate can pass in to distribution to > not bother with the rehash. We want to keep the mergemaster/etcupdate > rehash because if they're running then the target's certainly not been > rehashed. OTOH, we probably don't want to rehash multiple times in > that pipeline- from a certctl standpoint, it's completely harmless, > but it's not the most effective use of system resources and probably > looks a bit bad. In some ways I wonder if we actually want a three phase split: installworld (programs, fixed data, etc), distribution (config bits), and regen (generate files based on the -- possibly altered -- contents of config files, etc. If we could split out the regen into (e.g.) an installed shell script or set of scripts then we could call that from the installer, etc. That might provide a generic solution. So far this isn't a well thought out idea, but maybe there's something here? Presumably we'd need to bolt a compatibility shim onto distribution to start out given how much blowback I got for breaking some nonsensical orderings of the top-level targets when adding METALOG support. -- Brooks --VUDLurXRWRKrGuMn Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAEBAgAGBQJevbHfAAoJEKzQXbSebgfAXz4IAKJe/GUr4dMARu12mHKUuHmG bM4tktcHFy3McgwyHlHg6nZluFKx33USbA6jrK8kAu+hLOzyULmL9UgfmAIi7Wf8 VGJRJJcK99kfsPxFzEKFOpJvcOhN6pTOdJbisfXEvgFtRXJAhdLtqcM8pWk9Nwow J9O5N3Zy8BStmKiXuCXLBMdxWJSXkVHVRqH3KiouIAVEowlVua1+oySg4V3E6unL mCQBpXLc/nA6Dkv4FDtaYfOfUdZssY0IVXtOFZ+mPCS7GUkJTAxlz6GRuALYm6He 1iHAdg6qSB80X0pJxfn8syN5Ysco+vX3i7cLU95r6AUpLliwrzB/3BjMFylpWM0= =QAcX -----END PGP SIGNATURE----- --VUDLurXRWRKrGuMn--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200514210224.GE60902>