Moriah Waterland wrote: > I have generated a new webrev for: > 6739234 move SVR4 packaging to ONNV gate > > This webrev includes the changes that I detailed in my responses to code > review comments. > > Updated Webrev (2nd ed.): > [http://cr.opensolaris.org/~mwaterl/6739234.changes_only.2/webrev/] > > Old Webrev: > [http://cr.opensolaris.org/~mwaterl/6739234.changes_only/webrev/] > > Jim, Mark, and Sundar: I would appreciate an explicit "ok" from each of > you before noon(PT) on Tuesday, May 26th. > > thanks, > Moriah
I used the .3 version of the webrev. usr/src/cmd/Makefile: - Looks like you lost the change adding svr4pkg to MSGSUBDIRS? usr/src/cmd/Makefile.cmd: 314: Where is ROOTPKGSCRDIR defined? 483: Where is ROOTVARSADM defined? usr/src/cmd/svr4pkg/Makefile: - Has something gone wonky with the workspace? This isn't the makefile that belongs here, it's the one that should be in pkgtrans, isn't it? usr/src/cmd/svr4pkg/Makefile.svr4pkg.targ: 40: You add $(PROG) to $(CLOBBERFILES) in Makefile.svr4pkg, so you shouldn't need to list it separately here. - I thought that you planned to leave the lint target in place? usr/src/cmd/svr4pkg/libinst/Makefile: 32: I had previously suggested that ARFLAGS should be "r," rather than "cq." I couldn't find a reply from you saying why you chose this, but I also couldn't seem to find the "part 1" of your reply to my comments. 54-56: I had previously commented that this, too, seemed wrong--there's a ".l.i" suffix rule in usr/src/Makefile.master, which should make this unnecessary. If (when?) you fix this, you'll want to fix your clean target, too, to get rid of this .c file. 67: Why are you redefining CSTYLE? usr/src/cmd/svr4pkg/pkgscripts/Makefile: 46-49: Why bother to rename this to correspond to the directory? You're already getting "cmdexec.po" from Makefile.svr4pkg, that should still work. Especially since you're NOT extracting messages from the scripts. 51-52: These are no longer necessary; they're in Makefile.svr4pkg. 59-61: Order should be swapped; "all" should be default target. - I thought you had agreed to follow my earlier suggestion to rename the scripts with ".sh" extensions? In which case they should also be dependencies of "all"? usr/src/lib/libinstzones/Makefile: - I thought you had decided to pass "_msg" to SUBDIRS, instead of building it here. If you do it here, then you must also deal with clean and clobber of the .i files and POFILE, respectively. usr/src/lib/libpkg/Makefile: - Same comment about deferring _msg build, and/or cleaning up after it. 28-29, 52: This doesn't make any sense unless you add libpkg to HDRSUBDIRS in usr/src/lib/Makefile. 66: Since you're appending to CPPFLAGS, you should not add CPPFLAGS.master here. usr/src/lib/libinstzones/common/mapfile-vers: usr/src/lib/libpkg/common/mapfile-vers: - SUNWprivate is not usually versioned; why is that done here? usr/src/lib/libpkg/THIRDPARTYLICENSE: - Did you talk to Jim about this? You need this here just because you link to OpenSSL, or is there additional code? The copyrights in p12lib.[ch] are accurate? usr/src/pkgdefs/Makefile: - 199-200: please alphabetize usr/src/pkgdefs/SUNWinstallint/pkginfo.tmpl: - You should still define the zones-related parameters, even for an internal package. usr/src/pkgdefs/SUNWpkgcmdsr: 36: You're naming classes that you're not actually using in this package. Please remove logadmconf and manifest from CLASSES. usr/src/pkgdefs/SUNWpkgcmdsu/prototype_com: 76: please alphabetize usr/src/pkgdefs/SUNWpkgcmdsu/pkginfo.tmpl: 37: Should be "ONVERS,REV=0.0.0"
