Hi, On Sun, Mar 09, 2008 at 05:41:05AM +0000, Joseph Nahmias wrote: > On Fri, Mar 07, 2008 at 08:07:14PM +0100, Sebastian Harl wrote: > > On Fri, Mar 07, 2008 at 06:47:53PM +0100, Julien Cristau wrote: > > > On Fri, Mar 7, 2008 at 18:36:07 +0100, Sebastian Harl wrote: > > > > Sorry, this patch does not change anything. "a && b" is basically the > > > > same as "if a; then b; fi" which is what you're doing here. This issue > > > > has already been addressed in the fix for #447961. > > > > > > > Well no. "a && b" is only successful if both "a" and "b" exit > > > successfully. If you don't want to fail when udev is not running, that > > > patch is correct. > > > > Well, the problem is that "udev restart" fails (which is "b" in this > > case). It doesn't really matter if "a && b" fails (that's the reason I > > said "basically the same" ;-) as the "-e" option has not been set and > > thus the return value will simply be ignored - which (imho) is perfectly > > fine in this case. > > Um, unless my eyes deceive me, -e is set right on the first line -- just > as it should per policy.
Right, sorry for that - I wonder why it has worked in my tests then... ;-) > However, you bring up a good point in that the > udevd process that is running outside the chroot will be visible to > pidof within the chroot. Good catch! > --- postinst 2008-02-08 10:13:24.000000000 -0500 > +++ postinst.fixed 2008-03-09 00:34:04.000000000 -0500 > @@ -19,8 +19,14 @@ case "$1" in > chmod 770 /var/run/nut /var/lib/nut > > # restart udev to apply the USB rules to the already plugged devices > - [ -x /etc/init.d/udev ] && pidof udevd > /dev/null \ > - && /usr/sbin/invoke-rc.d udev restart > + # only if it's already running in this environment > + udevd_pid=$(pidof udevd) This should be "udevd_pid=$( pidof udevd 2> /dev/null ) || true" - the redirection of stderr is not strictly required but I'd add it for the sake of completeness. "|| true" is obviously required to not terminate because of "-e" in case udevd isn't running. > + if [ -x /etc/init.d/udev ] && [ -n "$udev_pid" ]; then > + if [ "$(stat -c %d/%i /)" = "$(stat -Lc %d/%i /proc/$udev_pid/root > 2>/dev/null)" ]; The builtin stat shipped with zsh does not seem to recognize the "-c" option. So, we should use "/usr/bin/stat" here to catch the (unlikely) case that anybody uses zsh (or possible other "incompatible" shells) as /bin/sh. Also, if udevd would be running both inside and outside the chroot (which does not seem to be possible from a quick test though [1]) pidof would return two PID's. So, I guess, we should put a "for pid in $udev_pid" around the inner if-statement (and replace $udev_pid with $pid in the second call to stat(1)). > + then > + /usr/sbin/invoke-rc.d udev restart > + fi > + fi > ;; > > abort-upgrade) Thanks for the patch - imho it should be uploaded ASAP as it prevents other packages from being built. Arnaud, any comments? Cheers, Sebastian [1] When trying to start udev inside the chroot, it fails with "erorr initializing udevd socket". -- Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/ Those who would give up Essential Liberty to purchase a little Temporary Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin
signature.asc
Description: Digital signature
