Spec URL: http://mrceresa.fedorapeople.org/libgcal.spec SRPM URL: http://mrceresa.fedorapeople.org/libgcal-0.9.3-1.fc12.src.rpm Description: This is a library to access google calendar events and contacts, its purpose is - provide easy access to available events/contacts - enable common operations: add, delete, edit - have few dependencies (up until now, only requires libcurl and libxml) It implements Google Data API 2.0 and is tested on Linux and MacOSX. Hi! this is a nice package to sync with with google address and calendar A second package is required which is akonadi-googledata to be actually able to do the sync with akonadi. The couple of them worked for me flawlessy $ rpmlint ../RPMS/i686/libgcal-0.9.3-1.fc12.i686.rpm libgcal.i686: W: incoherent-version-in-changelog libgcal-0.9.3-1.fc12 ['0.9.3-1.fc12', '0.9.3-1'] 1 packages and 0 specfiles checked; 0 errors, 1 warnings. $ rpmlint ../RPMS/i686/libgcal-devel-0.9.3-1.fc12.i686.rpm libgcal-devel.i686: W: no-documentation 1 packages and 0 specfiles checked; 0 errors, 1 warnings $ rpmlint ../SRPMS/libgcal-0.9.3-1.fc12.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint libgcal.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. builds ok in koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=1894557
Changelog format is not correct: http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs This is why you get a warning in rpmlint.
> Name: libgcal > Group: Development/Libraries The base shared library still belongs into "System Environment/Libraries". > %doc COPYING > %doc INSTALL > %doc README > %doc Changelog.txt You can put multiple files behind a single %doc. File "INSTALL" is irrelevant to RPM package users. > %postun > /sbin/ldconfig > > #--------------------------------------------------------------------------------- > %package devel Caution! Avoid '#comments' after scriptlet sections. They would be included as scriptlet bodies, which breaks -p /sbin/ldconfig for example. In this particular case, simply delete that silly #---- line. It doesn't serve any purpose and only causes trouble here. Then "%postun -p /sbin/ldconfig" will work again. $ rpm -qp --scripts libgcal-0.9.3-1.fc12.i686.rpm postinstall program: /sbin/ldconfig postuninstall scriptlet (using /bin/sh): /sbin/ldconfig #--------------------------------------------------------------------------------- > %description devel > > libgcal library Header Files and Link Libraries This description would benefit from some cleanup. Please make this a complete sentence in English. * Have you looked into enabling the built-in test-suite and creating a proper %check section for it? * The -devel packages depends on libxml and libcurl headers, so adding "Requires: libxml2-devel libcurl-devel" would be necessary for all targets that don't add automatic pkgconfig dependencies (Fedora 10 and older, EPEL 5 and older). If you want to rely on the automatic pkgconfig dependencies, you can drop the "Requires: pkgconfig". * The header files in libgcal-devel bear a huge risk of causing file conflicts. This is a blocker IMO. This library ought to install them into a libgcal sub-directory: $ rpmls -p libgcal-devel-0.9.3-1.fc12.i686.rpm -rw-r--r-- /usr/include/atom_parser.h -rw-r--r-- /usr/include/gcal.h -rw-r--r-- /usr/include/gcal_parser.h -rw-r--r-- /usr/include/gcal_status.h -rw-r--r-- /usr/include/gcalendar.h -rw-r--r-- /usr/include/gcont.h -rw-r--r-- /usr/include/gcontact.h -rw-r--r-- /usr/include/internal_gcal.h -rw-r--r-- /usr/include/xml_aux.h [...]
Any progress?
Hello Scott, Andrea and Michael, I'm very sorry to reply only now, but I've been out for work and had so little time to focus on the review: @Andrea: thanks a lot for pointing me to the right source! @Michael: thanks for the comments: > The base shared library still belongs into "System Environment/Libraries". > You can put multiple files behind a single %doc. > File "INSTALL" is irrelevant to RPM package users. Fixed. Install added as a doc for development subpackage. > Caution! Avoid '#comments' after scriptlet sections. They would be included as > scriptlet bodies, which breaks -p /sbin/ldconfig for example. Thanks! This one was trubling me for a while but I'd never thought the comments were the problem :) > This description would benefit from some cleanup. Please make this a complete > sentence in English. Oops! I must have missed how ugly the previous sentence was. Hope now it's a bit better > Have you looked into enabling the built-in test-suite and creating a proper %check section for it? Yes, but I had some problems so thought to overlook. If it is really important I could give it another shot... > The -devel packages depends on libxml and libcurl headers, so adding >"Requires: libxml2-devel libcurl-devel" would be necessary for all targets that >don't add automatic pkgconfig dependencies (Fedora 10 and older, EPEL 5 and >older). > If you want to rely on the automatic pkgconfig dependencies, you can drop the > "Requires: pkgconfig". I don't understand this: when is that I have to manually specify Requires and when it's better to let them be picked up by rpm? > The header files in libgcal-devel bear a huge risk of causing file conflicts. > This is a blocker IMO. This library ought to install them into a libgcal > sub-directory: Ok I added two lines that solve the problems in the INSTALL section: mkdir -p %{buildroot}/usr/include/libgcal mv %{buildroot}/usr/include/*.h %{buildroot}/usr/include/libgcal/ but I'm not sure this is the best way to do it. New files are: http://mrceresa.fedorapeople.org/libgcal.spec http://mrceresa.fedorapeople.org/libgcal-0.9.3-2.fc12.src.rpm Mario
Mario, you can drop the %{?dist} macro from changelog entries.
> > File "INSTALL" is irrelevant to RPM package users. > > Fixed. Install added as a doc for development subpackage. It is irrelevant to RPM package users. It explains how to compile libgcal. [%check] > Yes, but I had some problems so thought to overlook. > If it is really important I could give it another shot... Well, "BuildRequires: check-devel" plus --enable-check plus "make check", and here the first test makes glibc find a realloc problem. Surely that is something to look into. Upstream should tell you whether this release is expected to pass the test-suite. > when is that I have to manually specify Requires and > when it's better to let them be picked up by rpm? In Fedora 11 and newer, rpmbuild examines .pc files for dependencies on other .pc files. Example: $ grep Req libgcal.pc Requires: libcurl libxml-2.0 $ rpm -qpR libgcal-devel-0.9.3-2.fc12.i686.rpm | grep pkg /usr/bin/pkg-config pkgconfig <-- this is your manual one pkgconfig(libcurl) pkgconfig(libxml-2.0) The dependencies on /usr/bin/pkg-config, libcurl.pc and libxml-2.0.pc are added by rpmbuild. On older platforms, rpmbuild doesn't do this. And where the pkgconfig .pc file does not specify dependencies, it may be necessary to add them to your .spec file manually (by examining C/C++ header files for needed dependencies). > mkdir -p %{buildroot}/usr/include/libgcal > > mv %{buildroot}/usr/include/*.h %{buildroot}/usr/include/libgcal/ > > but I'm not sure this is the best way to do it. This is incomplete, because libgcal.pc still thinks the files are found in /usr/include. Please talk to upstream about this. [One cannot move header file locations without either adjusting all "#include <...>" statements or header search paths.]
The review is stalled. Please respond soon.
Hello Scott, Andrea, Michael and Thomas, thanks for your messages. I'm sorry to have proceeded slowly for this package so far: I hope to collect all the data from the revisions and post a new version of the package this week. Thanks for your patience, Mario
Contacted upstream about libgcal.pc include path (Comment 6) http://code.google.com/p/libgcal/issues/detail?id=59 I'll wait for their answer before going on with the review. With best regards, Mario
Hello everybody, thanks to a patch from Peter Lemenkov: http://peter.fedorapeople.org/patches/libgcal--pc_include_fix.diff I can resubmit the package: http://mrceresa.fedorapeople.org/libgcal-0.9.3-3.fc12.src.rpm http://mrceresa.fedorapeople.org/libgcal.spec the package builds ok with koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2050755 BTW rpmlist is not silent: [makerpms@shadow rpmbuild]$ rpmlint SPECS/libgcal.spec SRPMS/libgcal-0.9.3-3.fc12.src.rpm RPMS/i686/libgcal-0.9.3-3.fc12.i686.rpm RPMS/i686/libgcal-devel-0.9.3-3.fc12.i686.rpm SPECS/libgcal.spec: W: invalid-url Source0: http://libgcal.googlecode.com/files/libgcal-0.9.3.tar.bz2 HTTP Error 404: Not Found libgcal.src: W: spelling-error Summary(en_US) google -> Google, goggle, googly libgcal.src: W: spelling-error %description -l en_US google -> Google, goggle, googly libgcal.src: W: spelling-error %description -l en_US libcurl -> lib curl, lib-curl, liberal libgcal.src: W: spelling-error %description -l en_US libxml -> Librium, libel, libido libgcal.src: W: invalid-url Source0: http://libgcal.googlecode.com/files/libgcal-0.9.3.tar.bz2 HTTP Error 404: Not Found libgcal.i686: W: spelling-error Summary(en_US) google -> Google, goggle, googly libgcal.i686: W: spelling-error %description -l en_US google -> Google, goggle, googly libgcal.i686: W: spelling-error %description -l en_US libcurl -> lib curl, lib-curl, liberal libgcal.i686: W: spelling-error %description -l en_US libxml -> Librium, libel, libido libgcal-devel.i686: W: spelling-error %description -l en_US google -> Google, goggle, googly 3 packages and 1 specfiles checked; 0 errors, 11 warnings.
Ok you will need to remove INSTALL from the devel package, all the rpmlint warnings can be safely ignored ( the invalid-url warning is invalid ). As a personal ( I'm not sure if it's a guideline ) preference could you use the standard format for the spec file i.e run this to generate a standard format spec file rpmdev-newspec libgcal
ok these are the new files without the INSTALL file and with the standard format spec file: http://mrceresa.fedorapeople.org/libgcal-0.9.3-4.fc12.src.rpm http://mrceresa.fedorapeople.org/libgcal.spec
I haven't forgotten this review request, hopefully I should have some free time in the next couple of weeks to complete it.
That's ok Scott, thanks for the message. Mario
Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on: f13/x86_64 [x] Rpmlint output: rpmlint libgcal-0.9.3-4.fc13.src.rpm libgcal.src: W: spelling-error Summary(en_US) google -> Google, goggle, googly libgcal.src: W: spelling-error %description -l en_US google -> Google, goggle, googly libgcal.src: W: spelling-error %description -l en_US libcurl -> lib curl, lib-curl, liberal libgcal.src: W: spelling-error %description -l en_US libxml -> Librium, libido, Libyan libgcal.src: W: invalid-url Source0: http://libgcal.googlecode.com/files/libgcal-0.9.3.tar.bz2 HTTP Error 404: Not Found 1 packages and 0 specfiles checked; 0 errors, 5 warnings. spelling errors can be ignored (you should capitalize google in summary and description) invalid-url warning is invalid rpmlint libgcal-0.9.3-4.fc13.x86_64.rpm libgcal.x86_64: W: spelling-error Summary(en_US) google -> Google, goggle, googly libgcal.x86_64: W: spelling-error %description -l en_US google -> Google, goggle, googly libgcal.x86_64: W: spelling-error %description -l en_US libcurl -> lib curl, lib-curl, liberal libgcal.x86_64: W: spelling-error %description -l en_US libxml -> Librium, libido, Libyan 1 packages and 0 specfiles checked; 0 errors, 4 warnings. rpmlint libgcal-devel-0.9.3-4.fc13.x86_64.rpm libgcal-devel.x86_64: W: spelling-error %description -l en_US google -> Google, goggle, googly libgcal-devel.x86_64: W: no-documentation 1 packages and 0 specfiles checked; 0 errors, 2 warnings. rpmlint libgcal-debuginfo-0.9.3-4.fc13.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type per spec: BSD [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package match the upstream source, as provided in the spec URL. SHA1SUM of package: da2368f6ccd9b4a77fa435b3181d298f814ef4ff libgcal-0.9.3.tar.bz2 [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [x] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT. [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [-] Package uses nothing in %doc for runtime. [x] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [x] Package requires pkgconfig, if .pc files are present. [x] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. =>see preamble [x] Package does not own files or directories owned by other packages. [x] Final provides and requires are sane. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on: http://koji.fedoraproject.org/koji/taskinfo?taskID=2115844 [x] Package should compile and build into binary rpms on all supported architectures. Tested on: F13/x86_64 [x] Package functions as described. [-] Scriptlets must be sane, if used. [x] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane. [-] %check is present and the test passes. 1) You Must drop the %{?dist} tag from the changelog 2) I Would like the spec file in standard format, but this isn't covered by review guidelines Package approved
https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs Your changelog should look like this (note the email address as well) %changelog * Sun Mar 21 2010 Mario Ceresa <mrceresa> 0.9.3-4 - Removed INSTALL from the devel package * Thu Mar 13 2010 Mario Ceresa <mrceresa> 0.9.3-3 - Patched pc_include (Thanks to Peter Lemenkov for the patch) * Thu Jan 21 2010 Mario Ceresa <mrceresa> 0.9.3-2 - Fixed the comments from the reviewers * Tue Dec 29 2009 Mario Ceresa <mrceresa> 0.9.3-1 - Initial RPM Release
Created attachment 406653 [details] spec file I would like to see your spec file use this format.
Hello Scott, thanks for the approval. I'll be out of town all the week and unable to apply your modifications on the package for now: I'll do that as soon as I'm back.
Hello Scott, here there are the new spec and source rpm: http://mrceresa.fedorapeople.org/libgcal.spec http://mrceresa.fedorapeople.org/libgcal-0.9.3-4.fc12.src.rpm and this is the CVS request: New Package CVS Request ======================= Package Name: libgcal Short Description: A library to access google calendar events and contacts Owners: mrceresa Branches: F-12 F-13 EL-5 InitialCC: leigh123linux
CVS done (by process-cvs-requests.py).
Looks like this got imported and build for dist-f14 at least (closing). I'd recommend getting at least f12, f13 builds done soonish too.
Hello everybody! Adenilson Cavalcanti from the libgcal's team, very kindly changed the new version of libgcal cmake's script to install headers in include/libgcal, as previously requested in comment 9. The patch is visible at: http://gitorious.org/libgcal/libgcal/commit/c06ef5e2269cea1d7533b999db19845b9a4a7a3f When a new version of libgcal is released, we should remove from the specfile the part that copies the header in the new location. I could make the change if you wish. Thanks and regards, Mario