On Mon, Jun 19, 2006 at 11:15:54PM +0200, Frans Pop wrote:
On Monday 19 June 2006 22:56, David Härdeman wrote:
currently at least partman-lvm, partman-crypto and partman-md seem to
contain copies of confirm_changes() from partman-base/partman.
They are at least different in that they use some differently named
templates at some points.
I've done a manual pen-and-paper comparison of the four implementations
in partman-{base,md,lvm,crypto}. The results are at
http://www.hardeman.nu/~david/files/patches/debian/confirm_changes_diff.html
Would it perhaps be an idea to move confirm_changes into
/lib/partman/definitions.sh or is there a reason for the duplication?
If that is the only difference though, it should be possible to generalize
that and pass the template (or even only the component name) as a
parameter.
Looking at the above mentioned comparison, there are three differences:
1) The templates used
This can be taken care of with a parameter as you mentioned
2) Additional comments in partman-base
Not relevant
3) Additional detection of previously formatted partitions
Looking at this difference, it seems like a feature that should
be made available in a general version of the function.
I've attached a patch which merges the four implementations into one
while taking the above comments into consideration. The result is a net
reduction of a bit less than 300 lines of code.
diffstat:
partman-base/definitions.sh | 108 +++++++++++++++++++++++
partman-base/partman | 107 ----------------------
partman-crypto/choose_partition/crypto/do_option | 96 --------------------
partman-lvm/choose_partition/lvm/do_option | 94 --------------------
partman-md/choose_partition/md/do_option | 94 --------------------
5 files changed, 112 insertions(+), 387 deletions(-)
I've done a build and some testing, and it seems to work.
Any objections to committing it?
Regards,
David
Index: partman-base/definitions.sh
===================================================================
--- partman-base/definitions.sh (revision 38213)
+++ partman-base/definitions.sh (working copy)
@@ -927,6 +927,114 @@
done
}
+# List the changes that are about to be committed and let the user confirm
first
+confirm_changes () {
+ local template dev x part partitions num id size type fs path name
filesystem partitems items formatted_previously
+ template="$1"
+
+ # Compute the changes we are going to do
+ partitems=''
+ items=''
+ for dev in $DEVICES/*; do
+ [ -d "$dev" ] || continue
+ cd $dev
+
+ open_dialog IS_CHANGED
+ read_line x
+ close_dialog
+ if [ "$x" = yes ]; then
+ partitems="${partitems} $(humandev $(cat device))
+"
+ fi
+
+ partitions=
+ open_dialog PARTITIONS
+ while { read_line num id size type fs path name; [ "$id" ]; };
do
+ [ "$fs" != free ] || continue
+ partitions="$partitions $id,$num"
+ done
+ close_dialog
+
+ formatted_previously=no
+ for part in $partitions; do
+ id=${part%,*}
+ num=${part#*,}
+ [ -f $id/method -a -f $id/format \
+ -a -f $id/visual_filesystem ] || continue
+ # if no filesystem (e.g. swap) should either be not
+ # formatted or formatted before the method is specified
+ [ -f $id/filesystem -o ! -f $id/formatted \
+ -o $id/formatted -ot $id/method ] || continue
+ # if it is already formatted filesystem it must be
formatted
+ # before the method or filesystem is specified
+ [ ! -f $id/filesystem -o ! -f $id/formatted \
+ -o $id/formatted -ot $id/method \
+ -o $id/formatted -ot $id/filesystem ] ||
+ {
+ formatted_previously=yes
+ continue
+ }
+ filesystem=$(cat $id/visual_filesystem)
+ db_subst partman/text/confirm_item TYPE "$filesystem"
+ db_subst partman/text/confirm_item PARTITION "$num"
+ db_subst partman/text/confirm_item DEVICE $(humandev
$(cat device))
+ db_metaget partman/text/confirm_item description
+
+ items="${items} ${RET}
+"
+ done
+ done
+
+ if [ "$items" ]; then
+ db_metaget partman/text/confirm_item_header description
+ items="$RET
+$items"
+ fi
+
+ if [ "$partitems" ]; then
+ db_metaget partman/text/confirm_partitem_header description
+ partitems="$RET
+$partitems"
+ fi
+
+ if [ "$partitems$items" ]; then
+ if [ -z "$items" ]; then
+ x="$partitems"
+ elif [ -z "$partitems" ]; then
+ x="$items"
+ else
+ x="$partitems
+$items"
+ fi
+ db_subst $template/confirm ITEMS "$x"
+ db_input critical $template/confirm
+ db_go || true
+ db_get $template/confirm
+ if [ "$RET" = false ]; then
+ db_reset $template/confirm
+ return 1
+ else
+ db_reset $template/confirm
+ return 0
+ fi
+ else
+ if [ "$formatted_previously" = no ]; then
+ db_input high $template/confirm_nochanges
+ db_go || true
+ db_get $template/confirm_nochanges
+ if [ "$RET" = false ]; then
+ db_reset $template/confirm_nochanges
+ return 1
+ else
+ db_reset $template/confirm_nochanges
+ return 0
+ fi
+ else
+ return 0
+ fi
+ fi
+}
+
log '*******************************************************'
# Local Variables:
Index: partman-base/partman
===================================================================
--- partman-base/partman (revision 38213)
+++ partman-base/partman (working copy)
@@ -9,111 +9,6 @@
exit $1
}
-confirm_changes () {
- local dev x part partitions num id size type fs path name filesystem
partitems items formatted_previously
- # Compute the changes we are going to do
- partitems=''
- items=''
- for dev in $DEVICES/*; do
- [ -d "$dev" ] || continue
- cd $dev
-
- open_dialog IS_CHANGED
- read_line x
- close_dialog
- if [ "$x" = yes ]; then
- partitems="${partitems} $(humandev $(cat device))
-"
- fi
-
- partitions=
- open_dialog PARTITIONS
- while { read_line num id size type fs path name; [ "$id" ]; }; do
- [ "$fs" != free ] || continue
- partitions="$partitions $id,$num"
- done
- close_dialog
-
- formatted_previously=no
- for part in $partitions; do
- id=${part%,*}
- num=${part#*,}
- [ -f $id/method -a -f $id/format \
- -a -f $id/visual_filesystem ] || continue
- # if no filesystem (e.g. swap) should either be not
- # formatted or formatted before the method is specified
- [ -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method ] || continue
- # if it is already formatted filesystem it must be formatted
- # before the method or filesystem is specified
- [ ! -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method \
- -o $id/formatted -ot $id/filesystem ] ||
- {
- formatted_previously=yes
- continue
- }
- filesystem=$(cat $id/visual_filesystem)
- db_subst partman/text/confirm_item TYPE "$filesystem"
- db_subst partman/text/confirm_item PARTITION "$num"
- db_subst partman/text/confirm_item DEVICE $(humandev $(cat device))
- db_metaget partman/text/confirm_item description
-
- items="${items} ${RET}
-"
- done
- done
-
- if [ "$items" ]; then
- db_metaget partman/text/confirm_item_header description
- items="$RET
-$items"
- fi
-
- if [ "$partitems" ]; then
- db_metaget partman/text/confirm_partitem_header description
- partitems="$RET
-$partitems"
- fi
-
- if [ "$partitems$items" ]; then
- if [ -z "$items" ]; then
- x="$partitems"
- elif [ -z "$partitems" ]; then
- x="$items"
- else
- x="$partitems
-$items"
- fi
- db_subst partman/confirm ITEMS "$x"
- db_input critical partman/confirm
- db_go || true
- db_get partman/confirm
- if [ "$RET" = false ]; then
- db_reset partman/confirm
- return 1
- else
- db_reset partman/confirm
- return 0
- fi
- else
- if [ "$formatted_previously" = no ]; then
- db_input high partman/confirm_nochanges
- db_go || true
- db_get partman/confirm_nochanges
- if [ "$RET" = false ]; then
- db_reset partman/confirm_nochanges
- return 1
- else
- db_reset partman/confirm_nochanges
- return 0
- fi
- else
- return 0
- fi
- fi
-}
-
db_capb backup
# Measure the width of partman/text/number here to make things faster.
@@ -168,7 +63,7 @@
exitcode=$?
if [ $exitcode -eq 255 ]; then
abort 10 # back up to main menu
- elif [ $exitcode -ge 100 ] && confirm_changes; then
+ elif [ $exitcode -ge 100 ] && confirm_changes "partman"; then
break
fi
done
Index: partman-lvm/choose_partition/lvm/do_option
===================================================================
--- partman-lvm/choose_partition/lvm/do_option (revision 38213)
+++ partman-lvm/choose_partition/lvm/do_option (working copy)
@@ -28,7 +28,7 @@
log-output -t partman-lvm vgscan
# Commit the changes
- confirm_changes || exit 0
+ confirm_changes "partman-lvm" || exit 0
for s in /lib/partman/commit.d/*; do
if [ -x $s ]; then
$s || {
@@ -72,98 +72,6 @@
fi
}
-confirm_changes () {
- local dev x part partitions num id size type fs path name filesystem
partitems items
- # Compute the changes we are going to do
- partitems=''
- items=''
- for dev in $DEVICES/*; do
- [ -d "$dev" ] || continue
- cd $dev
-
- open_dialog IS_CHANGED
- read_line x
- close_dialog
- if [ "$x" = yes ]; then
- partitems="${partitems} $(humandev $(cat device))
- "
- fi
-
- partitions=
- open_dialog PARTITIONS
- while { read_line num id size type fs path name; [ "$id" ]; };
do
- [ "$fs" != free ] || continue
- partitions="$partitions $id,$num"
- done
- close_dialog
-
- for part in $partitions; do
- id=${part%,*}
- num=${part#*,}
- [ -f $id/method -a -f $id/format \
- -a -f $id/visual_filesystem ] || continue
- [ -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method ] || continue
- [ ! -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method \
- -o $id/formatted -ot $id/filesystem ] || continue
- filesystem=$(cat $id/visual_filesystem)
- db_subst partman/text/confirm_item TYPE "$filesystem"
- db_subst partman/text/confirm_item PARTITION "$num"
- db_subst partman/text/confirm_item DEVICE $(humandev
$(cat device))
- db_metaget partman/text/confirm_item description
-
- items="${items} ${RET}
-"
- done
- done
-
- if [ "$items" ]; then
- db_metaget partman/text/confirm_item_header description
- items="$RET
-$items"
- fi
-
- if [ "$partitems" ]; then
- db_metaget partman/text/confirm_partitem_header description
- partitems="$RET
-$partitems"
- fi
-
- if [ "$partitems$items" ]; then
- if [ -z "$items" ]; then
- x="$partitems"
- elif [ -z "$partitems" ]; then
- x="$items"
- else
- x="$partitems
-$items"
- fi
- db_subst partman-lvm/confirm ITEMS "$x"
- db_input critical partman-lvm/confirm
- db_go || true
- db_get partman-lvm/confirm
- if [ "$RET" = false ]; then
- db_reset partman-lvm/confirm
- return 1
- else
- db_reset partman-lvm/confirm
- return 0
- fi
- else
- db_input critical partman-lvm/confirm_nochanges
- db_go || true
- db_get partman-lvm/confirm_nochanges
- if [ "$RET" = false ]; then
- db_reset partman-lvm/confirm_nochanges
- return 1
- else
- db_reset partman-lvm/confirm_nochanges
- return 0
- fi
- fi
-}
-
do_display() {
lvm_get_config
db_subst partman-lvm/displayall CURRENT_CONFIG "$RET"
Index: partman-md/choose_partition/md/do_option
===================================================================
--- partman-md/choose_partition/md/do_option (revision 38213)
+++ partman-md/choose_partition/md/do_option (working copy)
@@ -2,100 +2,8 @@
. /lib/partman/definitions.sh
-confirm_changes () {
- local dev x part partitions num id size type fs path name filesystem
partitems items
- # Compute the changes we are going to do
- partitems=''
- items=''
- for dev in $DEVICES/*; do
- [ -d "$dev" ] || continue
- cd $dev
+confirm_changes "partman-md" || exit 0
- open_dialog IS_CHANGED
- read_line x
- close_dialog
- if [ "$x" = yes ]; then
- partitems="${partitems} $(humandev $(cat device))
-"
- fi
-
- partitions=
- open_dialog PARTITIONS
- while { read_line num id size type fs path name; [ "$id" ]; }; do
- [ "$fs" != free ] || continue
- partitions="$partitions $id,$num"
- done
- close_dialog
-
- for part in $partitions; do
- id=${part%,*}
- num=${part#*,}
- [ -f $id/method -a -f $id/format \
- -a -f $id/visual_filesystem ] || continue
- [ -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method ] || continue
- [ ! -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method \
- -o $id/formatted -ot $id/filesystem ] || continue
- filesystem=$(cat $id/visual_filesystem)
- db_subst partman/text/confirm_item TYPE "$filesystem"
- db_subst partman/text/confirm_item PARTITION "$num"
- db_subst partman/text/confirm_item DEVICE $(humandev $(cat device))
- db_metaget partman/text/confirm_item description
-
- items="${items} ${RET}
-"
- done
- done
-
- if [ "$items" ]; then
- db_metaget partman/text/confirm_item_header description
- items="$RET
-$items"
- fi
-
- if [ "$partitems" ]; then
- db_metaget partman/text/confirm_partitem_header description
- partitems="$RET
-$partitems"
- fi
-
- if [ "$partitems$items" ]; then
- if [ -z "$items" ]; then
- x="$partitems"
- elif [ -z "$partitems" ]; then
- x="$items"
- else
- x="$partitems
-$items"
- fi
- db_subst partman-md/confirm ITEMS "$x"
- db_input critical partman-md/confirm
- db_go || true
- db_get partman-md/confirm
- if [ "$RET" = false ]; then
- db_reset partman-md/confirm
- return 1
- else
- db_reset partman-md/confirm
- return 0
- fi
- else
- db_input high partman-md/confirm_nochanges
- db_go || true
- db_get partman-md/confirm_nochanges
- if [ "$RET" = false ]; then
- db_reset partman-md/confirm_nochanges
- return 1
- else
- db_reset partman-md/confirm_nochanges
- return 0
- fi
- fi
-}
-
-confirm_changes || exit 0
-
# Commit the changes
for s in /lib/partman/commit.d/*; do
Index: partman-crypto/choose_partition/crypto/do_option
===================================================================
--- partman-crypto/choose_partition/crypto/do_option (revision 38223)
+++ partman-crypto/choose_partition/crypto/do_option (working copy)
@@ -15,100 +15,6 @@
crypto_check_setup || exit 1
-# this comes from partman-lvm
-confirm_changes () {
- local dev x part partitions num id size type fs path name filesystem
partitems items
- # Compute the changes we are going to do
- partitems=''
- items=''
- for dev in $DEVICES/*; do
- [ -d "$dev" ] || continue
- cd $dev
+confirm_changes "partman-crypto" || exit 0
- open_dialog IS_CHANGED
- read_line x
- close_dialog
- if [ "$x" = yes ]; then
- partitems="${partitems} $(humandev $(cat device))
-"
- fi
-
- partitions=
- open_dialog PARTITIONS
- while { read_line num id size type fs path name; [ "$id" ]; }; do
- [ "$fs" != free ] || continue
- partitions="$partitions $id,$num"
- done
- close_dialog
-
- for part in $partitions; do
- id=${part%,*}
- num=${part#*,}
- [ -f $id/method -a -f $id/format \
- -a -f $id/visual_filesystem ] || continue
- [ -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method ] || continue
- [ ! -f $id/filesystem -o ! -f $id/formatted \
- -o $id/formatted -ot $id/method \
- -o $id/formatted -ot $id/filesystem ] || continue
- filesystem=$(cat $id/visual_filesystem)
- db_subst partman/text/confirm_item TYPE "$filesystem"
- db_subst partman/text/confirm_item PARTITION "$num"
- db_subst partman/text/confirm_item DEVICE $(humandev $(cat device))
- db_metaget partman/text/confirm_item description
-
- items="${items} ${RET}
-"
- done
- done
-
- if [ "$items" ]; then
- db_metaget partman/text/confirm_item_header description
- items="$RET
-$items"
- fi
-
- if [ "$partitems" ]; then
- db_metaget partman/text/confirm_partitem_header description
- partitems="$RET
-$partitems"
- fi
-
- if [ "$partitems$items" ]; then
- if [ -z "$items" ]; then
- x="$partitems"
- elif [ -z "$partitems" ]; then
- x="$items"
- else
- x="$partitems
-$items"
- fi
- db_subst partman-crypto/confirm ITEMS "$x"
- db_input critical partman-crypto/confirm
- db_go || true
- db_get partman-crypto/confirm
- if [ "$RET" = false ]; then
- db_reset partman-crypto/confirm
- return 1
- else
- db_reset partman-crypto/confirm
- return 0
- fi
- else
- db_input critical partman-crypto/confirm_nochanges
- db_go || true
- db_get partman-crypto/confirm_nochanges
- if [ "$RET" = false ]; then
- db_reset partman-crypto/confirm_nochanges
- return 1
- else
- db_reset partman-crypto/confirm_nochanges
- return 0
- fi
- fi
-}
-
-confirm_changes || exit 0
-
-# Commit the changes
crypto_setup yes || exit 1
