[illumos-Developer] review: nuke staleness from svr4pkg

[Garrett D'Amore]

On Sun, 2010-10-17 at 21:43 +0100, Peter Tribble wrote:
> 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.

Oops.

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

It was one file... I'll probably add it to others *if* I add
functionality or make non-obvious changes.  I probably should remove it
from this file, since all I did we was remove functionality.

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

Nothing was using it, and the Makefile to create it is removed.  I have
no idea why it was ever delivered in the first place.

> 
> 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?

Ah, I just noticed that one in particular.  I didn't go on a hunt.  I'll
do that now though.

> 
> > 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)

Ok, I'll check into it..  If there are other places I can clean stuff
up, I'd like to. :-)

> 
> 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.

Oh wow.  Well, its best removed in any case.

> 
> > 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.
> 

There are other programs (fstype?) that use libadm.

In fact, I'd like to gut the whole device.tab thing, but that's another
windmill for a different day.  If/when we tackle that, *then* I'll gut
libadm most likely. ;-)

	- Garrett