Re: [PATCH] The Ubuntu Collection

[Scott James Remnant <scott@ubuntu.com>]

[-- Attachment #1: Type: text/plain, Size: 4837 bytes --]

On Wed, 2005-12-07 at 23:37 +0100, Kay Sievers wrote:

> On Wed, Dec 07, 2005 at 05:12:59PM +0000, Scott James Remnant wrote:
> > 01-lib-udev.patch
> >     This arranges for the various extras programs to be installed
> >     directly into /lib/udev/ rather than into /sbin/.  Pretty sweet and
> >     obvious.
> 
> That's nice. I wanted to do this too, but I need to make sure, not to
> break existing users of the tools, cause they have been installed in
> sbin for a long time. But in general I agree, that we should move all
> the udev tools to /lib/udev. Btw: Marco had a nice idea and udev 077
> automatically picks up the tools there, if RUN="firmware.sh" is given.
> 
Yup, I've already picked that up and run with it.

> > 50-result-whitespace.patch
> >     In a few places udev calls the replace_untrusted_chars() function on
> >     things like program output; it treats \t, \r, \n, and other
> >     whitespace characters (other than space itself) as illegal and
> >     replaces them with an underscore.  This patch slightly alters that
> >     behaviour, it leaves them as untrusted, but instead replaces those
> >     of whitespace-ilk with an ordinary space, instead of an underscore. 
> >     We use this because various helpers output a newline-separated list
> >     of modules, and we wanted to pass that $result straight to modprobe.
> 
> Yes, I see, may make sense at some places as a separate function. What are
> the tools you call and this is useful?
> 
Mostly it's just our pnp_modules helper, and grepmap -- we could modify
those to output space-separated modules, but then it prevents other
people from doing the same.  This seemed neater, somehow.

> > 55-run-program.patch
> >     This reduces the severity of the "exec of program failed" error IF
> >     the reason the program couldn't be run was because it didn't exist
> >     yet.  With /dev/.udev/failed, helpers not existing yet isn't
> >     serious, it just means they need to be retried later and there's no
> >     point screaming too loudly about it.
> 
> Yeah, I'm still unsure if the used tool should be required to be in the
> rootfs or not. But it sounds like a sane default, if we don't require
> this.
> 
Right, this was actually because we don't include some of the helpers in
the initramfs; we've established a "no helpers under /usr" policy for
the rest.

> For the higher level things HAL is the approproate place to do device
> setup anyway. That's the reason I ignored all the map conversion things,
> cause digital cameras, scanners and such things belong to HAL these days
> and not in udev rules.
> 
Right, actually the digital camera ones are just GROUP= rules now for
the usb_device subsystem -- those were easy.

> > 60-firmware-hunt.patch
> >     This modifies the firmware_helper extra to also look in
> >     /lib/firmware/$KERNEL_VERSION which is where we put firmware that
> >     comes with our kernel images.
> 
> Hmm, I'm no longer sure that the C programs for something that is
> usually 20 lines of shell long and called 3 times at bootup make sense.
> For the firmware case I always wanted to add some handling for missing
> firmware, but even for this we would not need a C program. Hmm...
> 
I like C, and they run about a zillion times faster than the busybox
shell; probably why I'm hitting so many new race conditions, but in
general I tend to prefer little binary helpers as they're less
susceptible to the various silly shell quirks.

I won't be upset if you don't accept a single one ;)  they're pretty
specific to our needs.

> > 80-extras-storage_enum.patch
> >     An odd one, we use this in the installer to make the
> >     devfs-like /dev/discs, /dev/cdroms and /dev/floppy directories which
> >     the debian-installer we use still requires.  We didn't use the
> >     existing devfs scripts because they don't cope well with everything
> >     getting plugged at once, this is reasonably safe against that and
> >     ensures that each gets a unique number.
> 
> Yeah, you should better get rid of _any_ simple enumeration now on a
> system where devices can come and go any time and no order is guaranteed.
> Such an approach will cause much more trouble than it solves. :)
> 
Right, this is just used in installer-world; not in the real system.
I'm begging Colin to use /sys to detect cd-roms etc. but he currently
has more pressing assignments.

> > 80-extras-vio_type.patch
> >     Binary helper to look up the type of a vio device under /proc, and
> >     like ide_media wait for the node to appear to avoid races.
> 
> Is this needed, even as we have MODALIAS for vio now?
> 
Yeah, the devices this deals with actually don't have MODALIAS yet.

Scott
-- 
Scott James Remnant
scott@ubuntu.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]