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)
Tracking
(firefox45 fixed, b2g-v2.5 fixed)
RESOLVED
FIXED
FxOS-S11 (13Nov)
People
(Reporter: julienw, Unassigned)
Details
Attachments
(4 files, 15 obsolete files)
126 bytes,
text/plain
|
mwu
:
review+
|
Details |
6.57 KB,
patch
|
fabrice
:
review+
mpotharaju
:
approval-gaia-v2.5+
|
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.
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.
v2 WIP. trying to get the variables from .config to load.
Attachment #8685063 -
Attachment is obsolete: true
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)
Comment 7•9 years ago
|
||
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...
Attachment #8685170 -
Flags: review?(fabrice)
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...
Comment 11•9 years ago
|
||
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)
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.
Comment 13•9 years ago
|
||
Cygwin is probably too heavy for our purposes. Other bash ports are better.
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
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 #8685123 -
Attachment is obsolete: true
Attachment #8685131 -
Attachment is obsolete: true
Attachment #8685135 -
Attachment is obsolete: true
Attachment #8685135 -
Flags: review?(fabrice)
Attachment #8685222 -
Attachment is obsolete: true
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)
Attachment #8685289 -
Flags: review?(mwu)
Attachment #8685289 -
Flags: review?(fabrice)
Still need to fix issue with the flash.bat
Comment 20•9 years ago
|
||
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.
Attachment #8685292 -
Flags: review?(mwu)
Attachment #8685292 -
Flags: review?(fabrice)
Comment 21•9 years ago
|
||
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+
platform prebuilds push: https://github.com/mozilla-b2g/platform_prebuilts_misc/commit/c739cffa6394c06e099ea48879a20341b6163338 https://github.com/mozilla-b2g/platform_prebuilts_misc/commit/179d485578c6907c0daf343a3bd7bc53b5e137a1
Attachment #8685292 -
Flags: review?(fabrice)
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)
Attachment #8685281 -
Attachment is obsolete: true
Attachment #8685285 -
Attachment is obsolete: true
Reporter | ||
Comment 24•9 years ago
|
||
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 26•9 years ago
|
||
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 27•9 years ago
|
||
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 30•9 years ago
|
||
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-
Running patch on TC : https://tools.taskcluster.net/task-inspector/#NrRpkaoIQvCdRdbGdv3fYw/0
Comment 32•9 years ago
|
||
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/
Comment 34•9 years ago
|
||
Mentioned offline, but we'll need to update the MDN article for Nexus 4 (https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Phone_guide/Nexus_4) and Nexus 5 (https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Phone_guide/Nexus_5).
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)
Updated•9 years ago
|
Attachment #8686148 -
Flags: review?(fabrice) → review+
Comment 39•9 years ago
|
||
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)
Updated•9 years ago
|
Attachment #8686172 -
Flags: review?(fabrice) → review+
Updated•9 years ago
|
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 43•9 years ago
|
||
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+
Comment 44•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/61ffc88ffff4
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S11 (13Nov)
rebase for 2.5
Comment 46•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/958407d2f351
status-b2g-v2.5:
--- → fixed
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 → ---
Updated•9 years ago
|
Attachment #8689276 -
Flags: review?(fabrice) → review+
Updated•9 years ago
|
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
Comment 50•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/7ceb9e1bed7c
Keywords: checkin-needed
Comment 51•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7ceb9e1bed7c
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•