On Tue, Jun 18, 2002 at 07:01:24PM -0700, Jason Thorpe wrote:
  | On Wed, Jun 19, 2002 at 03:17:11AM +0300, Luke Mewburn wrote:
  | 
  |  > Module Name:	basesrc
  |  > Committed By:	lukem
  |  > Date:		Wed Jun 19 00:17:11 UTC 2002
  |  > 
  |  > Modified Files:
  |  > 	basesrc: Makefile
  |  > 
  |  > Log Message:
  |  > In afterinstall, only run postinstall check if DESTDIR != / (or equivalents).
  |  > Fixes [misc/17275] from Gregory McGarry <g.mcgarry@ieee.org>
  | 
  | This change is incorrect (yes, I know the commit message has a typo).

I respectfully disagree.

I am aware of at least the following issues with running the
``postinstall-check DESTDIR != /'' in the default "afterinstall"
target:

    +	Not fully populated ${DESTDIR}/etc

	This could potentially be solved by running postinstall-check
	after make release,distribution,...


    +	UNPRIVED causes issues; at least the uid check to fail

	Not a major hassle, but users have expressed concerns about it.


    +	${DESTDIR}/etc/rc.conf references /etc/defaults/rc.conf
	rather than ${DESTDIR}/etc/defaults/rc.conf, possibly
	resulting in incorrect warnings about rc.conf(5)
	variables being incorrectly set.  (PR 17024 hit this)

	It was suggested that we use something like chroot(8) in
	rcconf_is_set() when doing this check, but this doesn't work
	for UNPRIVED, nor if the target architecture is different to
	the source architecture, and I'm sure there's other problems
	with this approach as well.

	Using grep in rcconf_is_set() won't work either, since
	rc.conf(5) is a shell script sourced in by rc.d and might
	contain logic to change how things operate.

(I think there were other problems raised for DESTDIR!=/ case, but
I can't recall them right now).

Moving the ``postinstall-check DESTDIR != /'' code to the
"after distribution (et al)" part of the Makefile would still
be affected by some or all of the remaining issues.




  | Consider the case of building into an already populated NFS root
  | for a diskless client (this is something I do quite regularly).
  | I would now have to run the postinstall-check phase manually, which
  | means extra work.

In your case (where you are most likely aware of the limitations listed
above), given the special cases and hairy bits for the DESTDIR!=/ case
in other situations (for other users), requiring that you (as a
clueful developer) need to run "make DESTDIR=/nfsroot postinstall-check"
for your special case isn't that much of a burden, IMHO.


  | Instead, I think that you should skip the postinstall-check in
  | afterinstall: only if the target originally invoked at the top-level
  | was "distribution" or "release".  You can safely assume that the
  | DESTDIR is empty (or at least known to be invalid) in that case, and
  | this preserves the previous (and quite useful) behavior for the case
  | I mentioned above.

See above for some problems with that. :-(


I *would* like for postinstall to be useful to the greatest group of
people as possible, with the minimum amount of hassle for them and
for us generated by people legitimately asking about a weird problem
caused by a special case scenario that we hadn't catered for (some of
which we've experienced already and I've listed above).


Luke.