partman-auto-crypto - round2 :)

[Max Vozeler <max@nusquama.org>]

Hi all,

I've had another detailed look at partman-auto-crypto tonight.
(/people/alphix-guest/partman-auto-crypto) Quick summary: The
package seems solid from my tests and review of the code; I 
think it's ready to move to trunk/ and have a first upload after
two points are solved.

Thanks to David's recent work, p-a-c has shrunk significantly 
and there is now little code specific to the crypto parts - most
is shared with -auto and -auto-lvm. Although I don't feel very
competent to speak about partman-auto in general, I can say that
the crypto parts look fine to me and worked flawlessly in two
test installs I did tonight, 

What I think needs to be solved before a first upload:

  1. Priority. The package currently has standard priority, 
  which gets it loaded by default installs. I'm not sure given
  the closeness to rc1 and would prefer input about this point.
  Should we start with optional and promote to standard later -
  or vice versa in case problems should show up?

  2. Dependencies. I think they need to be tightened a bit to
  reflect code now shared with -auto{,-lvm} after the reorga-
  nisation. I'm thinking of select_auto_disk() for example which
  seems to depends on partman-auto-crypto >= 55. It would be
  great if they could be updated.

Less important to minor (mostly direct feedback to David)

  3. Yet a bit more code could perhaps be dropped, I think :-)
  The open_dialog PARTITIONS loops save details about individual
  partitions which don't seem to actually get used afterwards.
  Please have a look at the attached patch to see if I don't have
  a thinko. (It's been a long day)

  4. debian/copyright still mentions substantial changes to
  automatically_partition/some_device_crypto/do_option; Does this
  still apply given the reorganisation?

If we don't find any new problems, and depending on feedback, I
hope to move the package to trunk/packages/partman/ later this
week and maybe look into a first upload this weekend. Please let
us know if you see other issues/questions to be resolved or any
reasons to wait yet a little bit longer. In particular I think it
would be important to have an evaluation from a d-i release
perspective before we make a first upload.

cheers,
Max

Index: partman-auto-crypto/autopartition-crypto
===================================================================
--- partman-auto-crypto/autopartition-crypto	(revision 40477)
+++ partman-auto-crypto/autopartition-crypto	(working copy)
@@ -14,25 +14,18 @@
 
 found="no"
 for dev in $DEVICES/*; do
-
 	[ -d "$dev" ] || continue
 	cd $dev
 	partitions=
 	open_dialog PARTITIONS
 	while { read_line num id size type fs path name; [ "$id" ]; }; do
 		if [ "$fs" != free ]; then
-			partitions="$partitions $id:$num:$size:$path"
+			partitions="$partitions $id"
 		fi
 	done
         close_dialog
 
-	for p in $partitions; do
-		set -- $(IFS=: && echo $p)
-		id=$1
-		num=$2
-		size=$3
-		path=$4
-
+	for id in $partitions; do
 		[ -f $id/method ] || continue
 		method=$(cat $id/method)
 		[ $method = crypto ] || continue
@@ -48,7 +41,7 @@
 crypto_check_setup || exit 1
 crypto_setup no || exit 1
 
-# This is a cludge to workaround parted's refusal to allow dm devices
+# This is a kludge to workaround parted's refusal to allow dm devices
 # to be used for LVM
 for dev in $DEVICES/*; do
 	[ -d "$dev" ] || continue
@@ -64,13 +57,11 @@
 	open_dialog PARTITIONS
 	while { read_line num id size type fs path name; [ "$id" ]; }; do
 		[ "$fs" != free ] || continue
-		partitions="$partitions $id,$path"
+		partitions="$partitions $id"
 	done
 	close_dialog
 
-	for part in $partitions; do
-		id=${part%,*}
-		path=${part#*,}
+	for id in $partitions; do
 		for file in acting_filesystem filesystem format formatable use_filesystem; do
 			rm -f $id/$file
 		done
Index: partman-auto-crypto/debian/rules
===================================================================
--- partman-auto-crypto/debian/rules	(revision 40477)
+++ partman-auto-crypto/debian/rules	(working copy)
@@ -21,9 +21,8 @@
 	dh_testroot
 	dh_clean -k
 	dh_install autopartition-crypto bin
-	debian/install-rc automatically_partition
 	dh_install perform_recipe_by_crypto bin
-	rm -rf `find debian/$(PACKAGE) -name CVS`
+	debian/install-rc automatically_partition
 	rm -rf `find debian/$(PACKAGE) -name .svn`
 
 binary-indep: build install