[illumos-Developer] review: nuke staleness from svr4pkg

[Peter Tribble]

On Fri, Oct 15, 2010 at 7:36 AM, Garrett D'Amore <garrett at nexenta.com> wrote:
> As part of the first round of changes where I'm giving some luvin' to
> the SVR4 packaging command suite.  The first step is to trim a bunch of
> legacy that is not needed (stuff that should have been done *years
> ago*).
>
> The webrev is here:
>
> http://mexico.purplecow.org/gdamore/webrev/presvr4/

psvr4ck.c? The webrev indicates it just contains "/*" rather than just
being wiped.

libadm/pkginfo.c has a Nexenta copyright. Nothing against that, but
why just that file?

In package-svr4.mf you've removed the line
hardlink path=usr/sadm/install/scripts/i.CONFIG.prsv
why?

You've removed the SUNOS41 check from pkgremove/check.c - there are
a bunch of other checks (one in pkgtrans/main.c, something like 15 in libpkg,
aanother one in libadm). If you're going to kill it in one place, why not finish
the job?

> Specifically, what I've removed is:
>
> a) support for checking against prodreg on pkgrm (it created a
> dependency upon a closed, and I believe not-redistributable, bit of
> code-- the admin consolidation.)  In short, we can't use that code.

Something useless and can be a significant performance hit. (And one
reason I built my own fork, just to kill it, because it's actually broken.)

> b) support for "pre-SVR4" packaging.  This would have been used with
> SunOS 4.x.  I've not seen such a package *ever*, even back in my SunOS
> 4.x administration days.  I don't know if it was *ever* used with
> Solaris 2.x.  SVR4 packaging is about 20 years old now.  Do we really
> care about what came *before* it?  Removing this also saves a bunch of
> checks on each packaging operation, so the code will go faster with
> fewer system calls.  (Yay.)

You've missed pgknmchk.c in libadm (although see later for libadm)

Actually, I think this was SVR3 packaging; that's a different evolutionary
track to SunOS 4.x. I planned to do this several years ago, before the
install consolidation stopped releasing code drops. The feeling then was
that even if you could install such a package (and the chances are that
it would just break anyway, not having been tested in over a decade and
lots of other changes having been made), none of the software contained
in it would work anyway.

> f) a "duplicate" header file for libadm.h.  The packaging tools can and
> should just use the libadm.h from libadm itself.  (This did require
> adding two missing prototypes to libadm.h.)

Not a code review comment, but I believe this to be the wrong approach.
I think you want to kill libadm and if there's any functionality you need,
then bring it across to the packaging code directly.

-- 
-Peter Tribble
http://www.petertribble.co.uk/ - http://ptribble.blogspot.com/