Moriah,
I looked only at the changes and these comments are for the webrev2
(http://cr.opensolaris.org/~mwaterl/6739234.changes_only/webrev/ )
Some general comments
1. Several Files have changes only in the # include header files.
The copyrights were not updated.
2. Some files have 2006 in the new copyrights. Is that correct?
usr/src/cmd/Makefile.cmd:
Lines 144 and 145 looks identical.
Lines 314-315 and 324-325 are same.
usr/src/cmd/svr4pkg/Makefile:
31-45: Do these directories have some dependencies? Whether any of
the directories should be built before other directories? If there are
no dependecies, what order should they be listed? Unless there is a
dependency, installf can be moved to the top of the list.
usr/src/cmd/svr4pkg/hdrs/libadm.h and other files:
What about copyright change? You don't change it because of no
change in the code?
usr/src/cmd/svr4pkg/installf/main.c:
Update copyright to 2009.
usr/src/cmd/svr4pkg/libinst/fixpath.c:
Update copyright to 2009.
usr/src/cmd/svr4pkg/libinst/pkgdbmerg.c:
Update copyright to 2009. Is it intentional to change the copyrights
to 2006 (from 2008) and corresponding changes in lines 196-198? I don't
see any explanation for the code change.
usr/src/cmd/svr4pkg/pkgmk/Makefile:
29-32: quit.o goes before splpkgmap.o (alphabetical order)
usr/src/cmd/svr4pkg/pkgscripts/Makefile:
Update copyright to 2009.
32-38: Move r.awk up after i.awk
usr/src/lib/libinstzones/common/zones.c:
Where does the changes come from? Did the install code have a copy
of the same file and modified the code? Is it true for all the files
under usr/src/lib/libinstzones/common directory listed in the webrev?
usr/src/lib/libpkg/common/handlelocalfs.c:
Why did you update the year to 2009 in the copyrights here? I don't
see any code changes. Same comment for p12lib.c, and p12lib.h.
- Sundar
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.
>
>
> 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.
