[Fwd: [ksh93-integration-discuss]ksh93-integrationpre-reviewround"two" (webrev 2007-05-14)]

[Peter Memishian]

 > >         * There's no such thing as ROOTLINT64.  Please make sure it gets
 > >           removed from any Makefiles (e.g. libshell/amd64/Makefile).
 > 
 > Erm... why do most other libraries use this (I've followed the behaviour
 > of other libraries in this case - see

Those are bugs; see 4456822.  I'd strongly recommend following the advice
in README.Makefiles rather than examining existing Makefiles (which are
often highly erroneous).

 > >         * Seems like Makefile.astinclude and Makefile.libastl10n belong in
 > >           usr/src/lib, since they're used by more than libast.  (And maybe
 > >           Makefile.libastl10n should be renamed to Makefile.astmsg?)
 > 
 > Uhm... I tried to avoid shattering files all over the place and put the
 > AST-specific Makefile fragemnts into usr/src/lib/libast/ since this is
 > the base library for the rest of the AST stuff..
 > ... Ok: "move&&rename" or "leave the current layout" ?

Ideally, neither.  I'd instead prefer something like what we did with
lib/lvm, where all of the libraries sharing a given Makefile "architecture"
would exist below lib/ast (e.g., lib/ast/libast, lib/ast/libpp, ...), and
then the two Makefiles could live under lib/ast/Makefile.ast*.  But I'm
guessing you considered that approach and had an issue with it -- so the
next best thing would be to have those Makefiles live directly in lib.

 > > cmd/ast/Makefile
 > > 
 > >         * 40-42: Who uses the `check', `_msg', and `_dc' targets here?
 > 
 > Currently none uses them but the l10n targets are used in the future
 > once we ARC'ed the missing catalog files (welcome to PSARC 2xxx/666
 > "ksh93 Amendments II") ... and since this is a "base" subdir where it's
 > Makefile should only pass-through all targets I copied all relevant
 > targets into the list...

But you're going to have to update the Makefile anyway to handle that,
since there are no actual `check' `_msg' or `_dc' targets in that
Makefile.  So please just remove them and add them back when you have
a use for them.

 > >         * 73-78: Seems like this ROOTAST* business could just be replaced
 > >           with `ROOTCMDDIR=$(ROOT)/usr/ast/bin'
 > > 
 > >         * 81: Change `ASTPROG=' to `PROG='.  Then change `$(ASTPROG)' to
 > >           `$(PROG)' on line 83 and $(ROOTASTPROG) to $(ROOTCMD) on line 84
 > 
 > Isn't this an abuse of ROOTCMD ? There is no precedent for such a thing
 > and that's why I copied the current approach from other Makefiles.

As the person who invented ROOTCMD, I assure you it's exactly how I
intended it to be used ;-)  Also, there is precedent -- e.g.,
cmd-inet/usr.lib/wanboot/Makefile.com, cmd-inet/lib/nwamd/Makefile, ...

 > > lib/libshell/Makefile.demo:
 > > 
 > >         * Seems like the ROOTDEMO* logic is generic, and should be folded
 > >           into lib/Makefile.lib.
 > 
 > 1. Erm, I avoided "global" changes like that to avoid that we run into
 > trouble with other, parallel changes in the tree (there are other items
 > which could be implemented better _if_ I would feed "safe" to touch more
 > global things... for example OS/Net has "CLOBBERFILES" to clobber
 > files... but it has no "CLOBBERDIRS" to cleanup directories in the
 > "clobber" target etc. (and that's not the only item of things which need
 > IMO cleanup...)).

I understand your concern, but this one seems pretty safe.  My worry is
that someone will cut-and-paste that ROOTDEMO* stuff into something else
and it will start to spread.  I've seen this happen many times and I'd
like to not repeat that mistake again.

--
meem