[pkg-fso-maint] install.sh: OpenMoko's Wiki mentions a problem

[Luca Capello]

Hi Steffen!

Replying to the list, as we decided in private ;-)

On Tue, 10 Feb 2009 14:45:56 +0100, Steffen Moeller wrote:
> Luca Capello wrote:
>> On Sat, 07 Feb 2009 00:02:36 +0100, Steffen Moeller wrote:
>>>  echo " * Waiting for partitions to appear"
>>>  SLEEP_SECONDS=3
>>> -while ! ([ "$SINGLE_PART" = "true" ] && test -e ${SD_DEVICE}p1) ||\
>>> -	test -e ${SD_DEVICE}p1 -a -e ${SD_DEVICE}p2; do
>>> -	if [ "$SLEEP_SECONDS" = 15 ]; then
>>> +while ! (([ "$SINGLE_PART" = "true" ] && test -e ${SD_DEVICE}p1) ||\
>>> +	test -e ${SD_DEVICE}p1 -a -e ${SD_DEVICE}p2); do
>>> +	if [ "$SLEEP_SECONDS" = 30 ]; then
>>>  		echo "E: Partitioning has failed, partitions have not been created"
>>>  		exit 1
>> 
>> The original code above is outdated [1] and the diff introduces at the
>> same time two changes which have different meanings: the first one, like
>> you stated at the end of your mail, is clearly correct [2]; the second
>> one just adds more time to the check.  Is this really needed?
>
> Is it truely more time to check only?

I think my words above are not so clear, so let me try to better
explain.  The patch above corrects at the same time two issues:


1) the test, i.e.

   +while ! (([ "$SINGLE_PART" = "true" ] && test -e ${SD_DEVICE}p1) ||\
   +	test -e ${SD_DEVICE}p1 -a -e ${SD_DEVICE}p2); do

> It took me a while to parse. Even if the two forms may be identical in
> their semantics, I agree to the patch that the parenthese are helping
> the expressiveness .... not everyone is an expert in operator
> precendences.
>
> original:  ! ( a && b ) || (b && c)
> patch:  ! (( a && b ) || (b && c))
>
> I recall that ! is stronger than ||, but I may be completely wrong for
> the bash. The extra ()s help ... or are wrong :)

If you remove the extra ()s, the ! acts only on the first test and not
in general.

Anyway, the new test is clearly better and more readable :-)


2) more seconds for the while test, i.e. more seconds waiting for the
   partition to appear

   +	if [ "$SLEEP_SECONDS" = 30 ]; then

My question was about this modification: I do not think it is worth
waiting so long, because I would say that there is a problem if this
happens at all.

>> Steffen, can you please remove the whole part?
>
> I have removed the install.sh part, in complete confidence that you
> investigate the  () issue.
[...]
>> I have now discovered, on the same page, the other two problems:
[...]
> I added a ref to [4] for the bug reporting and left the remainder as
> it was.

Thank you!

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