Merge lp://staging/~beehock/ubuntu/natty/lirc/lirc-fix-695767 into lp://staging/ubuntu/natty/lirc

ls, grep x 2, sed and tr is a little clunky..

Does something like:

find /dev/input/* -type c -prune -printf ", %p"

Make more sense?

Dave,

Thanks for the advice. It is indeed much better than what I did.

Using your proposal, I managed to merge into a single command which can also support future new directory branch in /dev/input

Thanks!

Can you please forward this to Debian? It doesn't need to go to "real" upstream, as this is only a packaging change. But it should be reported as a Debian bug with the patch, see http://www.debian.org/Bugs/Reporting

Please note that your branch changes #DEBHELPER# into the dh_installinit generated code, which should not be done. Please exclude that from forwarding (I'll do that for sponsoring).

Hang on, don't you mean to search devices in /dev/input/by-id/ only? With the current patch you'd filter those _out_, but I think that's wrong. /dev/input/eventN aren't stable, and the user doesn't know which is the right one anyway. by-id symlinks are the ones which should be used IMHO.

On Wed, Mar 2, 2011 at 8:39 PM, Martin Pitt <email address hidden> wrote:
> Review: Needs Fixing
> Hang on, don't you mean to search devices in /dev/input/by-id/ only? With the current patch you'd filter those _out_, but I think that's wrong. /dev/input/eventN aren't stable, and the user doesn't know which is the right one anyway. by-id symlinks are the ones which should be used IMHO.
> --
> https://code.launchpad.net/~beehock/ubuntu/natty/lirc/lirc-fix-695767/+merge/51706
> You are the owner of lp:~beehock/ubuntu/natty/lirc/lirc-fix-695767.
>

No, I meant to fix the issue where devices in the /dev/input/by-id are
not shown. The current code included the /dev/input/eventN. Would
there be cases where devices does not appear in in by-id and by-path?

> Please note that your branch changes #DEBHELPER# into the dh_installinit generated code, which should not be done. Please exclude that from forwarding (I'll do that for sponsoring).
> --

Can you let me know what I did to change that? I only modified the
debian/changelog and lirc.postinst from my branch and follow the steps
from the sponsorship. I have no idea on it what cause the change to
#DBHLPER#

> No, I meant to fix the issue where devices in the /dev/input/by-id are
> not shown. The current code included the /dev/input/eventN.

Hm, when I install current lirc, I get a full tree of /dev/input/, including by-id, by-path, etc.

> Would there be cases where devices does not appear in in by-id and by-path?

In theory yes, if a device isn't on USB, or doesn't have a vendor/product name. This shouldn't happen with USB devices, only perhaps with some builtin stuff like lid switches, etc.

But in general I'd really avoid hardcoding /dev/input/eventX in a configuration file. It's inevitably going to break, as the enumeration is completely dynamic, and random up to some degree.

> > Please note that your branch changes #DEBHELPER# into the dh_installinit
> generated code, which should not be done. Please exclude that from forwarding
> (I'll do that for sponsoring).
> > --
>
> Can you let me know what I did to change that?

I don't know either -- building a package does not change debian/lirc.postinst. The only plausible way I can imagine is that you first modified it in /var/lib/dpkg/info, and ran dpkg-reconfigure until it worked, and then just copied that into the package.

Isnt this the current code?

EVENTS_BY_PATH=`ls /dev/input/by-path | sed 's/^/,\
\/dev\/input\/by-path\//' | tr '\n' '\0'`
23 - EVENTS=`ls /dev/input | grep -v "by-path" | sed 's/^/,\
\/dev\/input\//' | tr '\n' '\0'`
24 - db_subst lirc/dev_input_device EVENTS $EVENTS_BY_PATH $EVENTS

Only /dev/input/by-id is down and not the actual device.

Didnt `find /dev/input/* -not -type d -printf ", %p"` cover all the
devices in /dev/input excluding the directory?

I still dont understand what I need to fix.

On Wed, Mar 2, 2011 at 10:11 PM, Martin Pitt <email address hidden> wrote:
>> No, I meant to fix the issue where devices in the /dev/input/by-id are
>> not shown. The current code included the /dev/input/eventN.
>
> Hm, when I install current lirc, I get a full tree of /dev/input/, including by-id, by-path, etc.
>
>> Would there be cases where devices does not appear in in by-id and by-path?
>
> In theory yes, if a device isn't on USB, or doesn't have a vendor/product name. This shouldn't happen with USB devices, only perhaps with some builtin stuff like lid switches, etc.
>
> But in general I'd really avoid hardcoding /dev/input/eventX in a configuration file. It's inevitably going to break, as the enumeration is completely dynamic, and random up to some degree.
>
>> > Please note that your branch changes #DEBHELPER# into the dh_installinit
>> generated code, which should not be done. Please exclude that from forwarding
>> (I'll do that for sponsoring).
>> > --
>>
>> Can you let me know what I did to change that?
>
> I don't know either -- building a package does not change debian/lirc.postinst. The only plausible way I can imagine is that you first modified it in /var/lib/dpkg/info, and ran dpkg-reconfigure until it worked, and then just copied that into the package.
> --
> https://code.launchpad.net/~beehock/ubuntu/natty/lirc/lirc-fix-695767/+merge/51706
> You are the owner of lp:~beehock/ubuntu/natty/lirc/lirc-fix-695767.
>

> Isnt this the current code?
>
> EVENTS_BY_PATH=`ls /dev/input/by-path | sed 's/^/,\
> \/dev\/input\/by-path\//' | tr '\n' '\0'`
> 23 - EVENTS=`ls /dev/input | grep -v "by-path" | sed 's/^/,\
> \/dev\/input\//' | tr '\n' '\0'`
> 24 - db_subst lirc/dev_input_device EVENTS $EVENTS_BY_PATH $EVENTS
>
> Only /dev/input/by-id is down and not the actual device.

Correct. Above code filters out the by-path/ devices.

> Didnt `find /dev/input/* -not -type d -printf ", %p"` cover all the
> devices in /dev/input excluding the directory?

It does. My point is that it should _only_ show by-id/ (and perhaps per-path/ too), since the user should never select something like /dev/input/event7.

I thought that was the actual point of this bug report/fix?

In other words, I think that

  EVENTS=`ls /dev/input/by-id/* /dev/input/by-path/*`

would be quite appropriate here?

And finally you need to revert the #DEBHELPER# change in the postinst, as pointed out above.

Thanks!

EVENTS=`find /dev/input/* -type d -exec find {} -not -type d -printf ", %p" \;`

I used this to avoid the issue of similar bug from arising. it cover
all sub-directory devices with /dev/input.

On Thu, Mar 3, 2011 at 1:04 AM, Martin Pitt <email address hidden> wrote:
> In other words, I think that
>
>  EVENTS=`ls /dev/input/by-id/* /dev/input/by-path/*`
>
> would be quite appropriate here?
> --
> https://code.launchpad.net/~beehock/ubuntu/natty/lirc/lirc-fix-695767/+merge/51706
> You are the owner of lp:~beehock/ubuntu/natty/lirc/lirc-fix-695767.
>

Well, *shrug*, I still think it's a lot better to only show by-path/ and by-id/, but if you want to show all devices, so be it.

Can you then please change it to not use a double-find, but the much simpler

  find /dev/input ! -type d -printf ", %p"

?

Thanks!

Actually, the double find was meant to do what you ask of(ignore the
eventN). Another reason I use the double find was to factor in future
additional /dev/input/by-whatever.

find /dev/input/* -type d -exec find {} -not -type d \;

/dev/input/by-id/usb-VirtualBox_USB_Tablet-mouse
/dev/input/by-id/usb-VirtualBox_USB_Tablet-event-mouse
/dev/input/by-path/pci-0000:00:06.0-usb-0:1:1.0-mouse
/dev/input/by-path/pci-0000:00:06.0-usb-0:1:1.0-event-mouse
/dev/input/by-path/platform-i8042-serio-1-event-mouse
/dev/input/by-path/platform-i8042-serio-1-mouse
/dev/input/by-path/platform-i8042-serio-0-event-kbd

while your latest recommendation list everything,

 find /dev/input ! -type d -printf ", %p"

/dev/input/by-id/usb-VirtualBox_USB_Tablet-mouse
/dev/input/by-id/usb-VirtualBox_USB_Tablet-event-mouse
/dev/input/js0
/dev/input/event3
/dev/input/mouse0
/dev/input/event4
/dev/input/mouse1
/dev/input/by-path/pci-0000:00:06.0-usb-0:1:1.0-mouse
/dev/input/by-path/pci-0000:00:06.0-usb-0:1:1.0-event-mouse
/dev/input/by-path/platform-i8042-serio-1-event-mouse
/dev/input/by-path/platform-i8042-serio-1-mouse
/dev/input/by-path/platform-i8042-serio-0-event-kbd
/dev/input/event2
/dev/input/event1
/dev/input/event0
/dev/input/mice

On Thu, Mar 3, 2011 at 4:40 PM, Martin Pitt <email address hidden> wrote:
> Well, *shrug*, I still think it's a lot better to only show by-path/ and by-id/, but if you want to show all devices, so be it.
>
> Can you then please change it to not use a double-find, but the much simpler
>
>  find /dev/input ! -type d -printf ", %p"
>
> ?
>
> Thanks!
> --
> https://code.launchpad.net/~beehock/ubuntu/natty/lirc/lirc-fix-695767/+merge/51706
> You are the owner of lp:~beehock/ubuntu/natty/lirc/lirc-fix-695767.
>

Ah, of course. Sorry, I missed that.

Thanks, this looks good now! I'll merge this after the alpha-3 freeze.