Re: r53484 - in trunk/packages/partman/partman-partitioning: debian free_space/new
On Tue, May 27, 2008 at 12:40:29PM +0200, Frans Pop wrote:
> On Tuesday 27 May 2008, Colin Watson wrote:
> > Author: cjwatson
> > Date: Tue May 27 09:17:38 2008
> > New Revision: 53484
> >
> > Log:
> > Move 'local' down a line in create_new_partition in order to work
> > properly with bash, which initialises local variables without an
> > accompanying assignment to empty rather than to any previous value.
> >
> > Modified:
> > trunk/packages/partman/partman-partitioning/debian/changelog
> > trunk/packages/partman/partman-partitioning/free_space/new/do_option
> [...]
> > create_new_partition () {
> > - local num id size type fs path name mp mplist mpcurrent numparts device
> > open_dialog NEW_PARTITION $type ext2 $freeid $place $size
> > + local num id size type fs path name mp mplist mpcurrent numparts device
> > id=''
> > read_line num id size type fs path name
> > close_dialog
>
> I don't get this. Why is this move needed?
It's not important in d-i, but I got a very confused user turning up on
IRC a while back who was trying to use ubiquity with bash as the default
/bin/sh, and the partitioner broke in very weird ways; I eventually
tracked it down to this.
If you declare a variable with 'local' that already has a value, bash
reinitialises it to empty, while busybox sh and dash leave it alone. In
fact, this difference is explicitly mentioned in recent versions of
policy (section 10.4).
> It looks wrong to me as now some vars are being defined as local after they
> are first used.
No, that isn't the case. ask_for_type sets the 'type' variable which is
used at the top of create_new_partition. create_new_partition then
reuses type for something entirely different (and localises it in the
process).
> Should the local line maybe be split into two lines instead?
Probably wouldn't hurt to rename the type variable in
create_new_partition, but there's nothing actually incorrect about my
change as far as I can see.
Cheers,
--
Colin Watson [cjwatson@debian.org]
Reply to: