Paul Cunningham wrote: > Huie-Ying, > > See below comments from my very quick skip through .. > > Paul > Hello Paul,
Nice to hear from you again. 8-) > Huie-Ying Lee wrote: >> >> Please help review code for the pyOpenSSL package which is to be >> integrate to SFWNV consolidation. >> >> PSARC case: 2008/705 >> Bug id: CR6759597 include pyopenssl >> Webrev: http://cr.opensolaris.org/~hylee/pyopenssl/ > > === Start of Comments ==== > > 1. usr/src/lib/pyopenssl/Makefile.sfw > Change 'env ' to 'env - ' > > Combine the two 'clean' 'rm' lines, eg .. > clean: > -rm -rf $(VER) $(VER64) > > You could extract the VER= info from the METADATA, see > recent sfwnv integrations on how to do that. > > I don't think you need this ... > 68 ($(INSTALL_PROTO) -d $(ROOTPYTHONMODS_DOC); > as its created from content of Targetdirs ? > OK. I have fixed all 4 problems in Makefile.sfw. > 2. usr/src/pkgdefs/SUNWpyopenssl/pkginfo.tmpl > You could put the version number at the end of the DESC= > line as most people do, eg ... > DESC="........... (0.8)" > OK. The version numbe is added to the DESC field. > 3. usr/src/pkgdefs/SUNWpyopenssl/prototype_com > Do you need the 'write' permission on the files, if not > remove it! No, the write permission is not needed on the files. However, if I change the permission to be 444, there will be build errors about the permission inconsistency between the proto area and the package files, because the pyOpenSSL default install script choose 644 to the permission for those script files. > Just an observation ... > 48 d none usr/lib/python2.4/vendor-packages/OpenSSL/64 > in /usr/lib, 64 would be a symbolic link to > the appropriate arch dir - maybe yours should be to. > The current arrangement is to follow the setup in the existing usr/lib/python2.4/vendor-packages/64 directory, which does not contain any arch dir. The "64" directory works fine for both sparcv9 and amd64. > usr/lib/python2.4/vendor-packages/OpenSSL/test/* > Isn't it normal to put 'test' stuff into a separate > package if you really need to deliver it (but I may be > remember incorrectly). > These test files serve as samples. Since there are only a couple of them, I think it makes sense to include them in the same package, as stated in the PSARC case. > === End of Comments ====== > I will make the changes and send out an updated webrev once I complete the build and testing with the new source. Thank you very much for your thorough review. Huie-Ying
