Bug 642773

Updated to iguanaIR 1.0.1 release:

http://jwilson.fedorapeople.org/packaging/iguanaIR/iguanaIR-1.0.1-1.el6.src.rpm

Builds fine; rpmlint has a few complaints:
  iguanaIR.x86_64: W: non-standard-uid /var/run/iguanaIR iguanair
  iguanaIR.x86_64: W: non-standard-gid /var/run/iguanaIR iguanair
  iguanaIR.x86_64: W: non-standard-uid /lib/udev/devices/iguanaIR iguanair
  iguanaIR.x86_64: W: non-standard-gid /lib/udev/devices/iguanaIR iguanair
Obviously these are OK because the package adds a user, though there's also a group which isn't added by this package.

  iguanaIR.x86_64: W: no-manual-page-for-binary igdaemon
  iguanaIR.x86_64: W: no-manual-page-for-binary igclient
  iguanaIR-devel.x86_64: W: no-documentation
  iguanaIR-python.x86_64: W: no-documentation
  iguanaIR-reflasher.noarch: W: no-documentation
  iguanaIR-reflasher.noarch: W: no-manual-page-for-binary iguanaIR-reflasher
Manpages would be nice, but if upstream doesn't provide documentation then it's not as if you should write it.

  iguanaIR.x86_64: W: incoherent-init-script-name iguanaIR
   ('iguanair', 'iguanaird')
Just rpmlint failing to understand the name of the initscript.

  iguanaIR-debuginfo.x86_64: W: spurious-executable-perm
   /usr/src/debug/iguanaIR-1.0.1/iguanaIR_wrap.c
Kind of weird for a .c file to be executable, but it just gets copied from the builddir by rpm magic.  If you like you can fix it with a quick chmod in %prep.

  iguanaIR-python.x86_64: W: private-shared-object-provides
   /usr/lib64/python2.7/site-packages/_iguanaIR.so _iguanaIR.so()(64bit)
I'd suggest filtering this but I don't think you can use the dependency filtering stuff because this package has other compiled libraries and executables.

Unfortunately I can't seem to fetch the upstream source.  Following the Source0: URL, I get redirected to http://iguanaworks.net/products.psp/releases/iguanaIR-1.0.1.tar.bz2 which redirects me again to /products.psp.  I think the proper thing is:

Source0: http://iguanaworks.net/downloads/%{name}-%{version}%{?snap}.tar.bz2

You can probably get rid of the snap stuff now that there's a release, unless you intend to pull further post-release snapshots.

I note that /etc/default is an odd place for settings used by an initscript.  Generally we'd put them in /etc/sysconfig.  

I'm somewhat confused about the license.  docs/license says everything is either LGPLv2 or GPLv2, though it also has a really troubling bit about them automatically claiming copyright over anything you "submit" to them.

However, you'd have to go through the files in the binary packages and figure out what's GPL and what's LGPL.  I'm not sure it all collapses down to GPL.

Shouldn't you be creating the iguanair group as well?  And is there any specific reason not to use the recommended scriptlets for setting that up?

There is no reason to call ldconfig for the -devel package; it only installs a symlink.

I note that udev is only pulled in very incidentally, I think because the scriptlets depend on initscripts.  It might be useful to make that dependency more explicit.

* source files match upstream.  sha256sum:
  1ba324667671892b2a36aee0089d0afafd266f8b255b073b113eb5ab71f38fcc
   iguanaIR-1.0.1.tar.bz2
  (using updated Source0: URL)
* 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.
? not sure about the license field 
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
X final provides and requires:
  iguanaIR-1.0.1-1.fc15.x86_64.rpm
   config(iguanaIR) = 1.0.1-1.fc15
   libiguanaIR.so.0()(64bit)  
   libiguanaIR.so.0(IGUANAIR_0)(64bit)  
   libusb.so()(64bit)  
   libusbpre1.so()(64bit)  
   iguanaIR = 1.0.1-1.fc15
   iguanaIR(x86-64) = 1.0.1-1.fc15
  =
   /bin/sh  
   /sbin/chkconfig  
   /sbin/ldconfig  
   /sbin/service  
   config(iguanaIR) = 1.0.1-1.fc15
   libiguanaIR.so.0()(64bit)  
   libiguanaIR.so.0(IGUANAIR_0)(64bit)  
   libpopt.so.0()(64bit)  
   libpopt.so.0(LIBPOPT_0)(64bit)  
   libusb-0.1.so.4()(64bit)  
   libusb-1.0.so.0()(64bit)  
   lirc  

  iguanaIR-devel-1.0.1-1.fc15.x86_64.rpm
   iguanaIR-devel = 1.0.1-1.fc15
   iguanaIR-devel(x86-64) = 1.0.1-1.fc15
  =
X  /sbin/ldconfig  
   iguanaIR = 1.0.1-1.fc15
   libiguanaIR.so.0()(64bit)  

  iguanaIR-python-1.0.1-1.fc15.x86_64.rpm
?  _iguanaIR.so()(64bit)  
   iguanaIR-python = 1.0.1-1.fc15
   iguanaIR-python(x86-64) = 1.0.1-1.fc15
  =
   iguanaIR = 1.0.1-1.fc15
   libiguanaIR.so.0()(64bit)  
   libiguanaIR.so.0(IGUANAIR_0)(64bit)  
   libpython2.7.so.1.0()(64bit)  
   python >= 2.4
   python(abi) = 2.7

  iguanaIR-reflasher-1.0.1-1.fc15.noarch.rpm
   iguanaIR-reflasher = 1.0.1-1.fc15
  =
   /usr/bin/python  
   iguanaIR-python = 1.0.1

* no bundled libraries (that I can see).
* shared libraries are installed:
   ldconfig is called properly (on the main package, at least).
   unversioned .so link is in the -devel package.
* 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.
* scriptlets on main package are OK (ldconfig).
X scriptlets on devel package are unnecessary.
* 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 static libraries.
* no libtool .la files.

Updated package w/most of the issues addressed, hopefully.

http://jwilson.fedorapeople.org/packaging/iguanaIR/iguanaIR-1.0.1-2.el6.src.rpm

Remaining issues:
- license says GPLv2 and LPGLv2, but doesn't break down which code is under which license (though headers in many of the source files do indicate their applicable license).
- private shared object Provides: still present, but yeah, can't use the filter macros here

I haven't heard anything more on the legal front, but I figure I can look things over anyway.

The only issue I see besides the first thing you point out above is that Requires(post): /sbin/ldconfig is missing.  Sometimes I think it's funny that we add that since you can't do much without glibc installed, but them's the rules.

Regarding the filter stuff, I know that the rpm in rawhide lets us do fancier stuff.  It might be worth it to investigate that, but then again it's only useful for rawhide right now.

I've got an updated spec file with the ldconfig fixup, and coincidentally, was just contacted by someone at iguanaworks, from whom I'm seeking clarification (and likely source code fixups) on the license terms. Will hold off on pushing for any further review until the license issues have been discussed with him.

I saw something from spot about progress with the legal issues; I thought that went to this ticket but I guess not.  Let me see if I can dig that up.

And it's here:

http://lists.fedoraproject.org/pipermail/legal/2011-February/001554.html

Tibbs, if I just update the package with the suggested license changes (pulled from the iguanaIR svn repo), would we be good to go?

Somewhat unhappy about rebuilding this package for each release, but otherwise very happy with it... is there any help I can provide to push things forward?

I'm so far behind on this.

I'm kind of surprised there's no newer upstream version.  Given that fact, yes, if you can pull a patch from their SVN or even just pull a whole new snapshot, that should be fine.

Also, we should actually have a proper dependency mechanism now that works on arch-specific packages.  At the moment I can't remember how to use it but I'll try to figure it out.

Finally, it's a systemd world now and initscripts are, like, so 80's or something. Maybe this will work, but I didn't test.  It doesn't support configuration via the /etc/sysconfig file but that's really not the best way to handle that anyway.  The spec will of course need adjustment for all of this.

[Unit]
Description=Iguanaworks USB IR transceiver

[Service]
Type=forking
ExecStart=/usr/bin/igdaemon --log-level=0 --send-timeout=1000 --receive-timeout=1000 --driver=libusb -l /var/log/iguanaIR.log
PIDFile=/run/igdaemon.pid

[Install]
WantedBy=multi-user.target

Went ahead and did a couple of other things since I'm trying to get better at the whole systemd thing.  Generally the reviewer wouldn't do this, but whatever.

Full systemd conversion.
tmpfiles.d conversion (since /run is tmpfs now).
New-style dependency filtering that works on arch-specific packages.

I tried to keep your packaging conventions in place but I probably made a mess of it or changed something unrelated.  The above unit file wasn't actually correct because the PID file was created by the initscript, not the daemon, so systemd timed out waiting for it to be created.  I just removed mention of it from the service.

Anyway, it appears to work.  I'm not sure if /run/iguanaIR is actually used by anything; it seems to be empty for me.  If it's not you can dump the whole tmpfiles.d thing.

Spec: http://www.math.uh.edu/~tibbs/rpms/iguanaIR/iguanaIR.spec
SRPM: http://www.math.uh.edu/~tibbs/rpms/iguanaIR/iguanaIR-1.0.1-3.fc16.src.rpm

Oh, the result is unabashedly a Fedora package.  I know it won't work at all on RHEL5, and obviously the systemd stuff precludes RHEL6 too.  The filtering stuff won't work there but since it's just a %define it doesn't hurt anything.

I have made a preliminary test on F16/i686 which have been running the previous RPM successfully. However, the server dies immediately at start. I will look more into this, so I don't care to attach logs. Running out of time now, though.

Created attachment 580203 [details]
Revised service file

The service file seems to have at least three possible problems (?). One is that it's running as root, which is bad in many ways. Also, the system-V code to create the logfile is missing - but this only matters when running as iguanair.

Also, the recommended way to handle the pid is to use the pid file (PIDFile=) if a forking server supports it as igdaemon does.

Enclosed service file:
 - uses su iguanair to run as non-root.
 - creates logfile before starting server
 - runs a non-forking server (--no-daemon) to simplify systemd's pid tracking.

This file works for me (tm).

Some other remarks after using the thing and skimming the spec:
- I had to enable the iguanair user: usermod -s /bin/bash iguanair
- Would it be an advantage to use the filter mechanisms from http://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering instead?
- Nit-picking: since python package is python2, shouldn't it BR:
  python2-devel? (http://fedoraproject.org/wiki/Packaging:Python#BuildRequires)

I didn't see the original initscript changing the user, but maybe I just missed it.  And in any case:

Why have ExecStartPre when the log file could just be in the package?

Why do su when User= (and maybe Group=) would work much better?

(In reply to comment #15)
> I didn't see the original initscript changing the user, but maybe I just missed
> it.
START="start-stop-daemon --start --chuid iguanair --group iguanair --exec...

> And in any case:
> Why have ExecStartPre when the log file could just be in the package?
hm... sooner or later a logfile need to be removed, and then you're stuck. It's easy to create a logfile owned by root, which iguanair can't write into. In my experience, you either need this kind of code or a directory owned by the package which doesn't go away. In this case, I think creating the file is the best (i. e., simplest) solution 


> Why do su when User= (and maybe Group=) would work much better?
Because I didn't find them in the manpages...  (I actually looked for them). I agree, they seem much better :)

Found User=. Seems that it will affect all processes started from the service file. For me, this seems to boil down to creating a directory like /var/log/igaunaIR, which iguanair has write permissions into. If the log file is placed there, the PreExec stuff can be removed and User=iguanair should work fine.

Created attachment 580353 [details]
Fix for logging, revised service file version2

See you are quite busy these days ;) Updated patch according to discussion. This one also works for me. 

I can confirm that /run/iguanaIR is actually used when daemon is running.

Created attachment 580355 [details]
Proposed fix for python BR

Created attachment 580356 [details]
Proposed filter update

Ping?

Who are you pinging?

Well, my interest in this is to get this package into Fedora, since I need it. OTOH, this bug is in the hand of you (Jason)  being the reviewer and Jarod W, who submitted the bug. 

After reporting some testing, including patches, I'm looking forward to some kind of feedback. Or perhaps some other activity regarding this package. Basically, I'm just wondering if this is moving forward. 

Is it?

--a

There is an in-kernel driver in 3.6.0-rc1. linuxtv has some patches for it queued for 3.7.

The user-space driver causes hunderds of wake-ups per second (even there is no IR activity) and it's not very readable.

@Sean: I assume this is https://patchwork.kernel.org/patch/1306511/ ?!

As I see this, it's certainly room for improvments. But the 25 ms saved by the patch is not a blocker for the typical HTPC usage.

Another question is the relations between this package and the in-kernel driver:
http://iguanaworks.net/projects/IguanaIR/ticket/285

I have updated the package to upstream 1.0.3 (using svn trunk is probably a better idea, but...). I have also revised it so much that maintaining diffs to current spec is a bit awkward, so I just send the links. I'm using this successfully myself, but it's  *not* tested on 64-bit systems.

spec: http://leamas.fedorapeople.org/iguanaIR/iguanaIR.spec
srpm: http://leamas.fedorapeople.org/iguanaIR/iguanaIR-1.0.3-1.fc17.src.rpm

Changelog:
* Thu Dec 27 2012 Alec Leamas <leamas.alec> - 1.0.3-1
- Updated to 1.0.3
- Set license to GPLv2, can't find any LGPL files.
- Stay closer to upstream: keep %%libdir/iguanaIR ínternal libs.
- Stay closer to upstream: keep driver.c:findDriverDir as-is.
- Moved most of fixes.patch to spec file, split rest to smaller ones.
- Use %%datadir instead of libexec for reflasher (acceptable upstream).
- Support sysconfig configuration file.
- Change python module name to python-iguanaIR.
- Include documentation files.
- Install udev rule in /lib/udev, not /etc/udev.
- Fixed udev issue invoking '/etc/init.d/iguanaIR rescan'.
- Added missing (pre) dep.
- Added logrotate file.

Since there is no response from the reviewer since June I'm applying https://fedoraproject.org/wiki/Package_maintainer_policy#Reviewer_not_responding: this review is stalled until we get some input from Jason. Jason: please respond ASAP.

I'm open to any alternative to get this in place including testing, co-maintaining or just keeping my mouth shut to let the original submitter and reviewer sort this out. But after more than two years and working packages in both Mandriva and el6 we definitely need to close this ticket.

If we all agree on that I'm prepared to submit an own request, should that solve anything. But only if we all agree.

Note that the in-kernel driver is unrelated to Iguanaworks, who actually makes the hardware and this driver. So this package is still needed.

Last I checked this wasn't waiting on me, and it still seems to be that way.  You stepped in and threw up your own package, even though you're not the submitter of this ticket, and then expect me to pay attention to it.  You're welcome to work with Jarod, that's your business.  Trying to invoke policy just to kick me off of this ticket isn't going to have the effect you desire.

Anyway, this has consumed enough of my time.  My apologies to Jarod, but I'm through here.  I would have waited patiently for Jarod to do his work on the ticket, but given Alec's interference I think I'll go and do something else.

@jason: Sorry you take this personal. Reiterating: my only intent is to get this fixed and approved, not to "kick you off" or anything like that. 

@jarod: Since you havn't been heard of since May here, could you please respond ASAP and tell what you think about the situation? Yes, this means that I invoke the policy in comment #26 also on you, we need to communicate to resolve this. Until then this review is stalled.

@Alex
This is the new in-kernel driver:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/media/rc/iguanair.c

This driver uses the new rc core interface. All IR drivers are slowly being ported from lirc to the new rc core interface; lirc is deprecated.

Using the new driver the iguanaworks ir transceiver becomes a proper input device. You can select the remote using ir-keytable(1) and/or /etc/rc_maps.cfg

The package you are proposing for inclusion uses the deprecated lirc daemon.

Hm... Doesn't using the current driver still makes some sense:
  - It has other functionality: reflashing, python bindings...
  - A lot of HTPC users out there are still using lirc, and so will for some time, having invested in configurations.
  - Coming from the hardware vendor, it's kind of a reference should there be hw problems or updates.

BTW, is lirc really deprecated? As I understand it, recent version uses the kernel drivers... and doesn't it provide a lot of glue between applications such as mythtv and vlc and the kernel drivers? That said, the lirc keymapping seems obsolete in this scenario.

Using the kernel rc core any IR device becomes a regular input device so no daemon or application specific support is needed any more. lirc daemon is no longer needed. The key mapping is done differently, so the lirc keymapping has been replaced with the keymaps in /etc/rc_keymaps/*

Admittedly the new kernel driver cannot do reflashing yet, however I think that this is a bug against the kernel driver. 

Just because a hardware vendor provides a linux driver does not mean fedora should use their code.

All of this is ignoring the fact that the iguanaworks driver is a complete abomination. Everything is wrong about it. Having a separate process is for such a simple device is wrong. The source code is makes you want to gouge your eyes out. Having a device specific python bindings is completely wrong.

Let's leave lirc out of this, it's a little off-topic.

The driver source could be disputed, agreed. But it's well tested and maintained against this hw. That's not bad. Also, the reflashing is an issue, it's kind of nice to have the guys delivering the firmware also testing that part. Note that the package also contains the firmware.

To allow low-level access through some python bindings is, well, different. But to say it's plain wrong really depends. I actually don't know the device well enough to say whether there are things which can't be reasonably done through the driver interface. But it's certainly possible.

Also, many eyes are reading this. Please refrain from statements like "abomination" and "gouge your eyes out". It just doesn't help.

I don't really understand why you are so strongly against this. This is not supposed to be the default long-term alternative, just an add-on for now when the kernel driver doesn't make it. It's likely to be obsolete at some point in the future, but that point is at earliest F19 IMHO. Until then, I think it would actually fill a need.

To be perfectly honest, I gave up on caring about this. I don't have the time or energy to work on IR stuff these days. I'm fine with closing this and letting someone else who is still interested work on it via a new bug.

Thanks for swift response and a really nice work! I'll do my best to finish this (will update with new bug as soon it's in place).

Changing resolution to DUPLICATE of 891339, where work will continue.

*** This bug has been marked as a duplicate of bug 891339 ***