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
