Red Hat Bugzilla – Bug 474992
Review Request: libirman - Library for IRMAN hardware
Last modified: 2011-10-06 08:26:15 EDT
Spec URL: http://www.salstar.sk/pub/fedora/SPECS/libirman.spec SRPM URL: http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.4-2.fc10.src.rpm Description: A library for accessing the IRMAN hardware from Linux and other Unix systems. This package contains only a static library. It's not suggested to use static libraries in Fedora, but it's not forbidden. If somebody can help me to patch this to build as shared (libirman.so), please send me or attach patch here. This library is required to update lirc to use IRMAN hardware. I can make required changes in lirc.spec after this package will be approved.
Just some quick comments on your spec file - 'Release: 3%{?dist}' should be 'Release: 1%{?dist}' and your changelog entry doesn't match the release. https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs [fab@laptop024 i386]$ rpmlint libirman* libirman.i386: W: incoherent-version-in-changelog 0.4.4-2 ['0.4.4-3.fc9', '0.4.4-3'] 3 packages and 0 specfiles checked; 0 errors, 1 warnings. - URL should point to the website of the upstream project. Why not http://www.lirc.org/ ?
(In reply to comment #1) > Just some quick comments on your spec file > > - 'Release: 3%{?dist}' should be 'Release: 1%{?dist}' and your changelog entry > doesn't match the release. > https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs ChangeLog entry fixed. This release 3 is because there is libirman of this release in my repository and I need updates from this version. > [fab@laptop024 i386]$ rpmlint libirman* > libirman.i386: W: incoherent-version-in-changelog 0.4.4-2 ['0.4.4-3.fc9', > '0.4.4-3'] > 3 packages and 0 specfiles checked; 0 errors, 1 warnings. Thanks. For info. > - URL should point to the website of the upstream project. Why not > http://www.lirc.org/ ? I think lirc is not upstream for this, it's just required by this package. But if you stand that this URL is better, I can change it.
Is there a new Source RPM somewhere?
Yes, I forgot to add them. Spec file URL is unchanged: http://www.salstar.sk/pub/fedora/SPECS/libirman.spec http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.4-3.fc10.src.rpm
Jarod, can you look if it helps to build IRMAN support in fedora's lirc package? I also can create another bug for lirc and help to add IRMAN support.
I believe lirc is now the effective upstream for this. I have just reworked the build process to make shared library builds possible (with the intention of then submitting a Fedora package) and I submitted those back to the lirc folks who committed them to their CVS tree. Getting lirc supporting libirman should be as simple as adding a libirman-devel BuildRequires to the lirc spec file - it was for me.
Apologies, meant to look at this earlier... Yeah, once libirman is being built into Fedora, it should definitely be as straight-forward as BR: libirman-devel.
Well, it's been a few weeks and nobody else has stepped in, so I'll review this even though I don't have the hardware. For a multiple license scenario like this, you need to indicate (usually by a comment in the spec) which parts of the package are under which license. I'm not sure which part of the main package might fall under the LGPL. I don't see anywhere in the code that a version of the GPL or LGPL is specified, which makes the situation complex. The LGPL parts end up as LGPLv2+ while GPL parts end up as GPL+, which when compiled together make the result GPLv2+. Ugh. I'm not sure why you call ldconfig; no dynamic libraries are installed by this package. Actually, you get a static lib even though you pass --disable-static. Any idea what's going on? * source files match upstream. sha256sum: b29d0858450c56fca97c03cb1032e3b469166d431bfa7327fa3183d31a9f64b2 libirman-0.4.4.tar.gz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * build root is OK. ? unsure whether the license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper (none). * compiler flags are appropriate. * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly. * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: libirman-0.4.4-3.fc11.x86_64.rpm config(libirman) = 0.4.4-3.fc11 libirman = 0.4.4-3.fc11 libirman(x86-64) = 0.4.4-3.fc11 = /sbin/ldconfig config(libirman) = 0.4.4-3.fc11 libirman-devel-0.4.4-3.fc11.x86_64.rpm libirman-static = 0.4.4-3.fc11 libirman-devel = 0.4.4-3.fc11 libirman-devel(x86-64) = 0.4.4-3.fc11 = libirman = 0.4.4-3.fc11 * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files X ldconfig scriptlets present, but I'm not sure why. * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * headers are in the -devel package. * no pkgconfig files. * static libraries are present: No dynamic libs, so they can be in the -devel package. -static provide is there. * no libtool .la files.
The reason it is static despite the switch is that the 0.4.4 release of libirman doesn't support shared builds at all. The current upstream CVS code does support shared builds and will respect the switch and produce shared objects.
Would it be better to either wait for a newer release or to pull a snapshot from CVS? I have no way of objectively determining whether we would benefit from having a dynamic library here.
Thank you for review. About license, there is 2 licenses mentioned in README. Added comment into spec file: #The files which make up the library are covered under the GNU Library #General Public License, which is in the file COPYING.lib. #The files which make up the test programs and the documentation are covered #under the GNU General Public License, which is in the file COPYING. License: GPLv2+ and LGPLv2+ My new rpms have included latest CVS patch. This patch removed Makefile.in and added Makefile.am, there is no configure script, so I have to include some buildrequires to create this with autogen.sh . This patch adds dynamic library build for libirman and also fixes IRMAN restart, because it's required to stop powering IRMAN before reinit. I see no other changes in this version. %changelog * Sat Mar 14 2009 Jan ONDREJ (SAL) <ondrejj(at)salstar.sk> - 0.4.4-4.20090314cvs - applied cvs patch, which fixed dynamic library build and IRMAN restart - added BuildRequires: autoconf, automake, libtool rpmlint show just this warning, I think I can do nothing with this: libirman.i386: W: shared-lib-calls-exit /usr/lib/libirman.so.0.0.0 exit@GLIBC_2.0 New packages: http://www.salstar.sk/pub/fedora/SPECS/libirman.spec http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.4-4.20090314cvs.fc10.src.rpm
Sorry for taking so long; I had too many reviews in flight and somehow this one slipped through the cracks. Please feel free to ping me if I've let something go idle for too long. You seem to have clarified the license situation well enough, and the static stuff is gone, which is good. However, it's kind of weird to patch up to a CVS snapshot by including a patch thats larger than the source tarball. It's OK to pick patches out of the upstream SCM if that's what you want, but if you want to ship a snapshot, it's better to simply do a checkout and include that as your tarball. See http://fedoraproject.org/wiki/Packaging/SourceURL for information on how we do this. Basically, you make the tarball but include instructions for generating it so that someone else who comes along will know where it came from. Currently you just have this big patch with no information on duplicating it.
(In reply to comment #12) > Sorry for taking so long; No problem. We have time. I also forgot that I have an open review. :) > However, it's kind of weird to patch up to a CVS snapshot by including a patch > thats larger than the source tarball. It's OK to pick patches out of the > upstream SCM if that's what you want, but if you want to ship a snapshot, it's > better to simply do a checkout and include that as your tarball. See > http://fedoraproject.org/wiki/Packaging/SourceURL for information on how we do > this. Basically, you make the tarball but include instructions for generating > it so that someone else who comes along will know where it came from. > Currently you just have this big patch with no information on duplicating it. OK, repackaged as source and also sent an email to upstream mailinglist, which is asking to release this as stable (or at least official snapshot). New package can be found here: http://www.salstar.sk/pub/fedora/SPECS/libirman.spec http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.4-5.20090314cvs.fc10.src.rpm It was also possible to strip patch by removing "configure" part, which is autoregenerated later from spec file.
If somebody interested, here is my email to lirc mailinglist: http://sourceforge.net/mailarchive/forum.php?thread_name=20090402060831.GO6207%40salstar.sk&forum_name=lirc-list Without any reply yet. :-(
Upstream release a new stable version. New RPMs: http://www.salstar.sk/pub/fedora/SPECS/libirman.spec http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.5-1.fc10.src.rpm
This seems to be new: libirman.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/workmanir ['/usr/lib64'] Any idea why that's just showing up now?
(In reply to comment #16) > Any idea why that's just showing up now? It has something with autoconf used to generate files in original package. In CVS there was no configure scripts and they was regenerated from spec file. Only --disable-rpath parameter does not help. There are 2 solutions: 1. use autoreconf in spec to create new scripts 2. use chrpath Solution #1 applied in my spec file: http://www.salstar.sk/pub/fedora/SPECS/libirman.spec http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.5-2.fc10.src.rpm
PING, anything else, what I can do to be this package approved? :)
(In reply to comment #12) > Please feel free to me if I've let something go idle for too long. Ping again. No reply more then 4 weeks. :(
I first reviewed this when I had time to do reviews, but because of the delay I now have very little time to do it. You are welcome to find another reviewer, or wait until I have more time. Your choice.
OK, I found some time. Unfortunately the package in comment #17 does not build for me in current F11. I get a bunch of bizarre errors with libtool such as: ./libtool: line 793: X--tag=CC: command not found ./libtool: line 826: libtool: ignoring unknown tag : command not found which cascade into: ./libtool: line 1103: X.deps/libirman_la-chunk.Tpo: No such file or directory ./libtool: line 1103: X-c: command not found ./libtool: line 1154: Xlibirman_la-chunk.lo: command not found ./libtool: line 1159: libtool: compile: cannot determine name of library object from `': command not found To rule out problems with my local mock setup, I did a koji build which failed as well: http://koji.fedoraproject.org/koji/taskinfo?taskID=1358328 It does build in F10, though. Can you work out what's broken in F11, or would you like me to review an F10 build?
%changelog * Sun May 17 2009 Ján ONDREJ (SAL) <ondrejj(at)salstar.sk> - 0.4.5-3 - added libtoolize to fix build for f11 Builds for f11 now. http://www.salstar.sk/pub/fedora/SPECS/libirman.spec http://www.salstar.sk/pub/fedora/SRPMS/10/libirman-0.4.5-3.fc10.src.rpm
Indeed, this builds fine. The complaints I had seem to be all solved now, and the package looks OK to me. APPROVED
Thank you. Looking that Kevin is on holiday, so please add CVS. :-) New Package CVS Request ======================= Package Name: libirman Short Description: Library for IRMAN hardware Owners: ondrejj Branches: F-10 F-11
CVS done.
libirman-0.4.5-3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/libirman-0.4.5-3.fc11
libirman-0.4.5-3.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/libirman-0.4.5-3.fc10
Building lirc w/irman support in rawhide now. Not sure if I need to wait a bit before I can do F10/F11 builds...
Ugh. Rawhide build failed, can't find libirman-devel yet for some reason. I'll just wait a bit to build.
For rawhide, you just have to wait for a newrepo task to complete. For the release branches, you need to wait until either the next updates push or request from releng that the package you need be tagged into the buildroot. Honestly I don't know if or how the not-yet-released status of F11 alters that.
Jarod, libirman has been just moved to F10/F11: Subject: libirman-0.4.5-3.fc11 successfully moved from dist-f11-updates-candidate into dist-f11-updates by bodhi Subject: libirman-0.4.5-3.fc10 successfully moved from dist-f10-updates-candidate into dist-f10-updates by bodhi These should be available now or in a few minutes.
Thanks to all, lirc-0.8.5-2.fc10.i386 works well with IRMAN. :-) Closing bug.
libirman-0.4.5-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
libirman-0.4.5-3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: libirman New Branches: el6 Owners: ondrejj Requested in https://bugzilla.redhat.com/show_bug.cgi?id=743566
Git done (by process-git-requests).