Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=469931 Terje Røsten <terjeros@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo? | --- Comment #31 from Terje Røsten <terjeros@xxxxxxxxxxxx> 2010-04-28 13:56:39 EDT --- Hi Andy, things are getting better. There are some small style issues, only some hangups I have that is not important. However there are still problems with the scripts. Let's looking at it line by line. > %pre %pre is empty, so remove that tag. > %post > pdir=%{_sbindir} This don't make sense, why define pdir as %{_sbindir}, use %{_sbindir} directly (if needed). > # echo "Installing IPMI Management Utilities %{version} ..." rpms should not generate any output (unless something wrong happens). Remove this line. > # POST_INSTALL post install in %post, well we know that, remove. > # check if chroot, which will not have /proc instances > if [ -f /proc/devices ] > then > isroot=1 > else > isroot=0 > fi > if [ "$1" = "1" ] > then > # doing rpm -i, first time > if [ $isroot -eq 1 ] > then > # detects IPMI interface type, output to %{_datadir}/%{name}/ipmi_if.txt > %{_datadir}/%{name}/ipmi_if.sh > > # Run some ipmiutil command to see if any IPMI interface works. > $pdir/ipmiutil wdt >/dev/null 2>&1 > fIPMIret=$? > else > # else chroot, set as if IPMI is not supported > fIPMIret=1 > fi > > # If fIPMIret==0, the IPMI cmd was successful, and IPMI is enabled. > if [ $fIPMIret -eq 0 ] > then You should move all this logic to the init scripts itself. When a user is adding the ipmtutil package, he/she will be surprised if the init scripts are not registered in the chkconfig system. If something goes when starting e.g. ipmi_port let the scripts itself return the correct error message and exit status code. More about init scripts in full glory details here: https://fedoraproject.org/wiki/Packaging:SysVInitScript > # IPMI_IS_ENABLED > # Enable services: ipmi_port reserves the IPMI RMCP port from portmap > if [ -x /sbin/chkconfig ]; then > /sbin/chkconfig --add ipmi_port > /sbin/chkconfig --add ipmiutil_wdt > /sbin/chkconfig --add ipmiutil_asy > fi > # Capture a snapshot of IPMI sensor data for later reuse. > sensorout=%{_datadir}/%{name}/sensor_out.txt > if [ ! -f $sensorout ] > then > echo "Saving a sensor snapshot to $sensorout ..." > $pdir/ipmiutil sensor >$sensorout Why are you doing this? And again, rpms should not produce any output. > fi > fi > fi > echo "done `date`" Please remove this too. > %preun > # before uninstall > echo "Uninstalling ipmiutil-%{version} package ..." yum/rpm will inform the user any way, remove this > # arg1 = 1 if rpm -U, arg1 = 0 if rpm -e > if [ "$1" = "0" ] >then > if [ -x %{_initrddir}/ipmi_port ] > then > %{_initrddir}/ipmi_port stop >/dev/null 2>&1 > %{_initrddir}/ipmiutil_wdt stop >/dev/null 2>&1 > %{_initrddir}/ipmiutil_asy stop >/dev/null 2>&1 > if [ -x /sbin/chkconfig ]; then > /sbin/chkconfig --del ipmi_port >/dev/null 2>&1 > /sbin/chkconfig --del ipmiutil_wdt >/dev/null 2>&1 > /sbin/chkconfig --del ipmiutil_asy >/dev/null 2>&1 > fi > fi >fi This is better than %post, however not according to policy: https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_in_spec_file_scriptlets > %postun Same as %pre, remove. One more thing, I had look at the script %{_datadir}/%{name}/ipmi_if.txt First it seems to require dmidecode, so add a requires on the dmidecode package. Then there is security issue: dmiout=/tmp/dmi.out [snip] dmidecode >$dmiout Remember the system is running this script as root, if a user does $ ln -s /etc/passwd /tmp/dmi.out before ipmutil is installed, the system is broken after impiutil is installed. A simple fix is to use this trick: dmiout=$(mktemp /tmp/impiutil.XXXXXX) You also have ifdir=/usr/share/ipmiutil ifout=$ifdir/ipmi_if.txt and writes to this file, however you should not write to files in /usr subdirs. If you want to write to files, use /var/lib/ e.g. /var/lib/ipmiutil/ipmi_if.txt -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review