Closed Bug 1221537 Opened 9 years ago Closed 9 years ago

Provide a flash.bat script for Windows to device image builds (especially aries)

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox45 fixed, b2g-v2.5 fixed)

RESOLVED FIXED
FxOS-S11 (13Nov)
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: julienw, Unassigned)

Details

Attachments

(4 files, 15 obsolete files)

126 bytes, text/plain
mwu
: review+
Details
6.57 KB, patch
fabrice
: review+
Details | Diff | Splinter Review
6.71 KB, patch
Details | Diff | Splinter Review
2.63 KB, patch
fabrice
: review+
Details | Diff | Splinter Review
      No description provided.
Attached file flash.bat (obsolete) —
First draft of the windows batch file.
We still require a adb and fastboot to be pre-installed on the machine with this version.  Also the batch file has to be placed in the out/target/product/<device>/ folder and the end user has to go to that directory in order to install.
note: aries requires a fastboot driver and windows driver.

There's a chance that other devices might require two different drivers.
Attached file flash.bat (obsolete) —
v2 WIP.  trying to get the variables from .config to load.
Attachment #8685063 - Attachment is obsolete: true
Attached file win_tools.zip (obsolete) —
packaging adb and fastboot with the batch file.
windows adb and windows fastboot along with the batch file packaged.

I will need to make another PR for adding these files into the build configs for the devices.
Attachment #8685135 - Flags: review?(fabrice)
I don't think it makes sense to put it in this particular repo though. I'd also somewhat prefer to use a port of bash rather than having two different versions of the flashing script.
Which repo should it go in?  Also I would prefer to make a port of the flash.sh as well, at the same time our time constraint for getting this done is 11 am tomorrow.  Apparently coming down from Mitchell.

If you're asking me to port now, I don't have time to do that.  There are other blockers for QA including no FOTA/OTA builds for 2.5 and issues with the current master/moz-central dogfood builds FOTA/OTA.
Flags: needinfo?(mwu)
WIP patch; need to know where the tools go in order to push it in the zip files...
Maybe the question is : If it's in it's own repo, how do I pull it from that repo into the build output so that it's at the root?  I'm not sure how to do that and I would need help.

Also shouldn't the flash script in itself be in it's own repo as well then?  Seems like we're upping the level of complexity in which case the probability of breaking will be higher...
If you need to land it ASAP, land it in the repo that handles generating packages (that is, mozilla-central). You'll need to add the path to the batch script to ${GECKO_ROOT}/b2g/config/*/config.json anyway.

Using a port of bash is likely to be quicker and less error prone than creating a partial batch file port of flash.sh. Google for windows bash - there's a few ports, though I don't know which ones are necessarily usable by themselves. msys comes with bash, and so does git for windows, though I have no idea if any come in a standalone binary.
Flags: needinfo?(mwu)
Attached file win_tools_v1.zip (obsolete) —
As far as I recall, we had issues with cygwin running the flash script.

Fabrice suggested that I create my own download/flash package outside the repo instead.
Cygwin is probably too heavy for our purposes. Other bash ports are better.
Attached file win_tools_v1a.zip (obsolete) —
revision.  still work in progress.
Attachment #8685210 - Attachment is obsolete: true
After talking to mwu, we decided there's much more breaks trying to make this tool than place it in the build; Need to make a patch to place my tools accordingly : 

mwu_: https://github.com/mozilla-b2g/platform_prebuilts_misc/tree/b2g-4.4.2_r1 might be a good location to land windows binaries
mwu_: that branch or https://github.com/mozilla-b2g/platform_prebuilts_misc/tree/b2g-5.1.0_r1 depending on which builds we're using
Attached file flash.bat (obsolete) —
I'm stuck on checking the error for fastboot.  For some reason it's not behaving correctly.  I think I'm overlooking something...
Flags: needinfo?(mwu)
Attachment #8685135 - Attachment is obsolete: true
Attachment #8685135 - Flags: review?(fabrice)
Attached file win_flash_tools.zip (obsolete) —
back to condensing down to the batch file to flash and the adb/fastboot executables.
Attachment #8685170 - Attachment is obsolete: true
Attachment #8685170 - Flags: review?(fabrice)
Still need to fix issue with the flash.bat
Comment on attachment 8685289 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

Review of attachment 8685289 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/config/aries/config.json
@@ +23,5 @@
>      ],
>      "zip_files": [
>          ["{workdir}/out/target/product/aries/*.img", "out/target/product/aries/"],
>          "{workdir}/flash.sh",
> +        "{workdir}/prebuilts/misc/windows/win_flash_tools/*",

I meant for only the adb/fastboot binaries to be stored here, and then the flash batch file itself is stored in the gecko repo.
Comment on attachment 8685292 [details]
github patches for platform_prebuilts_misc

I think one PR per attachment is more useful. Otherwise bugzilla doesn't do integration properly.
Attachment #8685292 - Flags: review?(mwu) → review+
Spoke with mwu.  Made the changes he asked for, and placed the flash.bat file in the b2g/installer.

He asked me to sort the config + flash.bat location with Fabrice.
Attachment #8685309 - Flags: review?(fabrice)
Attachment #8685289 - Attachment is obsolete: true
Attachment #8685289 - Flags: review?(mwu)
Attachment #8685289 - Flags: review?(fabrice)
I tried a bash port (I think from http://win-bash.sourceforge.net/ but not sure) but I couldn't really make it work. I found since than that it uses a very old version of Bash.

Other options if we want to pursue the shell port path:

* https://github.com/oldfaber/wzsh, a ZSH port for Windows
* http://zsh-nt.sourceforge.net/index.html another port for ZSH


Also I see our current flash.sh is quite complex: it tries to find the device we're flashing for and decides whether we should fullflash or shallowflash by default (and in addition it provides arguments to force one mode or the other). How about providing a different script for each device instead ? Chances are that most of the scripts would then be identical in .bat and .sh forms...
Julien, thanks.  :)

I think I'm just going to try to push the bat file that I created into the build structure.  Pending Fabrice's review.  I managed to get a hold of mwu last night via irc and we talked a lot of the batch file and stuff through.

(I had pinged him on SMS on his phone late night... sorry, mwu, for the late night message!)
Flags: needinfo?(mwu)
Comment on attachment 8685309 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

Review of attachment 8685309 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok overall, but I haven't tested. Naoki, do you have try build for one of the devices?

::: b2g/installer/flash.bat
@@ +27,5 @@
> +win_adb get-state > devicestate.txt
> +set /p DEVICE_STATE= < devicestate.txt
> +
> +IF NOT "%DEVICE_STATE%"=="device" (
> +   ECHO Please check : 

nit: trailing whitespace

@@ +40,5 @@
> +
> +Del devicestate.txt
> +win_adb reboot bootloader
> +
> +TIMEOUT 5

why do we need a timeout there? At least on linux, |fastboot devices| will block until the device is in fastboot mode.

@@ +47,5 @@
> +win_fastboot devices 2> fastboot_state.txt
> +set /p FASTBOOT_STATE= < fastboot_state.txt
> +
> +IF NOT [%FASTBOOT_STATE%]==[] (
> +   ECHO Please check : 

nit: trailing whitespace

@@ +61,5 @@
> +
> +ECHO "Flashing build. If nothing mentions that it flashed anything and it looks stuck, make sure you have the drivers installed."
> +
> +win_fastboot flash boot out/target/product/%PRODUCT_NAME%/boot.img
> +

please remove all these blank lines

@@ +83,5 @@
> +
> +echo "Just close the windows as you wish."
> +
> +
> +TIMEOUT 5
\ No newline at end of file

why?
Attachment #8685309 - Flags: review?(fabrice)
Comment on attachment 8685309 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

Review of attachment 8685309 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok overall, but I haven't tested. Naoki, do you have try build for one of the devices?

::: b2g/installer/flash.bat
@@ +27,5 @@
> +win_adb get-state > devicestate.txt
> +set /p DEVICE_STATE= < devicestate.txt
> +
> +IF NOT "%DEVICE_STATE%"=="device" (
> +   ECHO Please check : 

nit: trailing whitespace

@@ +40,5 @@
> +
> +Del devicestate.txt
> +win_adb reboot bootloader
> +
> +TIMEOUT 5

why do we need a timeout there? At least on linux, |fastboot devices| will block until the device is in fastboot mode.

@@ +47,5 @@
> +win_fastboot devices 2> fastboot_state.txt
> +set /p FASTBOOT_STATE= < fastboot_state.txt
> +
> +IF NOT [%FASTBOOT_STATE%]==[] (
> +   ECHO Please check : 

nit: trailing whitespace

@@ +61,5 @@
> +
> +ECHO "Flashing build. If nothing mentions that it flashed anything and it looks stuck, make sure you have the drivers installed."
> +
> +win_fastboot flash boot out/target/product/%PRODUCT_NAME%/boot.img
> +

please remove all these blank lines

@@ +83,5 @@
> +
> +echo "Just close the windows as you wish."
> +
> +
> +TIMEOUT 5
\ No newline at end of file

why?
Made changes for nit + removal of the one line.

Double clicking the batch file will have things run and shut down so I left the last TIMEOUT statement in so people can read what happened before they close it out.
Attachment #8685309 - Attachment is obsolete: true
Fixed another white space issue.
Attachment #8685534 - Attachment is obsolete: true
Comment on attachment 8685538 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

Review of attachment 8685538 [details] [diff] [review]:
-----------------------------------------------------------------

We really need an end-to-end test before landing.

::: b2g/config/nexus-5-l/config.json
@@ +23,5 @@
>      "zip_files": [
>          ["{workdir}/out/target/product/hammerhead/*.img", "out/target/product/hammerhead/"],
>          ["{workdir}/boot.img", "out/target/product/hammerhead/"],
>          "{workdir}/flash.sh",
> +        "{workdir}/b2g/installer/flash.bat",

I think that should be {workdir}/gecko/b2g/installer/flash.bat
Attachment #8685538 - Flags: review-
Did you update with what's in comment 30 before pushing to TC?
I forgot to commit my git repo with this change....
The build failed for a different reason.  Not sure why.

Giving another go with aries config being changed with what's in comment 30.
https://tools.taskcluster.net/task-inspector/#E-jtu-gySsa8cAHbbtdPPg/
needed to update the sources.xml files for the binary bits for windows...
Attachment #8685538 - Attachment is obsolete: true
Comment on attachment 8685538 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

I updated the sources file to include the push that I made for the binaries.
Attachment #8685538 - Flags: review- → review?(fabrice)
white space fix.
Attachment #8685791 - Attachment is obsolete: true
Attachment #8686148 - Flags: review?(fabrice) → review+
Comment on attachment 8686148 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

Review of attachment 8686148 [details] [diff] [review]:
-----------------------------------------------------------------

Actually the sources.xml changes can't be done there.
Attachment #8686148 - Flags: review+
Removed sources.xml updates.  
They would have to be done somewhere else?  Where do I have to update them?
Attachment #8686148 - Attachment is obsolete: true
Attachment #8686172 - Flags: review?(fabrice)
Attachment #8686172 - Flags: review?(fabrice) → review+
Attachment #8685538 - Flags: review?(fabrice)
QA Contact: nhirata.bugzilla
Comment on attachment 8686172 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

No windows batch file for 2.5 builds unless this lands in 2.5 as well.
Attachment #8686172 - Flags: approval-gaia-v2.5?
Flags: needinfo?(mpotharaju)
Comment on attachment 8686172 [details] [diff] [review]
bug_1221537_flash_for_windows.patch

Approved for 2.5 Uplift. 

Thanks for the NI, Naoki
Flags: needinfo?(mpotharaju)
Attachment #8686172 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+
https://hg.mozilla.org/mozilla-central/rev/61ffc88ffff4
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S11 (13Nov)
Fabrice, the windows batch file itself wasn't pushed for some reason to MC and b2g-inbound.  Here's an b2g-inbound patch.
Flags: needinfo?(fabrice)
Attachment #8689276 - Flags: review?(fabrice)
reopened due to missing parts of the check in...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8689276 - Flags: review?(fabrice) → review+
Flags: needinfo?(fabrice)
Check-in of patch in comment 47 needed in b2g-inbound. 

 The other parts have been checked in.  (both for 2.5 and mc)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7ceb9e1bed7c
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: