Merge lp:~xnox/hw-detect/fix-firmware-order into lp:~ubuntu-core-dev/hw-detect/ubuntu

[Dimitri John Ledkov]

[Adam Conrad (adconrad)]

50-install-firmware is possibly the scariest thing I've read all week, but +1 to the change in this MP for shuffling it around in the installation process.

[Adam Conrad (adconrad)]

I take it back, after thinking about it a bit. Random chatter from IRC:

12:20 < infinity> xnox: Hrm, after I've approved it, I now wonder if the above scary bit is a
                  problem with reordering.
12:20 < infinity> xnox: Since it could be overwriting firmware from linux-firmware (presumably, it
                  ran before linux-firmware was installed before you moved it)
12:21 < infinity> Honestly, I wonder who thought that was a good idea AT ALL. Copying random
                  firmware from the installer to the target system just seems scary.
12:26 < superm1> 50-install-firmware wasn't written as part of the driver injection disk stuff, i
                 thought that part came from debian from earlier
12:26 < infinity> Indeed, you just leveraged it, since it already copies debs around and installs
                  them.
12:26 < infinity> In fact...
12:26 < infinity> Hrm.
12:27 < infinity> xnox: It might be better to put 50-install-firmware back where it came from.
12:27 < infinity> xnox: And do a new XX-driver-disk in hw-detect.pre-pkgsel.d that uses a different
                  /tmp directory.
12:28 < infinity> xnox: And mangle driver-injection-disk.sh to use that instead of overloading
                  /tmp/firmware

[Colin Watson (cjwatson)]

There's another ordering problem with moving these, although technically it's probably only a problem when merging the change back to Debian: the apt-setup/non-free handling needs to come before apt-setup runs, and pre-pkgsel.d is too late for that.

For now we should keep the handling of loose firmware files, although I agree that it has some obvious problems such as not distinguishing those that were inserted manually from those that are in linux-firmware. We don't need to rearrange this to fix the bug at hand.

My suggestion would be:

 * strip out the parts of hw-detect.post-base-installer.d/50install-firmware that actually install the .debs, but keep the handling of loose firmware files and of apt-setup/non-free (in other words, if there's anything in /var/cache/firmware/*.deb that passes [ -f ], set apt-setup/non-free to true)
 * create a new hw-detect/pre-pkgsel.d/50install-firmware and move the parts of the previous code that actually install the .debs there

[lp:~xnox/hw-detect/fix-firmware-order]

1486. By Dimitri John Ledkov on 2013-09-05

catch up from trunk

1487. By Dimitri John Ledkov on 2013-09-05

revert initial implementation

1488. By Dimitri John Ledkov on 2013-09-05

* Make driver-injection-disk use a directory which is different from the
  one firmware injection uses.
* Move driver-injection-disk debs installation to pre-pkgsel.d stage,
  such that it is after the kernel has been installed. In cases injected
  driver .debs depend on particular kernel packages.

1489. By Dimitri John Ledkov on 2013-09-05

Enable nonfree repositories in install-firmware, if there are injected
drivers to install later.

1490. By Dimitri John Ledkov on 2013-09-06

Move firmware/injected drivers' .deb packages installation from
post-base-installer.d stage to pre-pkgsel.d stage.

[Dimitri John Ledkov (xnox)]

Following a discussion with Colin on IRC, I agree with implementation outline in his comment above.
Branch updated.

[lp:~xnox/hw-detect/fix-firmware-order]

1491. By Dimitri John Ledkov on 2013-09-11

Bump question to load driver injection disk from medium to high, and
raise driver-injection-disk package priority to standard. Thus the
udeb will be loaded by default, but before installing any debs from
the OEMDRM a confirmation question will be asked (default true, can be
pre-seeded)

[Adam Conrad (adconrad)]

This looks much saner now, except for the s/OEMDRV/OEMDRM/ typo in your changelog. I'm happy with it.

[Adam Conrad (adconrad)]

review: Approve

[Adam Conrad (adconrad)]

Oh, and please close the relevant bug in your changelog too.

[lp:~xnox/hw-detect/fix-firmware-order]

1492. By Dimitri John Ledkov on 2013-09-19

OEMDRV typo

1493. By Dimitri John Ledkov on 2013-09-20

Add bug #

=== modified file 'debian/changelog'
--- debian/changelog	2013-09-05 14:06:58 +0000
+++ debian/changelog	2013-09-20 15:34:57 +0000
@@ -1,5 +1,6 @@
 hw-detect (1.95ubuntu1) UNRELEASED; urgency=low
 
+  [ Colin Watson ]
   * Resynchronise with Debian.  Remaining changes:
     - Remove FireWire Ethernet support.
     - Register lp module on i386 and amd64.
@@ -30,6 +31,16 @@
   * Drop changes for PS3 and Cell, since we haven't been able to support
     these platforms for some time.
 
+  [ Dmitrijs Ledkovs ]
+  * Move firmware/injected drivers' .deb packages installation from
+    post-base-installer.d stage to pre-pkgsel.d stage. (LP: #1209287, LP:
+    #1216043)
+  * Bump question to load driver injection disk from medium to high, and
+    raise driver-injection-disk package priority to standard. Thus the
+    udeb will be loaded by default, but before installing any debs from
+    the OEMDRV a confirmation question will be asked (default true, can be
+    pre-seeded)
+
  -- Colin Watson <[e-mail]>  Thu, 05 Sep 2013 15:01:17 +0100
 
 hw-detect (1.95) unstable; urgency=low

=== modified file 'debian/control'
--- debian/control	2013-09-05 14:02:44 +0000
+++ debian/control	2013-09-20 15:34:57 +0000
@@ -38,7 +38,6 @@
 Package-Type: udeb
 Architecture: all
 Depends: cdebconf-udeb (>= 0.38), hw-detect, mountmedia
-Priority: optional
 Enhances: hw-detect
 XB-Installer-Menu-Item: 1500
 Description: Detect OEM driver injection disks

=== modified file 'driver-injection-disk.sh' (properties changed: -x to +x)
--- driver-injection-disk.sh	2011-03-12 17:49:47 +0000
+++ driver-injection-disk.sh	2013-09-20 15:34:57 +0000
@@ -32,7 +32,7 @@
 for device in $(list-devices usb-partition); do
 	label=$(block-attr --label $device 2>/dev/null || true)
 	if [ "$label" = "OEMDRV" ]; then
-		db_input medium driver-injection-disk/load || true
+		db_input high driver-injection-disk/load || true
 		if ! db_go; then
 			exit 10 # back up
 		fi

=== modified file 'hw-detect.post-base-installer.d/50install-firmware'
--- hw-detect.post-base-installer.d/50install-firmware	2010-03-09 19:17:16 +0000
+++ hw-detect.post-base-installer.d/50install-firmware	2013-09-20 15:34:57 +0000
@@ -12,26 +12,8 @@
 	done
 fi
 
-deb_package () {
-	ar p "$1" control.tar.gz | tar zxO ./control | grep 'Package:' |  sed -e 's/Package: *//'
-}
-
-# install cached firmware debs
-if [ -d /var/cache/firmware ]; then
-	for deb in /var/cache/firmware/*.deb; do
-		if [ -f "$deb" ]; then
-			cp -a "$deb" /target/tmp
-			# TODO debconf passthrough
-			if ! in-target dpkg -i "/tmp/$(basename "$deb")"; then
-				# dpkg failed, force removal of package
-				in-target dpkg --force-depends --remove "$(deb_package "$deb")" || true
-			fi
-			rm -f "/target/tmp/$deb"
-			need_nonfree=1
-		fi
-	done
-fi
-
-if [ "$need_nonfree" ]; then
+# enable non-free repository if any firmware / injected drivers are
+# detected.
+if [ -n "/var/cache/firmware/*.deb" ]; then
 	db_set apt-setup/non-free true
 fi

=== added file 'hw-detect.pre-pkgsel.d/50install-firmware'
--- hw-detect.pre-pkgsel.d/50install-firmware	1970-01-01 00:00:00 +0000
+++ hw-detect.pre-pkgsel.d/50install-firmware	2013-09-20 15:34:57 +0000
@@ -0,0 +1,21 @@
+#!/bin/sh
+set -e
+
+deb_package () {
+	ar p "$1" control.tar.gz | tar zxO ./control | grep 'Package:' |  sed -e 's/Package: *//'
+}
+
+# install cached firmware debs
+if [ -d /var/cache/firmware ]; then
+	for deb in /var/cache/firmware/*.deb; do
+		if [ -f "$deb" ]; then
+			cp -a "$deb" /target/tmp
+			# TODO debconf passthrough
+			if ! in-target dpkg -i "/tmp/$(basename "$deb")"; then
+				# dpkg failed, force removal of package
+				in-target dpkg --force-depends --remove "$(deb_package "$deb")" || true
+			fi
+			rm -f "/target/tmp/$deb"
+		fi
+	done
+fi