Subject: Re: CVS commit: src/usr.bin/stat
To: Robert Elz <kre@munnari.OZ.AU>
From: Luke Mewburn <lukem@NetBSD.org>
List: current-users
Date: 07/23/2003 21:08:51
On Wed, Jul 23, 2003 at 05:46:31PM +0700, Robert Elz wrote:
  |   | Log Message:
  |   | * don't compile in support for st_birth #ifdef HOSTPROG
  | 
  | That will make -current be able to be built again on NetBSD systems
  | older than (moderately) recent -current (moderately means the past
  | few months).
  | 
  | However, attempting to make stat into a HOSTPROG is almost certainly
  | a mistake.   By its very nature, stat is a very NetBSD specific program.
  | Its purpose is to make available at the shell script level everything
  | that the stat(2) system call makes available.

Actually, stat(1) greatly simplified how symlink & hardlinks are
installed by bsd.links.mk, and the symlink support in bsd.obj.mk
and bsd.kinc.mk.  So it *is* useful as a hostprog, just like a bunch
of other programs (e.g, we added "cat", even though it was just for
one option - 'cat -l').


  | Hence the (totally useless, but that's a different issue) st_birthtime,
  | which made stat.c fail to compile on NetBSD 1.6.1 (or older) or
  | even on -current from more than a few months ago (before the ufs2
  | integration) - until your recent fix.
  | 
  | But, stat.c also make available
  | 	st_gen
  | 	st_blocks
  | 	st_blocksize
  | (and everything else).   How many of those do we really want to
  | depend upon being available on whatever random system is going to
  | be used to compile NetBSD ?
  | 
  | Note: none of this is actually necessary (I hope) for the build system,
  | all it wants stat for is some of the simpler operations.

Yes, but that doesn't preclude stat being it.


  | I'd suggest removing stat from being a HOSTPROG, and instead building
  | a much simpler, and truly portable program (call it stat, and keep the
  | syntax compatible if that helps - perhaps even use the stat sources
  | with HEAVY ifdef) that does just what is needed for the NetBSD build
  | system's use of it.

With all of our other host tools, we leverage off the existing code.
I don't see the "win" in changing this now for minor asthetic reasons,
and also at the expense of "separate code to maintain"...


  | 
  | This change however ...
  | 
  |   | * remove %B (birth) from default format strings, since %B isn't available
  |   |   #ifdef HOSTPROG, and it's not standard enough to be fussed about anyway
  | 
  | I think is a bug.   Some of those strings (at least) are intended to
  | dump *everything* (not just everything that someone thinks you might
  | need to know about).

Well, in that case, the prior behaviour was buggy anyway, because there
were other stat fields (%f - st_flags - springs to mind) that were NOT
in those format strings.


  | If you wanted to keep stat as a HOSTPROG (if not, this change can be
  | undone and forgotten), the solution would be to make %B print some other
  | time value (st_mtime) most likely) whan stat is compiled as a HOSTPROG.

Or we can just make %B and %f and other "non standard" stuff continue
to error out in the hostprog version (which is acceptable because we
know how we are using the hostprog), or print "empty" results for
unsupport flags.


  | All of this stuff raises a more general issue as well - when ever a well
  | defined (but extensible) structure (like struct stat) has something new
  | added to it, there should also be a "feature test" #define added to the
  | relevant include file, so other code can test whether this new thing
  | exists.
  | 
  | stat.c has issues with st_flags (which it has a special case #define to
  | deal with) and now with st_birthtime (etc) for which the HOSTPROG
  | definition is being used to eliminate.   That's bogus - when st_birthtime
  | was added, something like
  | 	#define	_FEAT_ST_BIRTHTIME
  | should have been added as well, then stat.c could simply do
  | 
  | #ifdef _FEAT_ST_BIRTHTIME
  | #endif
  | 
  | and be portable.

Sure that could be done, for all the non-posix st_ fields in stat(1).

However, many people were running into build issues on non-netbsd-current
platforms, and I made the most appropriate and simplest change that
the build system required to fix the problems.  If we waited until
myself or andrew pondered over and the implemented the correct
solution (a few more days, this is volunteer time), we'd be fielding
lots more "oh, but i can't build on xxx host platform", and they
certainly get tedious after a while.


Luke.