Ali, See my comments below from my quick *skip* through ...
Paul On Sat, 2008-08-02 at 00:54, Ali Bahrami wrote: > I'm working on getting GNU emacs integrated into the Solaris SFW > consolidation. Once this is done, it will be available both in Solaris Nevada, > and in OpenSolaris. > > The PSARC case is underway: > > http://www.opensolaris.org/jive/thread.jspa?threadID=68166 > > I would like to request an I-Team code review. The webrev is available at: > > http://cr.opensolaris.org/~alib/emacs/ > > I have done full nightly builds of sfw with these changes on both > x86 and sparc, without error. > > Although I have not yet started the C-team Review process, that process > also includes a code review. I believe that this workspace is ready to > for that as well, so I would like to also ask for a code review from the > members of sfwnv-discuss. > > The changes fall into several different categories. Somewhat gratifying > is the fact that there were very few changes to the files in the emacs > distribution itself: > > - How to get the emacs configure process to work for amd64 > - How to build emacs for all the different X11 toolkits > and collect the results into the proto. > - How to deliver both 32 and 64-bit emacs binaries > and determine which one to use at runtime (isaexec). > - How to select which version of emacs is the default > run by /usr/bin/emacs. > - Minor patches to use the system malloc, and Solaris dldump(). > - Determining which files go into which Solaris packages, > and the details of building those packages. > - How to produce a version of emacs built with Athena > widgets (Xaw) that will run on OpenSolaris, where > libXaw is missing. > - Mechanical packaging details > > Details of these changes are described in README.SUNWemacs: > > > http://cr.opensolaris.org/~alib/emacs/usr/src/cmd/emacs/README.SUNWemacs.html > > I've just subscribed to solaris-qe-pkgproj at sun.com and sfwnv-discuss at > opensolaris.org. > As there is a small lag between when one joins a list and the emails start > arriving, > please keep my email address in your replies for now so that I won't > miss any of the early ones. === Start of Comments ==== 1. usr/src/cmd/Makefile & usr/src/pkgdefs/Makefile This looks as though it needs resyncing with the gate, so it doesn't look as though you are try to delete stuff. 2. usr/src/pkgdefs/SUNWgnu-emacs-*/pkginfo.tmpl You don't need the version number on the NAME= lines 3. usr/src/cmd/emacs/Makefile.sfw This invokes '$(SH) install-sfw' but that is not in your webrev. 4. usr/src/cmd/emacs/augment/man/man1/*.1 Were these part of the original src tarball, if so why have you not used them from there rather than having your own copy of them? 5. src/cmd/emacs/Makefile.sfw Lines 142 to 147 '.... make install'; does the package's Makefile allow you to use something like 'make DISTDIR=$(LPROTO) install' rather than having to set prefix, bindir, etc. ? Is INSTALL_ROOT root (/) or is it the proto area? (I forget); if it's the proto area shouldn't it be root (/) ie. where it really runs from. === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
