Moriah Waterland wrote: > Mark, > > My response is broken-up into two sections. Here is the first > installment of my response. > > thanks, > Moriah > > > Mark J. Nelson wrote: >> >> Please be sure to run "hg pbchk" on your repository, and fix what it >> reports. There are currently some copyright errors, and you'll want >> to whack keywords and make sure CDDL is up to date. It's OK to NOT >> update copyright for files that you're not changing, but please do >> that intentionally, and let your CRT Advocate know. >> > Done, umm sort of. I cleaned up the CDDL problems. I inserted header > guard as well as _cplusplus clause into: > o usr/src/lib/libpkg/common/dbtables.h > There is a multitude of cstyle and lint errors throughout legacy > packaging code. However, I did scrub libinstzones so that it is now > lint clean as well as compliant with the ON cstyle standard. > I also verified that hg pbchk does not complain about any keywords.
Ok. You also added libinstzones to usr/src/Makefile.lint? >> usr/src/cmd/Makefile.cmd >> 62: Your ROOTUSRBIN is redundant with preexisting ROOTBIN. Basically, >> all of your changes to this file that do not involve ROOTUSRSADM are >> unnecessary, in this and other Makefiles. > Fixed. Removed ROOTUSRBIN and its associated targets. Thanks. >> 328-330: You should not need this. ROOTUSRSADMSCRIPT should not refer >> to a directory, but instead to a list of files to be installed in the >> ROOTUSRSADM directory. > This ended up being kinda problematic. I ended up with a solution > similar to what was in the Legacy Install gate. Additionally, I found > something else under usr/src/cmd that was also installing Class Action > scripts in the same place and modeled my changes in pkgscripts/Makefile > after: > usr/src/cmd/cmd-crypto/scripts/Makefile I'll want to see a followup webrev. This shouldn't have caused significant problems. >> usr/src/cmd/svr4pkg/Makefile >> 30 (old file): I'm fine with you moving your message into the >> SUNW_OST_OSCMD domain. Where was SUNW_PKG_CMDS.po (and friends?) >> previously delivered? Was there a SUNW0install or SUNW0adm package >> that you'll need to either modify or obsolete? > When this fix integrates into the ON gate, the following files need to > be removed from the package SUNW0adm > o usr/lib/locale/C/LC_MESSAGES/SUNW_PKG_CMDS.po > o usr/lib/locale/C/LC_MESSAGES/SUNW_PKG_LIBPKG.po > > Also, since SUNWpkgcmdr and SUNWpkgcmdu will now be delivered from ON, > so those packages need to be removed from the list of packages > integrated from the Legacy Install gate. I believe that I will need to > coordinate with the ON gate staff as well as RE. Yep. >> 44: please alphabetize this list > Done Thanks. >> 65-83: please delete, and update usr/src/Targetdirs appropriately >> (note that "please delete" includes CPPFLAGS, since you're not using >> them here) > Done. Updated Targetdirs and I gutted svr4pkg/Makefile and please see > Note 0. Thanks. >> 87-91: instead of >> >> 87 #all clean clobber lint: $(SUBDIRS) >> 88 all clean clobber lint: $(LIBSUBDIR) .WAIT $(CMDSUBDIRS) >> 89 >> 90 #install: $(INSDIRS) .WAIT $(SUBDIRS) .WAIT _msg >> 91 install: $(INSDIRS) .WAIT all .WAIT _msg >> >> this section should read simply >> >> 49 SUBDIRS= $(CMDSUBDIRS) $(LIBSUBDIR) >> >> 87 $(CMDSUBDIRS): $(LIBSUBDIR) >> 88 >> 89 all clean clobber install lint _msg: $(SUBDIRS) >> >> ...and line 89 should only include the lint target if it actually >> works correctly. (That's "correctly," not "cleanly," you already >> explained that this stuff isn't lint-clean.) > Fixed, please see Note 0. Thanks. >> Note that the definition of SUBDIRS will make lines 103-110 and >> 114-116 unnecessary. And you won't need a separate MSGSUBDIRS, since >> it's the same as SUBDIRS. >> > Fixed, I removed the definition for MSGSUBDIRS. Thanks. >> _msg: >> - As implied by the suggested line 89 above, the install target should >> not depend on _msg. > Fixed. Thanks. >> - The pkgname command ended up with no messages, so you could >> conceivably leave it out of MSGSUBDIRS, but it's not a big deal for it >> to be there. > Left alone. Because I removed MSGSUBDIRS I am just executing all > targets for each of the SUBDIRS. Ok. >> - instead of pkgcmds.po, please use svr4pkg.po to be consistent with >> directory name > Done, I just removed the top level .po file and directly install each of > the .po files from the subdirectories into the proto area Great. >> - add "include ../Makefile.targ" after the include for Makefile.msg.targ > Done Still needed, with _msg deferred to SUBDIRS and no headers to install? >> - remove 117, as it's covered by ../Makefile.targ > Done > >> 111-112: Just replace these lines with "CLOBBERFILES += $(POFILE)" > Fixed, added definition: > CLOBBERFILES += $(PROG) $(POFILE) > to usr/src/cmd/svr4pkg/Makefile.svr4pkg Great. > Note 0 > ====== > Now, my usr/src/cmd/svr4pkg/Makefile is arranged like the following > excerpt: > > .PARALLEL= $(CMDSUBDIRS) > > LIBSUBDIR= libinst > SUBDIRS= $(CMDSUBDIRS) $(LIBSUBDIR) > > all clean clobber install _msg lint: $(SUBDIRS) > > $(CMDSUBDIRS): $(LIBSUBDIR) > > $(SUBDIRS): FRC > @cd $@; pwd; $(MAKE) $(TARGET) This looks good, and should not need any other Makefiles included. I'll look in the followup webrev. --Mark
