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
Feedback for all files under usr/src/pkgdefs: SUNWpkgcmdsr/pkginfo.tmpl: 36: The pkginfo lists manifest and logadmconf classes, but no object in the prototype_* files have these classes. Please remove them from CLASSES, which should simply be "none" in this case. 37: Of 500 ON packages surveyed, three omit the double quotes around DESC, and the remaining 497 include them. Please include them, though it's only cosmetic. SUNWpkgcmdsr/prototype_com: - This package delivers a single file (var/sadm/install/admin/default) and a handful of directories. Does it really need even these dependencies? (Not a big deal, just curious.) SUNWpkgcmdsr/prototype_com: 34-36: These lines should not use the "=name" convention. SUNWpkgcmdsu/pkginfo.tmpl: 37: Should be VERSION="ONVERS,REV=0.0.0" 35, 44-46: Same cosmetic comment re: quotes. SUNWpkgcmdsu/depend: - Really? I'm not a technology expert, so I'll take your word for it, but I was surprised at this dependency list. SUNWpkgcmdsu/prototype_com: 80: We deliver a compilation symlink for this? Do we actually ever want anybody outside of the (soon-to-be) ON build process to link to it? Anything not mentioned looked fine. --Mark > ======================================================================== > ======================================================================== > > > 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. > > > Comments > ======= > The following notes are intended to provide more detail about why I made > certain changes: > > o Changed the name of the directory under cmd from pkgcmds to > svr4pkg to disambiguate this code from the new IPS packaging code. > > o Moved libinst out of libraries and into cmd/svr4pkg/libinst. Libinst > had compilation problems under the libraries directory in the ON > gate because it calls functions that are defined in code under > cmd/svr4pkg. Spoke with Dave Marker about situation and he suggested > this move because libinst is statically compiled, not delivered, and > will only be used by the package commands. > > o Leaving libinstzones (formally known as libspmizones) as NOT large > file aware. To track this, I filed: > 6833675 libinstzones is not large file aware > > o Created lint libraries for libinstzones and libpkg. Not linting > code under usr/src/cmd/svr4pkg. Changes required for this are out of > scope for this project. To track this, I filed: > 6833677 SVR4 package commands are not lint clean > > o I took changes to libinstzones directly from libinstzones in the > Caiman gate. This is where the library name was changed. > http://src.opensolaris.org/source/xref/caiman/slim_source/ > > o Moved the packages SUNWpkgcmdsr and SUNWpkgcmdsu from the install > gate to the ON gate. The only significant difference in what the > packages are delivering is the name change of libspmizones to > libinstzones. All references to libspmizones in the packaging code have > been changed to libinstzones. > > o Removed dependency from the packages for SUNWadmc as it is not required. > > o Used default text domain for the svr4pkg libraries or commands > because it didn't seem reasonable to define a separate domain just > for this old code. > > o Once this code is putback to ON, deliveries of SUNWpkgcmdsu and > SUNWpkgcmdsr from the Legacy Install gate will cease. The code will not > be yanked from the Legacy Nevada Install gate because the gate is in > maintenance mode until it is completely decommissioned.
