Bug 487665

? Rpmlint must be run on every package.
pmut.noarch: W: incoherent-version-in-changelog 0.1.2-1 ['0.1.2-2.fc10', '0.1.2-2']

? The package must be named according to the Package Naming Guidelines.
This name can collide with http://bioinformatics.oxfordjournals.org/cgi/content/abstract/21/14/3176 or libraries for genetic algorithm http://gtps.math.cmu.edu/htmldoc-etps/pmut.html

OK The spec file name must match the base package %{name}.
OK The package must meet the Packaging Guidelines.
OK The package must be licensed with a Fedora approved license.
OK The License field in the package spec file must match the actual license.
OK If (and only if) the source package includes the text of the license(s) in its own file.
OK The spec file must be written in American English ;-)
OK The spec file for the package MUST be legible.
? The sources used to build the package must match the upstream source.
The source must be located on f.e. project page.
OK The package MUST successfully compile.
OK Correct BuildRequires.
OK Proper use of %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
OK Shared library files (not just symlinks) must call ldconfig in %post and %postun.
OK Relocatable package must state this fact in the request for review.
OK A package must own all directories that it creates.
OK A package must not contain any duplicate files in the %files listing.
OK Permissions on files must be set properly.
OK Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
OK Each package must consistently use macros.
OK The package must contain code, or permissable content.
OK Large documentation files must go in a -doc subpackage.
OK If a package includes something as %doc, it must not affect the runtime of the application.
OK Header files must be in a -devel package.
OK Static libraries must be in a -static package.
OK Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
OK Library files with a suffix (e.g. libfoo.so.1.1) and files that end in .so (without suffix) must go in -devel.
OK In the vast majority of cases, devel packages must require the base package.
OK Packages must NOT contain any .la libtool archives.
OK Packages containing GUI applications must include a %{name}.desktop file.
OK At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).

The require line is too long, please create at least two instead. %doc can be on one line ;-)

project renamed:

Spec URL: http://plautrba.fedorapeople.org/soud/soud.spec
SRPM URL: http://plautrba.fedorapeople.org/soud/soud-0.1.2-3.fc10.src.rpm
Description: 

Soud is set of tools and services for automated starting or stopping services
related to hardware, using udev to detect event on hardware and DeviceKit to
examine hw devices in udev subsystems.

It's part of https://fedoraproject.org/wiki/Features/PowerManagement

Everything ok. Also the source is matching 531917bbd9c1c48e81799457f90f6c7e
The source should be changed after release into: https://fedorahosted.org/releases/something/soud-something.tgz

ACCEPTED

New Package CVS Request
=======================
Package Name: soud
Short Description: power management udev tools
Owners: foo bar
Branches: F-9 F-10
InitialCC: baz

New Package CVS Request
=======================
Package Name: soud
Short Description: Tools for hardware related services based on udev events
Owners: plautrba
Branches: F-9 F-10
InitialCC:

OK, so looking this over, this doesn't seem right.

1) It implements a critical path in the boot process... in perl.
2) It's needlessly baroque for what it does. You set up a perl daemon to listen to udev for events, look for events of a certain type, and then run actions.

Why not just write udev rules that run actions on certain events, and cut out the daemon middleman?

3) It's set to start bluetooth only if it's configured to start. Which means it's likely to have *already started*, and therefore doesn't decrease the running daemon footprint at all.

4) It drops stuff into predictable file names in /tmp, which is a security hole. (Presumably, that's leftover debugging)

CC'ing some other people who may have ideas on how to better implement this.

Spec URL: http://plautrba.fedorapeople.org/soud/soud.spec
SRPM URL: http://plautrba.fedorapeople.org/soud/soud-0.1.3-1.fc10.src.rpm


2) Daemon was part of development process and it is now optional. 

Now it is based on udev rules created by soud-save-to-udev.pl from config file.

3) bluetooth event.d script checks hw after runlevel changed. if there is no hw, calls stop on service, same situation if hw is removed.

4) yes, it was from debugging, removed now

simple faq is here http://plautrba.fedorapeople.org/soud/FAQ

(In reply to comment #6)
> OK, so looking this over, this doesn't seem right.
> 
> 1) It implements a critical path in the boot process... in perl.

As a maintainer of perl I'm really interested in this one. Could you be more precise about it?

> 2) It's needlessly baroque for what it does. You set up a perl daemon to listen
> to udev for events, look for events of a certain type, and then run actions.
> 
> Why not just write udev rules that run actions on certain events, and cut out
> the daemon middleman?
> 
> 3) It's set to start bluetooth only if it's configured to start. Which means
> it's likely to have *already started*, and therefore doesn't decrease the
> running daemon footprint at all.
> 
> 4) It drops stuff into predictable file names in /tmp, which is a security
> hole. (Presumably, that's leftover debugging)
> 
> CC'ing some other people who may have ideas on how to better implement this.

I suppose all your comments were answered. Could we push it to cvs now?

(In reply to comment #7)
> Spec URL: http://plautrba.fedorapeople.org/soud/soud.spec
> SRPM URL: http://plautrba.fedorapeople.org/soud/soud-0.1.3-1.fc10.src.rpm
> 
> 2) Daemon was part of development process and it is now optional. 
> 
> Now it is based on udev rules created by soud-save-to-udev.pl from config file.

OK, but then I really don't see what purpose this serves at all.

All it does is create an obfuscation/abstraction layer between udev rules and its own configuration. Instead of having the user write:

ACTION=="add", SUBSYSTEM=="bluetooth", RUN+="/sbin/initctl emit bluetooth-start"

(which works on any distro), you have them write, somewhere else:

[bluetooth_start]
filter=SUBSYSTEM=bluetooth ACTION=add
action=/sbin/initctl emit bluetooth-start

which will only work on Fedora for now.

There's no actual value added in the process - it's just a translation layer. And if you're packaging this config, and immediately post-processing it into rules files in %post ... just package the rules files directly and skip the processing.

I don't see how soud-watch.pl gives you anything you don't already have with 'udevadm monitor', and if we need more infrastructure there, we should get it added upstream.

> 3) bluetooth event.d script checks hw after runlevel changed. if there is no
> hw, calls stop on service, same situation if hw is removed.

However, given the normal way things work, as this is written now:

No hardware:
1) runlevel service starts bluetooth daemon
2) after runlevel, event is kicked off, checks hardware, then stops service

Has bluetooth hardware:
1) get udev event, trigger off upstart event
2) since upstart event is running before we enter a runlevel, we won't start the service
3) normal runlevel service will start bluetooth daemon
4) after runlevel, event is kicked off, checks hardware, then exits

So, for both the 'normal' boot cases, it adds code and complication to the boot process, without much benefit.

this all reminds me of the idea of having a third state for chkconfig:

on: start in runlevel
off: do not start in runlevel
demand: start in runlevel but only on demand

I once had it implemented with symlinks like chkconfig, but with a "D" prefix, like:
/etc/rc5.d/D50bluetooth -> ../init.d/bluetooth

> All it does is create an obfuscation/abstraction layer between udev rules and
> its own configuration.

It was designed as framework for multiple services, however it is configured only for one service now and sad to say I don't know about any other common service it could be used for. 

> I don't see how soud-watch.pl gives you anything you don't already have with
> 'udevadm monitor', and if we need more infrastructure there, we should get it
> added upstream.

I am missing filter functionality. e.g. if I want to see only udev events with ACTION=add or SUBSYSTEM=bluetooth ... 

> So, for both the 'normal' boot cases, it adds code and complication to the boot
> process, without much benefit.

Benefit should be for running system. Service need not run whole time but only in case when hardware is present or switched on and user need not to care about it.

(In reply to comment #11)
> > All it does is create an obfuscation/abstraction layer between udev rules and
> > its own configuration.
> 
> It was designed as framework for multiple services, however it is configured
> only for one service now and sad to say I don't know about any other common
> service it could be used for. 

Right, but it's still a one-way abstraction, even if it's for other services - you can accomplish the same just by adding an upstart event and a udev rule, without having the soud conf file in between.

> > So, for both the 'normal' boot cases, it adds code and complication to the boot
> > process, without much benefit.
> 
> Benefit should be for running system. Service need not run whole time but only
> in case when hardware is present or switched on and user need not to care about
> it.  

While it may make the service not run if you disable the hardware, it does it in a way that actually *increases* the boot time. I don't think that's how we want to go about it.

Thats why i really like the idea of harald to add a 3rd state for "ondemand" or something similar, but that needs to be discussed on fedora-devel imo.

CVS Done.  

Please have this sorted out before importing

Hm, well the issue I have is I find the package completely redundant, which can't be fixed before importing.

Hello Dennis,
so at the end we decided not to push this package and rather write patches for programmes. 

Could you remove it this branch from cvs? Then we can close this bug as wontfix.
Thank you.

cvs done.