have you got an updated webrev for me (AND OTHERS) to check? Don't forget to modify the METADATA first as per Norm's heads-up email to conform to ... "http://wikis.sun.com/display/SFWNotes/METADATA";
paul Mark Phalan wrote: > On Thu, 2009-05-07 at 14:37 +0100, Paul Cunningham wrote: >> Mark, >> >> See comments below ... >> >> paul >> >> Mark Phalan wrote: >>> Find below two webrevs (one each for ON and SFW) for review which move >>> OpenSSL from ON to SFW. >>> >>> http://cr.opensolaris.org/~mbp/6806387-move_openssl/ >> This looks okay to me, but I'm not sure what the new file >> is for ? ... >> usr/src/stand/lib/openssl/Makefile > > The new Makefile is there to pull in the openssl header files so that > WAN boot can still build. As the move removes openssl from ON the proto > area won't have (and shouldn't have) the header files. > >>> http://cr.opensolaris.org/~mbp/6806387-move_openssl_SFW/ >> 1. usr/src/lib/openssl/METADATA >> Change this so it conforms to that defined in .. >> "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; > > Done. > >> 2. Top of files (various files) >> Change the tops of files so that they conform to the >> prototypes in ... >> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; > > > I mostly just modified the Makefiles. Do you see any other files which > need modification? > >> 3. usr/src/lib/openssl/Makefile.sfw >> Delete as is already defined in Makefile.master ... >> 26 SHELL=/usr/bin/ksh93 > > Done. > >> Lines ... >> 81 $(VER64)/Configure: $(VER).tar.gz >> 182 mkdir -p tmp >> 183 gzip -dc $(VER).tar.gz | .... >> replace '$(VER).tar.gz' with '$(TARBALL)' and >> else where. >> > > Done. > >> 4. usr/src/lib/openssl/install-sfw >> Lines ... >> 35 mkdir -p ${LIBDIR} >> 36 mkdir -p ${BINDIR} >> 37 mkdir -p ${ROOT}/etc/sfw >> 38 mkdir -p ${ROOT}/usr/sfw/bin >> 39 mkdir -p ${ROOT}/usr/sfw/lib >> these should already be in Targetdirs so are not required >> here, delete > > Done. > >> Line ... >> 56 chmod 0755 $bin >> do not install with write permission bit set > > Not totally sure what you mean here. The "chmod" changes the permission > so post_process_so can be run. Later protofix will fix up the > permissions. > >> Line .. >> 80 gpatch ${ROOT}/usr/include/openssl/opensslconf.h \ >> opensslconf.patch >> shouldn't this be done before its installed into the proto >> area? > > Could be. It certainly needs to be patched after "Configure" is run. Do > you want me to change it? > >> Lines 67, 68, are there any libraries that need symbolic links >> into /usr/sfw/lib ? > > Nope. > >> 5. usr/src/lib/openssl/install-sfw-64 >> Lines ... >> 35 mkdir -p ${LIBDIR} >> 36 mkdir -p ${BINDIR} >> 37 mkdir -p ${USRLIBDIR}/pkgconfig >> 38 mkdir -p ${ROOT}/usr/sfw/bin/${MACH64} >> 39 mkdir -p ${ROOT}/usr/sfw/lib/${MACH64} >> these should already be in Targetdirs so are not required >> here, delete. >> >> > > Done. > >> Line ... >> 55 chmod 0755 $bin >> do not install with write permission bit set >> > > See (4). > >> I'm not sure about this ... >> 41 mv -f ${ROOT}/usr/bin/openssl ${BINDIR}/openssl >> 42 mv -f ${ROOT}/usr/lib/libcrypto.so* ${LIBDIR}/ >> 43 mv -f ${ROOT}/usr/lib/libssl.so* ${LIBDIR}/ >> 44 mv -f ${ROOT}/usr/lib/pkgconfig/openssl.pc ... >> isn't it moving the 32 bit stuff? > > Actually it isn't. This is done to move the 64bit binaries into the > proper directories. OpenSSL will always install into ${ROOT}/usr|lib etc > even if the build is 64 bit. > >> 6. patches >> I haven't looked at them; I've assumed they are correct > > I hope so :) The patches are mostly about making OpenSSL end up being > identical to what is currently in ON. > >> 7. usr/src/lib/openssl/llib-lcrypto >> & usr/src/lib/openssl/llib-lssl >> Shouldn't this have the CDDL header stuff? > > I don't think so. It doesn't have it in ON and anyway there isn't any > code there to license. > >> 8. usr/src/pkgdefs/Makefile >> This needs resyncing with the gate, so it doesn't look >> as though you are changing other stuff. > > Done. > >> 9. usr/src/pkgdefs/SUNWopenssl-commands/Makefile >> & usr/src/pkgdefs/SUNWopenssl-include/Makefile >> Does it need these lines ? ... >> 29 LICENSEFILES += ../../common/openssl/LICENSE >> 30 CDDL= > > Removed the LICENSEFILES as the openssl license is now included directly > in the "copyright" file. > I've left the CDDL=. It seems to be a common usage in packages in ON. > >> 10. usr/src/pkgdefs/SUNWopenssl-commands/depend >> & all others 'depend' files >> Copyright and ident are in the wrong place > > Done. > >> 11. usr/src/pkgdefs/SUNWopenssl-commands/pkginfo.tmpl >> & all others >> Add version to the of DESC= line, eg. ... >> DESC="............ (0.9.8a)" >> > > Done. > >> 12. usr/src/pkgdefs/SUNWopenssl-commands/prototype_com >> & usr/src/pkgdefs/SUNWopenssl-commands/prototype_sparc >> & usr/src/pkgdefs/SUNWopenssl-commands/prototype_i386 >> Move creation of symbolic link to after its >> target is put in place. >> > > Done. > >> 13. SUNW*/copyright >> It's not normal to include this unless it refers to the >> source ... >> 1 Copyright 2009 Sun Microsystems, Inc. All rights reserved. >> 2 Use is subject to license terms. >> > > I was duplicating ON here. I've removed the Sun copyright from every > package except SUNWopensslr as it contains a significant patch written > by Sun. > >> 14. SUNW*/depend >> Its normal to include all the dependencies from >> the default 'depend' also (as appropriate) >> > > Where is the default 'depend'? > >> 15. usr/src/pkgdefs/SUNWopenssl-include/prototype_com >> & usr/src/pkgdefs/SUNWopenssl-include/prototype_i386 >> & usr/src/pkgdefs/SUNWopenssl-include/prototype_sparc >> Do not install files into /usr with the write permission >> bit set. > > I probably agree with you that this is a bug however most header files > in /usr/include have the write bit set already: > > $ find /usr/include -type f -perm 644 | wc -l > 9094 > > I'd rather leave it as is for the moment and fix it in a follow-up push > after the move. OpenSSL is a very used piece of software in the core of > Solaris, minimizing impact when moving from ON to SFW is a high priority > to me. > >> 16. usr/src/pkgdefs/SUNWopensslr/postinstall >> & usr/src/pkgdefs/SUNWopensslr/preinstall >> Why /sbin/sh instead of /bin/sh ? > > I don't know - I just moved it from ON. I'll ask the original author. > >> 17. usr/src/pkgdefs/SUNWopensslr/prototype_com >> & usr/src/pkgdefs/SUNWopensslr/prototype_i386 >> & usr/src/pkgdefs/SUNWopensslr/prototype_sparc >> Do all the files need the write permission bit >> set ? > > See (15). > >> 18. usr/src/pkgdefs/SUNWopensslr/r.opensslcnf >> Is this required, it does not seem to do anything? > > See (16). > >> 19. usr/src/pkgdefs/SUNWopenssl-libraries/prototype_com >> & usr/src/pkgdefs/SUNWopenssl-libraries/prototype_i386 >> & usr/src/pkgdefs/SUNWopenssl-libraries/prototype_sparc >> Why are there no libraries in ? ... >> /usr/sfw/lib/sparcv9 >> /usr/sfw/lib/amd64 >> /usr/sfw/lib > > This looks like a bug introduced with: > 6449514 move OpenSSL from /usr/sfw to /usr, /lib > > I'll investigate and file a bug but may not fix that issue with my > putback. > > Thanks for the review! > > -M > -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
