Closed
Bug 1257127
Opened 9 years ago
Closed 9 years ago
Build failure in treeherder because Docker images need a gcc-4.8 host compiler version or newer
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(firefox48 fixed)
RESOLVED
FIXED
2.6 S11 - 4/8
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: _AtilA_, Assigned: _AtilA_)
References
Details
(Whiteboard: [ft:conndevices] fixed-in-pine)
Attachments
(4 files, 3 obsolete files)
(deleted),
patch
|
garndt
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
wcosta
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
wcosta
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/x-review-board-request
|
glandium
:
review+
|
Details |
Minimun host compiler version has been bumped to gcc-4.8, but the docker images that perform builds on the taskcluster for b2g still use gcc-4.7.3. We need to upgrade this version to at least gcc-4.8. In order to achive it, we need to:
* Create a gcc-4.8 RPM package for Centos 6 (x86_64) with our needed structure and upload it to http://mockbuild-repos.pub.build.mozilla.org/releng/public/CentOS/6/x86_64/
* Modify https://dxr.mozilla.org/mozilla-central/source/testing/docker/b2g-build/Dockerfile to install this new package and update the paths to point to our new package installation
Updated•9 years ago
|
Whiteboard: [ft:conndevices]
Assignee | ||
Comment 1•9 years ago
|
||
So this is what I changed in this patch:
* b2g-build/Dockerfile was modified to add a new path to PATH environment variable to point to the host compiler that will be downloaded later in the install-package.sh script, and that will be used to compile some parts of Gecko.
* get-pip.py URL has been updated because the old one is deprecated.
* Bumped VERSION number to 0.2.10
* We still need to link "gcc" binary to "cc", but this must be done right after tooltool is invoked and downloads the final host compiler used to build some parts of Gecko.
What I've found out so far, is that we need to upload the docker image created with ./build.sh b2g-build with the tag: 0.2.10. Don't know if I should do this.
All my local tests worked fine.
Attachment #8732647 -
Flags: review?(garndt)
Comment 2•9 years ago
|
||
Comment on attachment 8732647 [details] [diff] [review]
0001-Bug-1257127-Change-b2g-Docker-images-and-scripts-to-.patch
Review of attachment 8732647 [details] [diff] [review]:
-----------------------------------------------------------------
I think this is ok, but I don't know much about the hacks for getting gcc to work that are done here. Once this lands I can pull down the change and push the new image to our docker registry. This will temporarily break the existing builds until this image is pushed.
::: testing/docker/builder/Dockerfile
@@ -1,1 @@
> -FROM quay.io/mozilla/b2g-build:0.2.9
So the b2g-build image needs to be built and pushed (I can do that) before landing these changes. However, since these builds are tier 3, having them broke for one push (jobs can be retriggered though) is ok because it won't cause the trees to close.
Updated•9 years ago
|
Attachment #8732647 -
Flags: review?(garndt) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
This'll probably need to make its way to b2g-inbound at some point:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb7f6fbe3ba
Keywords: checkin-needed
This landed, image builds started failing. Garndt pushed something somewhere, retriggered jobs came back green.
Assignee | ||
Comment 5•9 years ago
|
||
Ok, Thanks Wesley and Greg!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 6•9 years ago
|
||
bugherder |
Assignee | ||
Comment 7•9 years ago
|
||
Device docker images works in a totally different way, so I've reopened the bug to fix the device images as well.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 8•9 years ago
|
||
This is what I changed in this patch, and why:
* The backup-aries.tar.xz file which is uploaded to our tooltool server was never being used, because it requires a 'get-blobs' step in the b2g_build.py script that was not being executed, so I removed all references to it. The backup is being downloaded and unpacked by a pre-build.sh script [1]
* testing/docker/phone-builder/Dockerfile now inherits from the correct image:tag => taskcluster/builder:0.5.12, this images was changed because of the previous patch in this very bug, :grandt have more details about it.
* For the same reason testing/docker/builder/VERSION has been bumped to 0.5.12, this image has been already uploaded to Docker Hub
* testing/mozharness/configs/b2g/taskcluster-phone* now added a new step to download the host toolchain ('get-blobs') and a new config to download tooltool tool (xDD) so we can call it later in the b2g_build.py script.
* testing/mozharness/scripts/b2g_build.py has been modified because we need to download and untar the gcc.tar.xz file downloaded by tooltool into the gecko src directory (/home/worker/workspace/gecko/gcc). This is needed because in my previous patch, I had to add this directory to the PATH environment variable so the system can use it to build Gecko later with the proper toolchain.
I tried this changes locally, but in an isolated way because the nature of the system and time constraints didn't allow me to do it in another way.
As this is for a tier3 platform, let's land it and if something goes wrong we can try to fix it asap, instead of backing out.
[1] https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/phone-builder/pre-build.sh#39
Attachment #8736253 -
Flags: review?(wcosta)
Updated•9 years ago
|
Attachment #8736253 -
Flags: review?(wcosta) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Assignee | ||
Comment 10•9 years ago
|
||
Attachment #8736360 -
Flags: review?(wcosta)
Updated•9 years ago
|
Attachment #8736360 -
Flags: review?(wcosta) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 11•9 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox48:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.6 S11 - 4/8
Assignee | ||
Updated•9 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•9 years ago
|
||
We need to land the third patch (0001-Bug-1257127-Bump-phone-builder-VERSION-0.0.25-r-wcos.patch), it was not landed before the first merge with m-i.
Keywords: checkin-needed
Comment 13•9 years ago
|
||
Keywords: checkin-needed
Comment 14•9 years ago
|
||
Whiteboard: [ft:conndevices] → [ft:conndevices] fixed-in-pine
Comment 15•9 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•9 years ago
|
||
So, looks like we need to link "gcc" command line to "cc", because the build system uses "cc".
I repacked gcc.tar.xz so it contains the link to cc on it, that means that we don't need to change any line of code (just a config file to update the SHa512)
:dusting is uploading the repacked package to relengapi servers.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 17•9 years ago
|
||
Attachment #8737784 -
Flags: review?(dustin)
Comment 18•9 years ago
|
||
Comment on attachment 8737784 [details] [diff] [review]
Update b2g manfiest files for tooltool to support new gcc toolchain
I can attest that I got an email from Juan pointing to a file with that sha512, and uploaded it to tooltool. I can't say much more for its provenance, nor do I have any idea whether it will fix the issue here.
Subject: =?UTF-8?q?Bug=201257127=20-=20Update=20b2g=20manifest=20config=20?=
=?UTF-8?q?files=20for=20tooltool=20to=20support=0Aupdated=20gcc=20host=20?=
=?UTF-8?q?toolchain=20r=3Ddustin?=
is not the prettiest commit message ;)
Attachment #8737784 -
Flags: review?(dustin) → review+
Assignee | ||
Comment 19•9 years ago
|
||
Wupss!! Yeah, that's pretty at all. This is far way better...
BTW, What I have done with the package is just:
1º - Download original gcc.tar.xz from https://api.pub.build.mozilla.org/tooltool/
2º - Uncompress & Untar
3º - Link "gcc" to "cc": $ ln -s gcc cc
4º - Compress and tar
Anyway, I'd like to run a try-job first to make sure that this change doesn't break anything.
Attachment #8737784 -
Attachment is obsolete: true
Assignee | ||
Comment 20•9 years ago
|
||
Assignee | ||
Comment 21•9 years ago
|
||
Assignee | ||
Comment 22•9 years ago
|
||
Assignee | ||
Comment 23•9 years ago
|
||
Assignee | ||
Comment 24•9 years ago
|
||
Added a runtime check to see if "cc" already exists in the package.
Attachment #8737929 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 25•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9677930b4b46
Got a green in try.
There only patch needed to land is the last one (fourth).
Comment 26•9 years ago
|
||
(In reply to Juan Gomez [:_AtilA_] (CET/CEST) from comment #1)
> * We still need to link "gcc" binary to "cc", but this must be done right
> after tooltool is invoked and downloads the final host compiler used to
> build some parts of Gecko.
No you don't need to link "gcc" to "cc". Which also means you don't need any change to the docker images. What you need is to set HOST_CC and HOST_CXX (which, btw, is set in b2g/config/mozconfigs/common, but maybe the tooltool directory changed or something), or wait for bug 1259382.
I'd go as far as say you should backout all the changes from this bug.
Comment 27•9 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #26)
> (In reply to Juan Gomez [:_AtilA_] (CET/CEST) from comment #1)
> > * We still need to link "gcc" binary to "cc", but this must be done right
> > after tooltool is invoked and downloads the final host compiler used to
> > build some parts of Gecko.
>
> No you don't need to link "gcc" to "cc". Which also means you don't need any
> change to the docker images.
Or to the tooltool packages, for that matter.
Assignee | ||
Comment 28•9 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #26)
> (In reply to Juan Gomez [:_AtilA_] (CET/CEST) from comment #1)
> > * We still need to link "gcc" binary to "cc", but this must be done right
> > after tooltool is invoked and downloads the final host compiler used to
> > build some parts of Gecko.
>
> No you don't need to link "gcc" to "cc". Which also means you don't need any
> change to the docker images. What you need is to set HOST_CC and HOST_CXX
> (which, btw, is set in b2g/config/mozconfigs/common, but maybe the tooltool
> directory changed or something), or wait for bug 1259382.
>
> I'd go as far as say you should backout all the changes from this bug.
Well, if you take a look at the patches I've fixed some other stuff that still needs to be fixed, like the URL to download get-pip.py which will require a new Docker image to be generated and uploaded anyway.
Another thing that I've fixed as well is that phone-builder image was not using tooltool at all, so the gcc.tar.xz package was not being downloaded and the image was using the compiler previously installed in the system (gcc-4.7).
I didn't like the link to cc hack either, but this is what emulator images were doing [1] before, so I used the same technique to link with the downloaded gcc package. But then I realized that phone-builder images were building in a totally different way, so I decided to link cc in the package which will work for all b2g builds.
With all this in mind I wouldn't backout all the patches. I could change the HOST_CC/CXX to point to the gcc downloaded by tooltool and remove the PATH and cc links related changes, so basically:
* Remove GCC_PATH from the b2g-build Dockerfile.
* Remove the cc link in the install-package.sh script.
* Delete the gcc.tar.xz from RelengAPI (the older without the link is still there)
* Add HOST_CC/CXX in b2g/config/mozconfigs/common to point to the unpacked gcc package. (we can change that later once bug 1259382 has landed)
[1] https://dxr.mozilla.org/mozilla-central/rev/17a0ded9bb99c05c25729c306b91771483109067/testing/docker/b2g-build/Dockerfile#111
Keywords: checkin-needed
Comment 29•9 years ago
|
||
(In reply to Juan Gomez [:_AtilA_] (CET/CEST) from comment #28)
> * Add HOST_CC/CXX in b2g/config/mozconfigs/common to point to the unpacked
> gcc package. (we can change that later once bug 1259382 has landed)
Actually, you don't need to wait for bug 1259382. Bug 1259382 changes things such that the "cc" hack wouldn't work (it doesn't search for "cc" anymore). And it turns out that yes, none of the b2g image builds are using the HOST_CC path already in b2g/config/mozconfigs/common because it's not in $topsrcdir/../gcc, but in $topsrcdir/gcc, now.
Assignee | ||
Updated•9 years ago
|
Attachment #8738098 -
Attachment is obsolete: true
Assignee | ||
Comment 30•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44813/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44813/
Attachment #8738917 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 31•9 years ago
|
||
Comment on attachment 8738917 [details]
MozReview Request: Bug 1257127 - [B2G] Fix HOST_CC/CXX paths for local builds r?glandium
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44813/diff/1-2/
Comment 32•9 years ago
|
||
Comment on attachment 8738917 [details]
MozReview Request: Bug 1257127 - [B2G] Fix HOST_CC/CXX paths for local builds r?glandium
https://reviewboard.mozilla.org/r/44813/#review41685
Attachment #8738917 -
Flags: review?(mh+mozilla) → review+
Comment 33•9 years ago
|
||
Comment 34•9 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 35•9 years ago
|
||
Reopening, Aries debug builds needs to be fixed too.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 36•9 years ago
|
||
Ok, mozReview seems to not manage reopens properly, so I'll create a new bug for patching the Aries debug builds.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 37•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•