bsdinstall: better support custom pkgbase repo config

[ifreund_freebsdfoundation.org]

Summary

Currently if the user attempts to set a custom repo configuration with
$BSDINSTALL_PKG_REPOS_DIR as documented in bsdinstall(8) they will still
be prompted if they would like to perform an offline or network
installation (assuming offline packages are available).

Furthermore, if they select an offline installation, bsdinstall will
silently overwrite the BSDINSTALL_PKG_REPOS_DIR variable before invoking
the pkgbase target.

This patch fixes that issue and improves the user experience when
$BSDINSTALL_PKG_REPOS_DIR is set. There is no change in behavior if
$BSDINSTALL_PKG_REPOS_DIR is not set.

Sponsored by: The FreeBSD Foundation

[ifreund_freebsdfoundation.org]

Git branch: https://github.com/ifreund/freebsd-src/tree/bsdinstall-custom-repo

[emaste]

Overall LGTM, one question inline

usr.sbin/bsdinstall/scripts/auto

198

Should we also rename this variable to HAVE_OFFLINE_BASE_PACKAGES to try to make things more clear?

[jrtc27]

usr.sbin/bsdinstall/scripts/auto

42

Now that I know about the existence of this variable, does BSDINSTALL_PKG_REPOS_DIR not need to be in this list?

220

I'm not convinced that we want to display a dialogue for the fact that the user asked to do something. We don't display a dialogue every time we skip a step as requested by the user's environment, for example.

But, if you really want to do it, displaying $BSDINSTALL_PKG_REPOS_DIR=... ... is a quite ugly UX (and $ isn't right there, moreover, the environment variable is just called BSDINSTALL_PKG_REPOS_DIR, the $ in $foo is shell syntax for "expand to the value of foo"). If you really want it I'd suggest something more along the lines of:

Packages will be installed from custom directory $BSDINSTALL_PKG_REPOS_DIR (BSDINSTALL_PKG_REPOS_DIR).

Also I don't understand why we're doing netconfig here. If BSDINSTALL_PKG_REPOS_DIR is set isn't this an offline install? Which is even more reason to not display a dialogue at all, you don't even need to tell the user about networking.

[ifreund_freebsdfoundation.org]

usr.sbin/bsdinstall/scripts/auto

42

Assuming I understand this mechanism fully, I believe this patch makes it so that BSDINSTALL_PKG_REPOS_DIR does not need to be in this list. With this patch, bsdinstall no longer modifies BSDINSTALL_PKG_REPOS_DIR if it is set by the user.

198

Yes, that seems like a good idea.

220

To me, overriding the system to be installed rather than using upstream FreeBSD feels important enough to assure the user that their configuration has been respected. If other reviewers agree that it's better to silently continue here I'd be happy to remove it though.

Your cleanup to the message looks like an obvious improvement to me, will apply shortly.

Also I don't understand why we're doing netconfig here. If BSDINSTALL_PKG_REPOS_DIR is set isn't this an offline install? Which is even more reason to not display a dialogue at all, you don't even need to tell the user about networking.

Not necessarily, $BSDINSTALL_PKG_REPOS_DIR allows the user to provide an arbitrary repository configuration. This configuration may tell pkg that the FreeBSD-base repository is on some private server controlled by the user for example. In this case, network configuration would be required before trying to fetch packages.

[ifreund_freebsdfoundation.org]

Improve new dialog wording, rename HAVE_BASE_PACKAGES to HAVE_OFFLINE_BASE_PACKAGES for clarity.

[jrtc27]

usr.sbin/bsdinstall/scripts/auto

42

My point is, if it's not set by the user, it may then later be set by this script, and therefore pollute a restart of the install process, as the next run of the script will think the user set it.

[ifreund_freebsdfoundation.org]

usr.sbin/bsdinstall/scripts/auto

42

Ah, I see. Yes you are correct, thanks for spelling it out for me :)

[ifreund_freebsdfoundation.org]

Add BSDINSTALL_PKG_REPOS_DIR to user_env_vars