Spec URL: http://kalev.fedorapeople.org/libdigidocpp.spec SRPM URL: http://kalev.fedorapeople.org/libdigidocpp-0.3.0-1.fc14.src.rpm Description: libdigidocpp is a C++ library for reading, validating, and creating BDoc and DDoc containers. These file formats are widespread in Estonia where they are used for storing legally binding digital signatures. Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2526323
I think I'm already well familiar with this family of projects :), so I'm taking this for the review too.
Awesome, thanks. Please pay special attention to the naming of binding packages, I'd like you to doublecheck if I got the naming right. I chose to use <language>-digidoc as digidoc is the name used for importing the module in those languages; for example in python you'd type 'import digidoc'.
REVIEW: Legend: + = PASSED, - = FAILED, 0 = Not Applicable + rpmlint is silent кwork ~/Desktop: rpmlint libdigidocpp-* perl-digidoc-0.3.0-1.fc15.i686.rpm php-digidoc-0.3.0-1.fc15.i686.rpm python-digidoc-0.3.0-1.fc15.i686.rpm libdigidocpp.i686: E: explicit-lib-dependency libdigidoc(x86-32) libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/ESTEID-SK.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/ESTEID-SK 2007 OCSP 2010.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/schema/conf.xsd libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/EID-SK 2007 OCSP.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/ESTEID-SK 2007.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/EID-SK.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/schema/datatypes.dtd libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/KLASS3-SK 2010.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/TEST-SK.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/schema/xmldsig-core-schema.xsd libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/EID-SK 2007.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/TEST-SK OCSP 2005.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/KLASS3-SK 2010 OCSP.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/schema/XMLSchema.dtd libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/KLASS3-SK OCSP 2009.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/EID-SK 2007 OCSP 2010.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/schema/XAdES.xsd libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/JUUR-SK.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/EID-SK OCSP 2006.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/KLASS3-SK.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/ESTEID-SK 2007 OCSP.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/certs/ESTEID-SK OCSP 2005.crt libdigidocpp.i686: W: non-conffile-in-etc /etc/digidocpp/schema/OpenDocument_manifest.xsd libdigidocpp-devel.i686: W: no-documentation perl-digidoc.i686: W: no-documentation php-digidoc.i686: W: no-documentation php-digidoc.i686: W: non-conffile-in-etc /etc/php.d/digidoc.ini python-digidoc.i686: W: no-documentation 6 packages and 0 specfiles checked; 1 errors, 28 warnings. work ~/Desktop: * All messages about non-conffile-in-etc in package "libdigidocpp" should be ignored - these files are not intended to be changed. In fact I believe that they should be installed in %{_datadir} - consider it in the future releases. * The message "non-conffile-in-etc" from php-digidoc should be ignored too - it contains library name for dlopening (I suppose) and also should not be changed. * The messsage "explicit-lib-dependency" really worries me - could you, please, explain why you need to explicitly add this dependency? + The package is named according to the Package Naming Guidelines. As for language-specific bindings, you reasonably added both lang-%{name} and %{name}-lang provides so no issues here. + The spec file name matches the base package %{name}, in the format %{name}.spec. +/- The package should meet the Packaging Guidelines but there is something else wwhich worries me - you filtered list of automatic provides. Could you explain the reasons? + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license (LGPLv2 or later). Also look at my note regarding bundled library. + The file, containing the text of the license(s) for the package, is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package, match the upstream source, as provided in the spec URL. Sulaco ~/rpmbuild/SOURCES: sha256sum libdigidocpp-0.3.0.tar.bz2* 3147e8b657507db4d30d20b46a864c673b0bcd41b8e5530bdec6171dbfccf116 libdigidocpp-0.3.0.tar.bz2 3147e8b657507db4d30d20b46a864c673b0bcd41b8e5530bdec6171dbfccf116 libdigidocpp-0.3.0.tar.bz2.1 Sulaco ~/rpmbuild/SOURCES: + The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji link above. + All build dependencies are listed in BuildRequires. 0 No need to handle locales. + The package stores shared library files in some of the dynamic linker's default paths, and it calls ldconfig in %post and %postun. - The package bundles copy of minizip library - src/minizip. Please, remove it or explain why it cant be done easily (heavy patched fork, as one of possible explanations). This also add another one license 0 The package is not designed to be relocatable. + The package owns all directories that it creates. + The package does not list a file more than once in the spec file's %files listings. + Permissions on files are set properly. + The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + The package consistently uses macros. + The package contains code, or permissible content. 0 No extremely large documentation files. + Anything, the package includes as %doc, does not affect the runtime of the application. + Header files are stored in a -devel package. 0 No static libraries. +/- The pkgconfig(.pc) files are stored in a -devel package. No pkgconfig runtime requirement was added so this could be a blocker if you plan to use this library on some EL branches. + The library file(s) that end in .so (without suffix) is(are) stored in a -devel package. + The -devel package requires the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} + The package does NOT contain any .la libtool archives. 0 Not a GUI application. + The package does not own files or directories already owned by other packages. + At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + All filenames in rpm packages are valid UTF-8. Ok, almost done. Here is a task list for you: * Explain why you need to explicitly added dependecy on libdigidoc(x86-32). * Explain why you filtered list of automatic provides for language-specific bindings? * Remove (or explain why it wouldn't be easy) bundled minizip. Not a blockers: * Consider storing immutable data (certificates and layout descriptions) in %{_datadir} instead of %{_sysconfdir} * Don't forget to add dependency on pkgconfig if you are planning to build on EL-5.
Thanks for your review. (In reply to comment #3) > Ok, almost done. Here is a task list for you: > * Explain why you need to explicitly added dependecy on libdigidoc(x86-32). libdigidocpp uses dlopen() to access symbols in libdigidoc and opensc libraries. Usually rpm automatically generates Requires for libraries which are in ELF's DT_NEEDED section; libdigidocpp however does NOT use conventional linking to access symbols in those two libraries so we need to specify these Requires manually. Fedora Packaging Guidelines discourage explicitly adding dependencies on libraries which can be automatically generated, but it isn't the case here. > * Explain why you filtered list of automatic provides for language-specific > bindings? It's common practice to filter shared object automatic provides in Perl, PHP, and Python private directories. RPM autogenerates virtual provides for all libraries no matter the location, but the provides are usually not useful for libraries in private directories. It makes sense to have autogenerated provides for libraries which are in dynamic linker's default search path (/usr/lib[64], /lib[64]), but not for private objects in Perl, PHP, and Python directories. RPM uses library virtual provides to find libraries which other programs link against. No program would want to link against the _digidoc.so file; it is loaded dynamically by Perl, PHP, and Python runtimes and the provides would only be confusing. See these two links for rationale: https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Rationale https://fedoraproject.org/wiki/Packaging/Python#Filtering_Requires:_and_Provides: > * Remove (or explain why it wouldn't be easy) bundled minizip. The package actually uses the system copy of minizip, but I'll make sure to remove the bundled minizip in %prep so we can be confident that the private copy doesn't get used. > Not a blockers: > * Consider storing immutable data (certificates and layout descriptions) in > %{_datadir} instead of %{_sysconfdir} Yeah, I'll work with upstream to get this resolved properly. > * Don't forget to add dependency on pkgconfig if you are planning to build on > EL-5.
* Tue Oct 12 2010 Kalev Lember <kalev> - 0.3.0-2 - Remove bundled minizip in prep Spec URL: http://kalev.fedorapeople.org/libdigidocpp.spec SRPM URL: http://kalev.fedorapeople.org/libdigidocpp-0.3.0-2.fc14.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2530364
(In reply to comment #5) > * Tue Oct 12 2010 Kalev Lember <kalev> - 0.3.0-2 > - Remove bundled minizip in prep > > Spec URL: http://kalev.fedorapeople.org/libdigidocpp.spec > SRPM URL: http://kalev.fedorapeople.org/libdigidocpp-0.3.0-2.fc14.src.rpm > Scratch build: > http://koji.fedoraproject.org/koji/taskinfo?taskID=2530364 Ok, good. This package is APPROVED.
New Package SCM Request ======================= Package Name: libdigidocpp Short Description: Library for creating and validating BDoc and DDoc containers Owners: kalev anttix tuju Branches: f12 f13 f14 InitialCC:
Git done (by process-git-requests).
Imported and built for rawhide, closing the ticket.