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
