[pkg-fso-maint] [PATCH] install.sh: add $SD_PART1_SIZE and $SD_SWAP_SIZE user variables (was Re: install.sh - 4 proposals for patches)

[Luca Capello]

Hi Steffen!

Finally, the last patch for today :-D

On Fri, 13 Feb 2009 02:47:31 +0100, Steffen Moeller wrote:
>>From 6834aca5e0a66ab795daae472356ccc3c2b5edae Mon Sep 17 00:00:00 2001
> From: Steffen Moeller <steffen_moeller at gmx.de>
> Date: Fri, 13 Feb 2009 02:38:37 +0100
> Subject: [PATCH] Added formatting of vfat systems, prepared swap partition.

These two issues are completely different: you should provide a first
patch for $SD_PART1_SIZE, then a second one for $SD_SWAP +
$SD_SWAP_SIZE.  However, my concerns...


1) why would one like a bigger /boot partition?

   * please always think about the ordinary user, i.e. someone who
     installs Debian on the Openmoko without being an expert GNU/Linux
     user

   * there is no automatic way to switch between various U-Boot kernel
     images (the reason for a bigger /boot partition), thus if you know
     how to deal with symlinks and know what you are doing, then you
     also know how to manually create a bigger /boot partition since the
     beginning (this is why install.sh is separated into self-contained
     stages)


2) why do you need a swap partition?  I have not followed the discussion
   on this matter, so providing link to constructive threads is enough

>  # run partitioner
> -if [ "$SD_PART1_FS" = "vfat" ]; then
> +rootpartno=2
> +bootfstypeno=83

Please, use uppercase variables.

> +if [ "vfat" = "$SD_PART1_FS" ]; then

Semantically, this test (and some following ones) order is wrong.

> +	bootfstypeno=4
[...]
> +EOBOOTCONF
[...]
> +EOSWAPCONF
[...]
> +EOROOTCONF

Are telling me that reading this shell script is linear?  ;-)

> +	if fdisk $SD_DEVICE < /tmp/argsToFdisk$$; then
> +		echo "Partitioning was successful."
> +		#rm /tmp/argsToFdisk$$
> +	else
> +		echo "Partitioning failed, could not execute with fdisk:"
> +		cat /tmp/argsToFdisk$$
> +		exit -1
> +	fi

I am not sure if this is clearer: I would prefer the previous way of
partitioning, because it is easier to repeat manually (you just follow
the order the commands are given).

>  echo " * Installing /root/.xsession"
> -case $WINDOW_MANAGER in
> -fbpanel) 
[...]
> +EOZHONE
> +
> +fi

This part has nothing to do with partitioning and should go into a
separate patch.

Thx, bye,
Gismo / Luca
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 314 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-fso-maint/attachments/20090218/87a4a046/attachment.pgp