Description of problem: NetworkManager scriplets need to be cleaned up, they contain unnecessary comments and little mistakes. Version-Release number of selected component (if applicable): NetworkManager-0.3.2-4.3.cvs20041208 Additional info: 1) There are comments in the spec file around the scriplets, which are obviously included in the real scriplets... which was most certainly not intended by the person who included them. For instance, the %preun scriplet is as follows : -- snip -- if [ "$1" -ge "1" ]; then service NetworkManager condrestart > /dev/null 2>&1 fi ################################## # Main Package Files ################################## -- snip -- Note that this block of comment is in the spec file, to introduce the %files section below... 2) There are small mistakes in the scriplets : - This should not be : if ! chkconfig --list | grep NetworkManager | grep on; then /sbin/chkconfig --add NetworkManager /sbin/chkconfig --level 123456 NetworkManager off fi Instead, one wants the "chkconfig" line of the init script to default all levels to off, and unconditionnaly run that --add line, which will not change the local changes to the on/offs in various runlevels, if any. Furthermore, that test with the "chkconfig --list" isn't redirected, so the line is actually outputted when the package is installed or upgraded, messy. - One should make up his mind about integers vs. characeters. Normally, $1 is an integer passed to the scriplets, so the proper "test" operands to use are -lt, -le, -eq, -ge or -gt unquoted. If one decides to quote everything, why not, then only = should be used. But here we have both wrong : preuninstall scriptlet (using /bin/sh): if [ $1 = 0 ]; then service NetworkManager stop > /dev/null 2>&1 /sbin/chkconfig --del NetworkManager fi ################################## # Post-uninstall ################################## postuninstall scriptlet (using /bin/sh): if [ "$1" -ge "1" ]; then service NetworkManager condrestart > /dev/null 2>&1 fi ################################## # Main Package Files ################################## I'll try to produce a quick patch to the spec file to correct all of these.
I'd be happy to apply the patch. The thing to keep in mind is that: 1) If the user _hasn't_ ever used NetworkManager, we don't want it to be started by default or added with chkconfig. At this time, users must explicitly enable NM, so just installing the RPM should _not_ run NM on startup or add it with chkconfig unless its already there 2) We should not restart NM if its not already running (condrestart) 3) If the user has NM set to run on startup and its currently running, we should restart NM The current specfile doesn't do these very well, I'll admit :)
I get those points, but for 1), the proper way of making sure NetworkManager doesn't get added to run on startup for people who didn't enable it explicitely is simply by changing this line : # chkconfig: 345 98 02 to : # chkconfig: - 98 02 in "initscript/RedHat/NetworkManager" of the sources, as this represents the runlevels in which to run the service by default. Making the change upstream, adding a patch to the srpm or adding a one-liner regexp substitution in %prep or %build are all solutions.
Aha, the "-" I was not aware of. Thanks.
Created attachment 109633 [details] Spec file patch Changes : - Keep consistent between spaces and tabs - Added URL so that people know where to look for more info - Remove big blocks of comments that caused trouble in the scriplets (sorry! a good syntax highlighting will do as well...) - Added missing BuildRequirements - Updated %description based on current README - Remove unneeded explicit dependencies - Use %find_lang macro for translations and file colouring - Clean up scriplets - Add directory default attributes - Re-organize %files order to increase readability - Add slashes to the %files lines for directories - Set the gnome sub-package dbus file as %config like the main one Feel free to take or throw away any parts separately.
One quick comment... The NetworkManager-gnome package does indeed require GConf2, since NetworkManagerInfo uses it and links to gconf libraries.
Absolutely. But this is a typical case of redundant dependency information, as the library requires/provides that rpm computes will already carry that information : $ rpm -q --requires NetworkManager-gnome | grep gconf libgconf-2.so.4 $ rpm -q --whatprovides libgconf-2.so.4 GConf2-2.8.1-1 Having "GConf2" as an explicit dependency can seem harmless at first (and mostly is, actually ;-)), but say that the GConf2 package dissapears because it's been swallowed by a compat-* one or replaced by a new binary compatible one (think fam vs. gamin)... relying only on automatic deps makes things easier since as long as the same libraries are provided somewhere, things won't break. This was untrue in the "old days" when apt for rpm, yum or even up2date didn't exist, since an explicit missing dep on "GConf2" lead more easily to the package required to satisfy the dependency than "libgconf-2.so.4". Anyway, just take a look at the output, it should make you feel more comfortable ;-) : rpm -q --requires NetworkManager-gnome
I see these changes have been merged in the FC3 update that just got released, thanks! The dbus_version and hald_version macros are nice and practical too ;-)