Imperative is definitely better, thanks for the catch.

What do you think about "Fetch and install base system packages..."?

It sounds less awkward to me and I think it preserves clarity. Perhaps simply "base packages" would be fine as well though.

I think it is important to clarify that this directory is only for "repository configuration files" not the main pkg.conf configuration file.

While your suggested wording is much more concise, I think it is important to make that distinction plain.

It would be easier to write this clearly if pkg had a separate man page for "repository configuration files" rather than describing them in a section of pkg.conf(5)

The latter is probably better. Since this is one of the first manuals mentioning pkgbase in the tree, we should be thoughtful about the precident.

Would it be clear using "repository configuration" instead of just "configuration" in my suggestion?

Indeed, we've been pretty inconsistent with pkgbase (or should I say PkgBase?) related terminology so far.

I think "base packages" is good, I'll switch to that in my next update.

Would it be clear using "repository configuration" instead of just "configuration" in my suggestion?

Hmm, I'm not sure. I think if the user was not already aware that there are two types of configuration files described in pkg.conf it could be quite confusing.

I tweak this wording in the latest update to get rid of some of the redundancy you highlighted while also being more accurate and concise. Let me know if you want even more conciseness though, I'll defer to your docs expertise :)

Yeah, we haven't had to think about this too hard before, because it's only now that pkgbase is really becoming end-user-facing. This terminology will also appear in the Handbook and other documentation.

"Base system packages" is good. I think it's also fine to shorten it to "base packages" depending on context.

After going back and forth a bit in my head, I've left it as "base system packages" here. It's more clear without being needlessly verbose.

If I were to repeat the phrase multiple times in the same sentence I would surely prefer "base packages" for the repetitions :)

Done, thanks.

I find it odd that minor doesn't feature in these URLs, it means if you're tracking 14.1-STABLE then you are forcefully upgraded to 14.2-STABLE the moment that exists, no?

I don't love splatting config files into the middle of the code. Is there a reason we can't just have it be a file that's built during buildworld that we then install? We already do that in usr.sbin/pkg/Makefile with _BRANCH to decide latest vs quarterly, I don't see why we can't do similar things for base_release_$MINOR vs base_latest.

Putting keys in the Lua is particularly bad, it's a bit shady and makes it hard for people to audit what keys are in the repo. Besides, why can't we just point pkg at the install media's copy of the key for the initial install step? Or at least if we really have to manually bootstrap things by putting a key in the chroot, do so by copying the install media's copy. But it would really be preferable if the key made it into the chroot by virtue of installing FreeBSD-pkg-bootstrap.

How do I get a non-default set?

People want things other than that to be supported in bsdinstall

Can we please not add more hard-coding of lib32 being the only supported libcompat? Very few places in base do that now. _ALL_libcompats from bsd.compat.pre.mk gives you the list of ones that exist, please use that somehow to automatically determine the sets of compatibility libraries the build system knows about.

This won't be true for architectures without lib32

Surely this should be done with normal lua APIs rather than os.execute?

Or just let this continue on, return from the function and exit the script?

As I understand it, the repo URLs map directly to the git branches of src from which the packages are continuously built. base_latest here maps to either the main or stable/14 branch depending on the version, and there are no stable/14.0, stable/14.1, etc branches.

Sure, I don't see a reason we couldn't install this to /usr/share/bsdinstall/FreeBSD-base.conf and then copy it from there.

I'll switch to that approach.

Unfortunately pkg always interprets the fingerprints path relative to the --rootdir (in this case the chroot path). This makes it necessary to have a copy of the key inside the chroot path in order to install packages there.

Copying the key from the install media would definitely be better than hardcoding it here though, I'll switch to that. We already rely on the install media's key to bootstrap pkg in any case.

Installing the FreeBSD-pkg-bootstrap package will overwrite this file during the pkg install for what it's worth.

With the current version of this patch, the answer is to manually invoke pkg during the "Manual Configuration" installer step or after installation is complete.

If I understand @bapt's plans correctly, pkg will gain first class support for "groups" of packages, making the pattern matching on package names done by this function obsolete. My intent was to wait for that proper solution to replace this adhoc filtering and then implement a prompt to select package groups directly.

It would certainly be possible however to create a component selection dialog based on this package name pattern matching which offers the same base-dbg, kernel-dbg, lib32, lib32-dbg, etc. options as the existing standard installation path. I'd be happy to implement that in this patch if you like.

Indeed, and it would be straightforward to add a kernel selection dialog in a follow up patch.

My intent was to limit the scope of this initial patch in order to simplify review, perhaps that was misguided though. Would you like me to add that to this patch?

This comment made me realize a related issue with the current design however: we always install a kernel even if targeting a jail. I'm not sure of the best way to fix that. The most straightforward option would be a BSDINSTALL_NO_KERNEL environment variable which is set by the jail target but not the auto target.

Good point, I'll just need to figure out how best to way to embed the value of that variable in this script.

Thanks for the catch, will fix

Unfortunately there is no mkdir available in the lua standard library. In fact, there are no functions that deal with directories at all, only files.

Sure, I don't have a strong opinion about this.

flua has at least some subset of lfs functionality available, see ^/libexec/flua/modules/lfs.c: attirbutes, dir, mkdir, rmdir

Aha! I stand corrected, thanks. Are there docs on the full set of flua extensions somewhere that I have missed?

Nope =(. Some of the newer ones have 3lua section manpages, but a lot of the ones we hacked out earlier on aren't documented outside of their existence in libexec/flua and use in the earlier sys/tools/makesyscalls.lua (or stand/lua).

I'm not sure it would really be better to use lfs.mkdir() here though, implementing the equivalent of -p manually is cumbersome and feels out of place in this script.

CC @bapt

IMO we should bias strongly towards having minimal functionality sooner, to facilitate wider testing of pkgbase. Regardless of whether we get pkg groups or implement a selection dialog I'm fine with that coming in a subsequent update.

We have in the past done something along the lines of naming this file pkgbase.in, putting %%LIBCOMPATS%% in it, and using sed to expand the placeholder, but I think that approach is a bit awkward. What about just reading the list of compats from a file?

That is correct.

Unfortunately pkg always interprets the fingerprints path relative to the --rootdir (in this case the chroot path). This makes it necessary to have a copy of the key inside the chroot path in order to install packages there.

This is by choice.

Copying the key from the install media would definitely be better than hardcoding it here though, I'll switch to that. We already rely on the install media's key to bootstrap pkg in any case.

Yup, it's the way to go :)

Installing the FreeBSD-pkg-bootstrap package will overwrite this file during the pkg install for what it's worth.

Having a way to select packages after is easier, it's better to soon have install media offering pkgbase (even if it's full install or all packages) so people can test without doing the bootstrap phase on a live system.

I ended up just doing the`pkgbase.in` + sed thing, I have to do it for FreeBSD-base.conf now as well.

Roff does not permit starting a new sentence on an existing line. You can check for these kind of issues with mandoc -Tlint here/is/the/file

Fixed, thanks for the catch.

This may not be correct for the installer -- I'm not sure we have a proper pkg db in the context that it runs. But I was able to successfully install a pkgbase system with this series of patches, so much prefer to get it in the tree asap and start producing snapshots with this support.

Hah, I was wondering if someone had already written this function somewhere in the src tree!

It's not too much code to copy and paste between our various lua scripts but I do wonder if it would make sense to collect this kind of thing in a module under libexec/flua.

Hmm, I'm fairly confident we have readonly access to the pkg db created when pkg was installed to the install media while the installer is running. I'm not sure how this check could be passing and allowing the installation to continue otherwise.