Bug 228296 - Review Request: python-lirc - Linux Infrared Remote Control python module
Review Request: python-lirc - Linux Infrared Remote Control python module
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ville Skyttä
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-02-12 10:58 EST by Matthias Saou
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-01 14:02:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ville.skytta: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matthias Saou 2007-02-12 10:58:59 EST
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/python-lirc/
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/python-lirc/
Description:
pyLirc is a module for Python that interacts with lirc to give Python programs
the ability to receive commands from remote controls.

Notes :
- This is fairly trivial python module package. Source is one single C file!
- The original source does not contain a copy of the GPL, which is why it isn't included, but the C file does contain the correct license information.
Comment 1 Ville Skyttä 2007-02-12 11:56:05 EST
- While the license of pylirc in the sources and PKG-INFO is said to be LGPL, it
links with liblirc_client which is unconditionally GPL AFAICT -> the license of
this package when distributed should be GPL too.  LGPL allows that (see its
chapter 3 - references to LGPL should be replaced by GPL when doing that).

- Add "Provides: pylirc = %{version}-%{release}" for upstream compatibility and
because "python-lirc" is substantially different from "pylirc"?

- Include PKG-INFO in %doc?
Comment 2 Matthias Saou 2007-02-12 12:11:44 EST
Thanks for this quick and valuable feedback :-) lirc-0.0.5-2.fc6 available :

* Thu Feb  8 2007 Matthias Saou <http://freshrpms.net/> 0.0.5-2
- Change License from GPL to LGPL as our package links with lirc which is GPL.
- Add pylirc (original name) virtual provides.
- Include the API doc and both examples from the website.

Oh, I added PKG-INFO to the %doc (is it really useful?), but also took the time
to add the only doc really available : the few pages on the website.
Comment 3 Ville Skyttä 2007-02-13 16:24:22 EST
Adding the extra docs is a nice idea, however they currently suffer from
genericish SourceX file names.  Prefix them by %{name}-* (in SRPM, not when
installed)?

About licensing, LGPL section 3 says: "You may opt to apply the terms of the
ordinary GNU General Public License instead of this License to a given copy of
the Library.  To do this, you must alter all the notices that refer to this
License, so that they refer to the ordinary GNU General Public License, version
2, instead of to this License."

I'm not 100% sure what that means in practical terms, but it sounds to me as if
the license info should be changed not only in the License tag, but also the C
source file as well as PKG-INFO, and possibly the newly added changes.  Or at
least add a note somewhere prominent about the change (separate file installed
as %doc or something).  What do you think?
Comment 4 Matthias Saou 2007-02-14 05:01:28 EST
I don't see where "generic" file names would be a problem, as long as they're
managed in CVS and not uploaded as sources. These will be managed in CVS.

About the LGPL -> GPL change, I'd change it in the PKG-INFO that will go with
the binary rpms, and that's it. No change to the C file, since the source rpm
can be considered LGPL, but our binary builds against lirc will have to be GPL.
Comments in the spec file will detail this a little. Would that be okay?
Comment 5 Ville Skyttä 2007-02-14 13:20:32 EST
Generic filenames have nothing to do with the Fedora CVS nor the lookaside
cache.  Both manage them just fine (lookaside cache using md5sums in dir names
for different revisions of the same file).  But when 'rpm -i'ing a bunch of
source rpms locally it may very well become a problem.

Adding specfile comments about the licensing stuff won't be visible to users of
this package, therefore a seprately installed file included in the binary
package detailing what was done is clearly a better solution in my opinion.
Comment 6 Matthias Saou 2007-02-15 04:52:29 EST
Oh, I get it, you mean people still using a single %_sourcedir... hmmm... this
is no longer relevant with mach/mock, and it makes the build simpler if you
don't have to change the file names when installing them... do you *really* want
me to change the file names?

As for the license, we re-license the binary package under the GPL since it's
linked to a GPL library. We aren't required to add something like "used to be
LGPL", and I doubt a single end-user will actually care about the change, so as
long as we comply with the terms and make the info available somewhere (in the
source rpm), I would think it's fine.
Comment 7 Ville Skyttä 2007-02-15 13:45:01 EST
(In reply to comment #6)
> do you *really* want me to change the file names?

Yes.

> We aren't required to add something like "used to be
> LGPL", and I doubt a single end-user will actually care about the change,

Shrug, even if not required, if it's worth it to spend much more time explaining
why not add a user visible file noting why the license is changed than adding
the file would have taken, and the possible questions about the change will flow
your way as the maintainer later on too, I guess I shouldn't care.  It's not
only about end users, packagers of apps requiring this package will have to
notice the change and take it into account in their packages as well.

Conditionally approved with the assumption that the source filenames get a
%{name}-* or something like that prefix before the first build.
Comment 8 Matthias Saou 2007-03-01 09:18:55 EST
http://ftp.es6.freshrpms.net/tmp/extras/python-lirc/python-lirc.spec
http://ftp.es6.freshrpms.net/tmp/extras/python-lirc/python-lirc-0.0.5-3.src.rpm

* Thu Mar  1 2007 Matthias Saou <http://freshrpms.net/> 0.0.5-3
- Prefix all our own documentation source files.
- Include a README about our license changes and added docs.

This should basically include all of what you suggested. Please confirm before I
import the package ;-)
Comment 9 Ville Skyttä 2007-03-01 10:49:30 EST
Looks like an improvement to me, thanks.  However, there's breakage - Both
python-lirc-API and python-lirc-README are Source1, and Source4 doesn't exist. 
Approval still stands, I'm sure you'll fix those before the first build :)
Comment 10 Matthias Saou 2007-03-01 10:52:42 EST
D'oh! Typical last second changes which... break. Good, that's fixed, and I just
tested a rebuild & install. All seems fine. I'll go ahead and request CVS
branches to be created now, thanks!
Comment 11 Warren Togami 2007-03-01 11:45:14 EST
http://fedoraproject.org/wiki/CVSAdminProcedure
In the future, please follow the CVS procedure here.

New Package CVS Request
=======================
Package Name: python-lirc
Short Description: Linux Infrared Remote Control python module
Owners: matthias@rpmforge.net
Branches: FC-5, FC-6, EL-4, EL-5
InitialCC: 
Comment 12 Matthias Saou 2007-03-01 14:02:48 EST
Sorry Warren, I didn't know there was now a better documented way to make those
CVS requests. Now I do. Ville : I'm pretty sure some people will be interested
in running their HTPC on "stable" distros like RHEL... are you planning on
getting lirc into EPEL?
Comment 13 Ville Skyttä 2007-03-01 16:13:08 EST
I am one of those people, but can't tell for sure yet - one big factor for me
regarding various multimedia related packages is whether RHEL5/CentOS5 kernels
will include DVB drivers (or if not, whether I feel like packaging them myself
or can get them sanely packaged from somewhere else) and if I get past this
point, how well I can get my VDR HTPC/PVR to run on it.  If this turns out to be
too much work for my taste, it's unlikely that I would be interested in
maintaining LIRC in EPEL.

Note You need to log in before you can comment on or make changes to this bug.