This is my last feedback. I explicitly did NOT review the source code, but concentrated on makefiles and packaging.
usr/src/cmd/svr4pkg/pkgscripts: - What is "special_contents" used for? (Or is it unreferenced?) - None of the scripts specify an interpreter--I think they should. (#!/bin/sh) - Usually, if you want a script to be executable, you name it "blah.sh," and use the ".sh:" suffix rule to create "blah." You wouldn't even need to update the Makefile to accomplish this--simply rename the scripts, and it should work correctly. usr/src/lib/libinstzones/Makefile: usr/src/lib/libpkg/Makefile: _msg: Since you're already enumerating the object files in Makefile.com, and you're preprocessing for message extraction, it makes much more sense to handle _msg in Makefile.com, instead of here. And if you're really being clever, for libinstzones, you only need to extract from zones_strings.h, using the -a flag for xgettext, and not calling the preprocessor. install_h: both of these makefiles have an install_h target, but they're not listed in HDRSUBDIRS in usr/src/lib/Makefile. If you don't plan to put the headers into the proto area, you can skip including Makefile.lib and Makefile.targ, and get rid of the HDRS and HDRDIR definitions. usr/src/lib/libinstzones/common/mapfile-vers: - please alphabetize It looks like you're adding lint libraries to the proto area, but I don't see where you're delivering them. If you plan for anything outside of ON to use them, they belong in SUNWarc. If not, they need to be in a packaging exception list, don't they? --Mark Moriah Waterland wrote: > Mark, Jim, and Sundar, > > Thank you for agreeing to review my changes related to move of the > SVR4 packaging code from the Legacy Nevada Install gate into ON. > > I am sure that I have already mentioned the following but here is a > quick overview: > > o This project is about moving the SVR4 packaging code to ON because > this code needs to live on and we need to move away from the > Legacy Install consolidation. > > o Patch code is not moving and is already dead in OpenSolaris. > David Comay is stopping the delivery of patch binaries in the > 2009.06 release. > > o The only goal of this project is moving the code. Yes, I know > that this means that problems are just moving from one place to > another but these issues must be dealt with separately. > > o Testing is on-going (Mary Ding has images she is working with and > I have additional tests I want to run etc..). I am reasonably > confident that this will not be an issue and I expect that if > changes need to be made they will be small. In other words, I > think that it is safe for you to start reviewing this now. > > I am cc'ing this code review to install-discuss at opensolaris.org as well > as several internal aliases. Please let me know if you think of other > aliases that this should also see this. Once this posts to > install-discuss, I will update the OpenSolaris project page for SVR4 > packaging with a link to this message. > > The bug report contains information about the reasons behind a lot of > the individual changes. I have also included it in this email for your > convenience. > > BugId: ( http://monaco.sfbay/detail.jsf?cr=6739234 ) > > 6739234 move SVR4 packaging to ONNV gate > > Webrev 1: This is a simple webrev of my changes to ON. It shows all the > new files that I am adding to ON as well as a the small number of > modifications that I made to several existing ON files. > > http://cr.opensolaris.org/~mwaterl/6739234.complete/webrev/ > > The first webrev shows all of the changes to ON but it doesn't show the > actual source modifications that I made to individual files. So I > generated a second webrev that should help highlight the actual source > changes that were required. > > > Webrev 2: This webrev was created by taking a clean ON child and adding > the files directly from the Legacy Nevada Install gate. These files > were inserted at their new locations. Then the updated files from the > SVR4 packaging move were added to the workspace so that reasonable diffs > could be generated. > > http://cr.opensolaris.org/~mwaterl/6739234.changes_only/webrev/ > > > Please get your feedback to me by COB on Friday, May 1st. Thanks again > for your help with this code review. > > -Moriah > > > ======================================================================== > ======================================================================== > > > Suggested Fix > ======= > > The location changes for the source code are as follows: > Legacy Nevada Install gate: (The layout of the code in the old gate.) > o /ws/install-nv-gate/usr/src/ > o cmd/pkgcmds/ > hdrs/ > installf/ pkgcond/ pkgname/ > pkgrm/ pkgadd/ pkginfo/ > pkgparam/ pkgscripts/ pkgadm/ > pkginstall/ pkgproto/ pkgtrans/ > pkgchk/ pkgmk/ pkgremove/ > > o lib/libpkg/ > o lib/libspmizones/ > o lib/libinst/ > > o pkgdefs/SUNWpkgcmdsr > o pkgdefs/SUNWpkgcmdsu > || > || > \/ > Nevada ON gate: (The new layout for the code in ON.) > o /ws/onnv-gate/usr/src/ > o cmd/svr4pkg/ > hdrs/ libinst/ > installf/ pkgcond/ pkgname/ > pkgrm/ pkgadd/ pkginfo/ > pkgparam/ pkgscripts/ pkgadm/ > pkginstall/ pkgproto/ pkgtrans/ > pkgchk/ pkgmk/ pkgremove/ > > o lib/libpkg/ > common/ i386/ sparc/ > > o lib/libinstzones/ > common/ i386/ sparc/ > > o pkgdefs/SUNWpkgcmdsr > o pkgdefs/SUNWpkgcmdsu > > My changes for this code move only pertained to what was absolutely > necessary for this migration. I am aware that there are quite a few > other issues that could be fixed in this code, but they are out of scope > for this project.
