> > * 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 _______________________________________________ opensolaris-code mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
