1242390 - Support flash.sh for RPi2 device

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[:gerard-majax]

The RPi2 will not be flashable with Fastboot for full partitions. We should land a device-specific flash.sh script that is able to dump raw partitions.

Comment 1

[Giovanny Gongora [:gioyik]]

Attachment: Github PR

Comment 2

[:gerard-majax]

Comment on attachment 8721882 [details] [review]
Github PR

I don't like this approach, we should do as for RPi and land a device specific flash.sh ; this device is way too different compared to a phone.

I left some comments on Github also.

Comment 3

[Giovanny Gongora [:gioyik]]

Attachment: Github PR

Comment 4

[Juan Gomez [:_AtilA_] (CET/CEST)]

Comment on attachment 8724650 [details] [review]
Github PR

Hi Giovanny,
you should address all comments Alex did in the previous PR.
* The script should behave like all other flash.sh scripts, we must have targets to flash specific images (e.g: ./flash.sh system). Add another target: "partition" which will only make the partitions in the sd-card.
* Using "sudo" inside the script is a bad practice, please remove them all. We should let the users to use sudo if they wanted to, when invoking the script.
* mkfs is not really needed, because the images have filesystems inside, so what we need to do is resize the filesystems once they are dumped. resize2fs tool will help to do that.

Looking forward for the next review ;)

Comment 5

[:gerard-majax]

(In reply to Juan Gomez [:_AtilA_] (CET/CEST) from comment #4)
> Comment on attachment 8724650 [details] [review]
> Github PR
> 
> Hi Giovanny,
> you should address all comments Alex did in the previous PR.
> * The script should behave like all other flash.sh scripts, we must have
> targets to flash specific images (e.g: ./flash.sh system). Add another
> target: "partition" which will only make the partitions in the sd-card.
> * Using "sudo" inside the script is a bad practice, please remove them all.
> We should let the users to use sudo if they wanted to, when invoking the
> script.
> * mkfs is not really needed, because the images have filesystems inside, so
> what we need to do is resize the filesystems once they are dumped. resize2fs
> tool will help to do that.
> 
> Looking forward for the next review ;)

I would like to add:
 - we should try to use bmaptool if it's available, since it will be faster to dump images
 - we should have support for detecting a sdcard contains the proper partition layout with correct sizes, and abort if not

Comment 6

[:gerard-majax]

Attachment: bmaptool improvements

Comment 7

[Giovanny Gongora [:gioyik]]

Ok, thanks for the comments. The flash script should look like the one you are waiting for. Let me know what needs to be changed or is missing.

Comment 8

[:gerard-majax]

It's getting better but there is still some work to be done :)

Comment 9

[:gerard-majax]

This script works? There is way too much problems for this to work after reading it :)

Comment 10

[Giovanny Gongora [:gioyik]]

I did some modifications and commented others in Github. The unique thing left is use bmaptool. I didn't found how to compile or get it for Mac. I saw in the website packages for Linux distributions only. Do you know how to make it work in Mac OS X?

Comment 11

[:gerard-majax]

I already said several times I don't care/have an opinion regarding OSX. Just make sure that people with bmaptool available can benefit from it. I have no idea if it can even works on OSX.

Comment 12

[:gerard-majax]

(In reply to Giovanny Gongora [:gioyik] from comment #10)
> I did some modifications and commented others in Github. The unique thing
> left is use bmaptool. I didn't found how to compile or get it for Mac. I saw
> in the website packages for Linux distributions only. Do you know how to
> make it work in Mac OS X?

I replied to your comments. Please, next time, needinfo on the bug when you reply on github.

Comment 13

[Giovanny Gongora [:gioyik]]

Sorry, but I understood the bmaptool change as something useful for everybody (all the platforms), that's why I agreed with you to make it a priority for the flash script.

> I don't care/have an opinion regarding OSX

^ I am sorry to read this because Juan and I started to think in a flash script to be available for every platform. I care about Mac OS X because I and the other people who is reviewing my work too is using Mac OS X, if the implementation of the bmaptool only makes a benefit for one platform, for me it's implementation has less priority for this PR to be landed.

I am not saying bmaptool is not necessary or not useful, but I am seeing more things to be fixed first and that are critical to have a working flash script. I will make the rest of the changes you mentioned on Github.

Thanks

Comment 14

[:gerard-majax]

(In reply to Giovanny Gongora [:gioyik] from comment #13)
> Sorry, but I understood the bmaptool change as something useful for
> everybody (all the platforms), that's why I agreed with you to make it a
> priority for the flash script.

Yes.

> 
> > I don't care/have an opinion regarding OSX
> 
> ^ I am sorry to read this because Juan and I started to think in a flash
> script to be available for every platform. I care about Mac OS X because I
> and the other people who is reviewing my work too is using Mac OS X, if the
> implementation of the bmaptool only makes a benefit for one platform, for me
> it's implementation has less priority for this PR to be landed.

No, you are not getting the point: for platforms where bmaptool is available, let's make use of it.
When it is not available, fallback to dd.

If bmaptool does not trivially support OSX, it's pointless you do the work to make it working there.

> 
> I am not saying bmaptool is not necessary or not useful, but I am seeing
> more things to be fixed first and that are critical to have a working flash
> script. I will make the rest of the changes you mentioned on Github.

It's easy to integrate it. We should land the flashing script with it because it is really simple to handle both tools.

Comment 15

[Giovanny Gongora [:gioyik]]

I did changes on the script. Please, take your time to review it again. I know there are nit indentation things to solve I will fix all of them after get a green light to land.

Comment 16

[:gerard-majax]

There is still fixes to do. See comments on Github, thanks!