Closed Bug 1056337 Opened 6 years ago Closed 5 years ago

Investigate upgrading the toolchain used for B2G ICS builds (and stop using GCC 4.4)

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.2 fixed)

RESOLVED FIXED
2.2 S3 (9jan)
Tracking Status
b2g-v2.2 --- fixed

People

(Reporter: RyanVM, Assigned: _AtilA_)

References

Details

Attachments

(15 files, 5 obsolete files)

108 bytes, text/plain
mwu
: review+
Details
815 bytes, patch
glandium
: review-
Details | Diff | Splinter Review
2.18 KB, patch
mwu
: review+
Details | Diff | Splinter Review
1.29 KB, patch
Details | Diff | Splinter Review
9.46 KB, patch
mwu
: review+
Details | Diff | Splinter Review
49 bytes, text/x-github-pull-request
Details | Review
53 bytes, text/x-github-pull-request
Details | Review
52 bytes, text/x-github-pull-request
Details | Review
52 bytes, text/x-github-pull-request
Details | Review
3.94 KB, patch
rillian
: review+
Details | Diff | Splinter Review
52 bytes, text/x-github-pull-request
mwu
: review+
Details | Review
1.05 KB, patch
kinetik
: review+
Details | Diff | Splinter Review
52 bytes, text/x-github-pull-request
mwu
: review+
Details | Review
52 bytes, text/x-github-pull-request
mwu
: review+
Details | Review
1.48 KB, text/plain
Details
B2G ICS builds are built with GCC 4.4, by far the oldest compiler we still support. It causes ongoing pain for developers and adds complexity to our codebase having to support it. Per discussion in #b2g today, it may be possible to move ICS builds to a newer toolchain which would allow us to drop 4.4 support. We should investigate if this is feasible and and consider doing so if it is.
I thought this used the android tolls which I thought we used the android toolset
t gcc 4.7  minimum anyway,
(In reply to Bill Gianopoulos [:WG9s] from comment #1)
> I thought this used the android tolls which I thought we used the android
> toolset
> t gcc 4.7  minimum anyway,

Unfortunately it doesn't.

I'm sure there's already an existing bug about this.
As gcc 4.4 is the official toolchain to build ICS on Android (so Gonk), we should change the toolchain to build just gecko. We are mainly using CAF repositories, so I will get the lastest toolchain there: arm-linux-androideabi-4.8. It should work without any patches, but if for any reason it cannot be done, I'll go for the next lowest version (arm-linux-androideabi-4.7), and so on.
Assignee: nobody → atilag
> we should change the toolchain to build just gecko.

This has been suggested countless times. I'm glad this is finally going to happen.
Update:
I'm trying few things in parallel:
1 - Compile Gonk with it's own toolchain (4.4.x) and Gecko with arm-linux-androideabi-4.8.
Gonk compiled as expected, but Gecko failed while linking libmozglue.so because of this:
/home/jgomez/b2g/build.emulator.ics/prebuilts/gcc/linux-x86/arm/arm-linux-androideabi-4.8/bin/../lib/gcc/arm-linux-androideabi/4.8/../../../../arm-linux-androideabi/bin/ld: error: read-only segment has dynamic relocations
collect2: error: ld returned 1 exit status
make[6]: *** [libmozglue.so] Error 1
Now I'm making sure that Gonk is being compiled with proper options (like -fpic / -fPIC) for all the libreries that are linked with Gecko. AFAIK this should fix the problem.

2 - Compile the whole system with arm-linux-androideabi-4.8
I had some compile errors in bionic, but didn't patch them because they were fixed in later versiones. So I tried to update bionic, but then it didn't compile because this newer version of Bionic needs more configuration data from the device, so I stoped this way so far.

3 - Compile the whole system with arm-linux-androideabi-4.7
I've had to fix some weird error on Android build system (build/core/TARGET_linux-arm.mk), because for some reason it wasn't compiling with crt* stuff (crtend/crtbegin) so the __dso_handle symbol wasn't being resolved. After fixing it, I needed to patch libicu4c because of a well known problem with gcc >= 4.6... and is still compiling while writting this, so I will update again in a few minutes.

I'll keep fighting... :)
Update:
Compiling the whole system (Gonk+Gecko) with arm-linux-androideabi-4.7 drives us to the same "read-only segment has dynamic relocation" error. I realized that libmozglue is not linking against any Gonk library, so we have the porlem with Gecko. Looks like the objects being linked with libMozGlue are: cpuacct.o, BionicGlue.o, arm.o and Nuwa.o. I'm lookigin at their ELF sections and try to figure out where is the problem.
Update:
I could compile Gecko with arm-linux-androideabi-4.6 toolchain.
Anyway, I'll keep trying with 4.7 and later with 4.8.
Great work here, Juan!

I'm curious, have you been in touch with anyone from CAF?  Do you know if our plans for stopping to use gcc 4.4 is OK with them?  Thanks!
Flags: needinfo?(atilag)
Thks Ehsan!
No, I didn't talk with anyone from CAF... Do we need to?
Flags: needinfo?(atilag)
Ok, I could compile Gecko with gcc-4.7 and gcc-4.8. 
I just needed to remove the "-z,text" flag. This linker flag was preventing us from having dynamic rellocations in a read-only segment. I suppose that we have set this flag for security reasons, so I'm not really sure if this could be the a valid change.
Maybe Mike Hommey could shed some light on this matter.
Flags: needinfo?(mh+mozilla)
(In reply to Juan Gomez [:_AtilA_] from comment #9)
> Thks Ehsan!
> No, I didn't talk with anyone from CAF... Do we need to?

Yes, I think so.  Michael, do you know if CAF still relies on using gcc 4.4 for ICS based Firefox OS builds?  Thanks!
Flags: needinfo?(mvines)
We have no active master-based ICS builds so it doesn't matter to us really.
Ok, but if we are planning to upgrade the toolchain for building Gonk too, it's ok for gcc-4.6 and gcc-4.7, but not for gcc-4.8 (so far). I can work on it but, is it really what we want? Build both Gonk+Gecko or just Gecko?
I don't think Gonk's compiler is really holding us back on anything of consequence. Gecko is where gcc 4.4 is a significant limitation. I think Gonk can just ride the Android trains.
Flags: needinfo?(mvines)
(In reply to Michael Vines [:m1] [:evilmachines] from comment #12)
> We have no active master-based ICS builds so it doesn't matter to us really.

That is great news!

Andreas, do you who we can talk to in order to figure out whether we still need to do ICS builds on trunk?  It would be tremendously helpful if we manage to kill ICS on trunk, because that means that we won't have to keep making our code build with gcc 4.4.
Flags: needinfo?(gal)
Ehsan, ICS can't be killed because we don't have testing running on KK or JB.
This bug is about decoupling GCC 4.4 from ICS builds, so whether or not we should kill the builds entirely seems like an orthogonal question.

I think Juan has already clearly shown that we can update GCC to at least 4.7 without much difficulty and it appears there is no partner need to stay on the older version anymore either, so this bug can be completed as-filed without needing to answer that question here.
Yes, my point is that for our infra test runs on the emulator, we can build Gecko with gcc 4.7+ and the rest with gcc 4.4.  If our partners do not depend on ICS any more, then we can fix this bug, and bump up the Gecko compiler requirements.
(In reply to Juan Gomez [:_AtilA_] from comment #10)
> Ok, I could compile Gecko with gcc-4.7 and gcc-4.8. 
> I just needed to remove the "-z,text" flag. This linker flag was preventing
> us from having dynamic rellocations in a read-only segment. I suppose that
> we have set this flag for security reasons, so I'm not really sure if this
> could be the a valid change.
> Maybe Mike Hommey could shed some light on this matter.

Yeah, you don't want to remove -z,text. I guess this is the good old "crt_begin.o contains text relocations bug".
Search the block that adds -nostartfiles in configure.in, and try to make it run on gonk (remove the condition for testing), and check what configure says for "whether the CRT objects have text relocations"
Flags: needinfo?(mh+mozilla)
Nailed it. Looks like we have text relocations in our crtbegin_so.o:

Relocation section '.rel.fini_array' at offset 0x408 contains 1 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
00000004  00000102 R_ARM_ABS32       00000000   .text

CRT stuff comes from bionic, so before patching/forking anything, I'm going to try upgrading it as our first option.
FWIW, the corresponding upstream bug: https://code.google.com/p/android/issues/detail?id=23203
There's no clear indication of what specific changeset fixed it.
Upgrading bionic to a working/fixed version (the one from JellyBean) is not possible, it depends on new device configurations that in turn depends on a newer version of the platform_build repository, so I've finally applied the necessary patches to Bionic current ICS version. With these patches applied and a little modification in configure.in to overcome the infamous __dso_handle symbol problems, I managed to compile Gecko with the google toolchain based on gcc-4.8.

I'll post non-Bionic related patches to be reviewed here ASAP, but we should first fork Bionic from: git://codeaurora.org/platform/bionic (tag: android-4.0.4_r2.1). When forked I'll make a Pull-Request on github to apply the patches, so with this fixed version of Bionic we can freely apply/land the other patches.
b2g_ics_strawberry and b2g-4.0.4_r2.1 created on mozilla-b2g/platform_bionic.
Comment on attachment 8489108 [details]
platform_bionic PRs for both branches: b2g_ics_strawberry and b2g-4.0.4_r2.1.

I think bugzilla only really supports a single PR per attachment.. oh well close enough. Thanks!
Attachment #8489108 - Flags: review?(mwu) → review+
Attachment #8490386 - Flags: review?(ted) → review?(mh+mozilla)
oh! didn't set Mike for review because I thought he was on PTO :)
Comment on attachment 8491851 [details] [diff] [review]
Set GECKO_TOOLS_PREFIX via path discovery to compile Gecko (for ARM)

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

::: core/combo/TARGET_linux-arm.mk
@@ +48,5 @@
>  endif
>  
> +ifeq ($(strip $(GECKO_TOOLS_PREFIX)),)
> +    # Use the Gonk toolchain by default to compile Gecko
> +    GECKO_TOOLS_PREFIX := $(TARGET_TOOLS_PREFIX)

I don't think we need a default - gonk-misc needs to fall back on TARGET_TOOLS_PREFIX if GECKO_TOOLS_PREFIX isn't available. Otherwise this will break on JB/KK.

@@ +50,5 @@
> +ifeq ($(strip $(GECKO_TOOLS_PREFIX)),)
> +    # Use the Gonk toolchain by default to compile Gecko
> +    GECKO_TOOLS_PREFIX := $(TARGET_TOOLS_PREFIX)
> +    # ... but look for the existence of a better toolchain
> +    gcc_toolchain_path := prebuilts/gcc/$(HOST_PREBUILT_TAG)/arm/arm-linux-androideabi-4.8/bin

Can we use a wildcard to select the GCC version? Then we can use any version in the future without touching this, as long as we don't put more than one toolchain in the directory.
Attachment #8491852 - Attachment is obsolete: true
Attachment #8491852 - Flags: review?(mwu)
Attachment #8492376 - Flags: review?(mwu)
Attachment #8492376 - Flags: review?(mwu) → review+
Attachment #8492381 - Attachment is obsolete: true
Attachment #8492381 - Flags: review?(mwu)
Attachment #8492388 - Flags: review?(mwu)
Comment on attachment 8492388 [details] [diff] [review]
Set GECKO_TOOLS_PREFIX via path discovery to compile Gecko for ARM (v2)

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

::: core/combo/TARGET_linux-arm.mk
@@ +50,5 @@
> +ifeq ($(strip $(GECKO_TOOLS_PREFIX)),)
> +    # Look for the existence of a better toolchain
> +    gcc_toolchain_path := $(firstword $(wildcard prebuilts/gcc/$(HOST_PREBUILT_TAG)/arm/arm-linux-androideabi*/bin))
> +    ifneq ($(strip $(wildcard $(gcc_toolchain_path))),)
> +        GECKO_TOOLS_PREFIX = $(gcc_toolchain_path)/arm-linux-androideabi-

Use := here.
Attachment #8492388 - Flags: review?(mwu) → review+
Comment on attachment 8490386 [details] [diff] [review]
Add gcc-4.7 and gcc-4.8 to the list of compilers that need to define __dso_handle symbol with the correct visibility

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

While it was kind of okayish to do that for a few old versions b2g is stuck with, it's not okay to do that with more widespread recent versions. In particular, this would affect linux desktop builds, as well as android builds that don't have the dso_handle problem.
It's time to switch to detecting whether the problem exists instead of relying on a gcc version, since the crt files are not tied to the gcc version on android/gonk.
Attachment #8490386 - Flags: review?(mh+mozilla) → review-
Ok, I just realized that we are compiling crt stuff with the GLOBAL/DEFAULT visibility for __dso_handle symbols because ICS uses gcc-4.4 and this is the default. So we don't really need to add any check to configure.in. Anyway, JB/KK Gonk are compiled with gcc-4.6 or greater and here the default visibility is HIDDEN/GLOBAL so for those use-cases we would still need the autodetect mechanism that Mike is proposing, but maybe we could handle this issue in another bug and try to fix ICS builds so far. 
Of course, I'd love to work in that bug too.
There are other ICS devices that uses other remotes to download their Bionic, like: Tarako, Fugu and Pandaboard. The latter gets its copy from CAF, but uses another revision (aosp-new/ics-plus-aosp).
What should we do with them?
Attachment #8496603 - Flags: review?(mwu)
Pandaboard can be forced to use the bionic we have here. It's also not an important device to support - we only added support since we were looking into using them for testing.

Fugu is deprecated and no products shipped from that - we can drop that manifest entirely.

Tarako is a bit tricky.. we can probably create a fork just for that.
Comment on attachment 8496603 [details] [diff] [review]
Updated ICS manifests to point new bionic branches and gcc-4.8 prebuilts

Per IRC, waiting for a new patch.
Attachment #8496603 - Flags: review?(mwu)
No longer blocks: 1056407
Comment on attachment 8496603 [details] [diff] [review]
Updated ICS manifests to point new bionic branches and gcc-4.8 prebuilts

As talked on IRC, we can update devices later so let's update these ones first: Emulator/s, Galaxy Nexus, Galaxy S2,Hamachi, Helix, Leo, Nexus-S 4g and Nexus-S.
As soon as Geeksphone guys merge the PR I launched them (to update their platfom_build fork), I will update Peak and Keon too.
So only 3 devices left: Tarako (the only who really matters), Fugu, and Pandaboard.
Attachment #8496603 - Flags: review?(mwu)
Comment on attachment 8496603 [details] [diff] [review]
Updated ICS manifests to point new bionic branches and gcc-4.8 prebuilts

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

r=me if you also add the OSX version of the toolchain.
Attachment #8496603 - Flags: review?(mwu) → review+
btw, the Firefox for Android build machine still uses gcc 4.7 (bug 899405), but could be updated to use gcc 4.8 (bug 900797) with a little work.
I guess it's a little too early to suggest GCC 4.9, but it does have ASan for GCC on ARM, which is likely to be useful for finding security issues. [1]

Plus, it's already available on Android NDK version 10c. [2]

[1] https://gcc.gnu.org/gcc-4.9/changes.html
[2] https://developer.android.com/tools/sdk/ndk/index.html#Revisions
Gary,
We can fill a new bug to update to gcc-4.9. 
With this bug landed, it will be fairly easy to test and update.
> We can fill a new bug to update to gcc-4.9. 
> With this bug landed, it will be fairly easy to test and update.

Thanks, filed bug 1087161.
Depends on: 1087192
Depends on: 1089194
Build tests passed!
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=eb36b4f8c541

We can merge the final PR (comment #50) and close the bug.
Flags: needinfo?(mwu)
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/b2g-manifest/commit/53a09b32c170d171e484cc8d14d1e87645cc2389

And a CLOBBER touch to be safe:
https://hg.mozilla.org/integration/b2g-inbound/rev/7aca153e69f0
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S8 (7Nov)
So, for the record, what version of GCC are the B2G builds using now? Also, perhaps there should be a PSA about this on mozilla.dev.platform as a heads up for the people who were waiting for GCC 4.4 to die ;)
Landed and backed out due to hamachi red.
Status: RESOLVED → REOPENED
Flags: needinfo?(mwu)
Resolution: FIXED → ---
UPDATE: 
* I'm trying to figure out why 3 cppunitests are crashing with gcc-4.8 (emulator build)
* Hamachi build are working ok locally, so it looks like we need to install a package that is not being installed into the build servers.
Well, looks like Hamachi builds are not working anymore with gcc-4.8. Ineed, I'm hitting a compiler bug [1] that was fixed and landed time ago in the gcc mainstream, but looks like the google/caf toolchain that I'm using didn't get the fix.
The bug was induced by Libopus (media/libopus),so I'll try to make a patch to overcome the code that is causing the compiler bug until the toolchain gets fixed (I'll report it).

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56797
Blocks: 1102195
I have finally come with a reduced testcase for the ICE and submited a bug report to Google https://code.google.com/p/android/issues/detail?id=80723

Let's see what happens.
This is a temporary patch for overcoming the ICE with GCC-4.8 in OPUS library.
Attachment #8529251 - Flags: review?(giles)
Comment on attachment 8529251 [details] [diff] [review]
0001-Bug-1056337-Upgrade-toolchain-libopus-gcc-4-8-ICE.patch

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

Approach is fine. Thanks for reducing the problem to the specific routine the compiler fails on. Please address comments and ask for review again.

::: media/libopus/gcc-4.8-ICE.patch
@@ +18,5 @@
> +    return codedBands;
> + }
> + 
> ++#if !defined(__clang__) && defined(__GNUC__) && \
> ++    __GNUC__ == 4 && __GNUC_MINOR__ == 8

Is there some way to limit this to arm, b2g or ICS builds? It's a shame to slow down other targets where gcc 4.8 works fine.

::: media/libopus/update.sh
@@ +74,4 @@
>  python gen-sources.py $1
>  
>  # apply outstanding local patches
> +patch -p3 < $1/gcc-4.8-ICE.patch

patch should be in the source directory, so you want ./ or $(dirname $0)/ instead of $1, which is the upstream source directory.

You also need to apply the patch to the in-tree opus code, either manually, or ideally by running update.sh against an opus checkout of the revision we're currently using (v1.1 tag).

update.sh is about future-proofing your change; you still have to make the change and commit it to the tree for it to take effect now.
Attachment #8529251 - Flags: review?(giles)
(In reply to Ralph Giles (:rillian) from comment #63)
> Comment on attachment 8529251 [details] [diff] [review]
> 0001-Bug-1056337-Upgrade-toolchain-libopus-gcc-4-8-ICE.patch
> 
> Review of attachment 8529251 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Approach is fine. Thanks for reducing the problem to the specific routine
> the compiler fails on. Please address comments and ask for review again.
> 
> ::: media/libopus/gcc-4.8-ICE.patch
> @@ +18,5 @@
> > +    return codedBands;
> > + }
> > + 
> > ++#if !defined(__clang__) && defined(__GNUC__) && \
> > ++    __GNUC__ == 4 && __GNUC_MINOR__ == 8
> 
> Is there some way to limit this to arm, b2g or ICS builds? It's a shame to
> slow down other targets where gcc 4.8 works fine.
> 

We will get the ICE in all ARM platforms using this gcc-4.8 version, so I think that the most appropiate tune will be add the arm architecture to the condition.

> ::: media/libopus/update.sh
> @@ +74,4 @@
> >  python gen-sources.py $1
> >  
> >  # apply outstanding local patches
> > +patch -p3 < $1/gcc-4.8-ICE.patch
> 
> patch should be in the source directory, so you want ./ or $(dirname $0)/
> instead of $1, which is the upstream source directory.
> 
> You also need to apply the patch to the in-tree opus code, either manually,
> or ideally by running update.sh against an opus checkout of the revision
> we're currently using (v1.1 tag).
> 
> update.sh is about future-proofing your change; you still have to make the
> change and commit it to the tree for it to take effect now.

Nice, I have added the fix of the offending file to the patch.
Comment on attachment 8529334 [details] [diff] [review]
0001-Bug-1056337-Upgrade-toolchain-libopus-gcc-4-8-ICE.v2.patch

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

Looks good to me. Thanks for the fix!
Attachment #8529334 - Flags: review?(giles) → review+
Keywords: checkin-needed
Keywords: leave-open
Please land just the last patch:  0001-Bug-1056337-Upgrade-toolchain-libopus-gcc-4-8-ICE.v2.patch
Pushed the last patch to inbound after fixing the commit message.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d1cbbd46b0d
Keywords: checkin-needed
I wanted to paste the try-server link before pushing, but I don't know why it took A LOT more time to complete as usual...

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f08b168c851b
(In reply to Juan Gomez [:_AtilA_] from comment #69)
> I wanted to paste the try-server link before pushing, but I don't know why
> it took A LOT more time to complete as usual...

We had infrastructure problems for most of today. No jobs were running.
Comment on attachment 8529334 [details] [diff] [review]
0001-Bug-1056337-Upgrade-toolchain-libopus-gcc-4-8-ICE.v2.patch

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

::: media/libopus/gcc-4.8-ICE.patch
@@ +17,5 @@
> + }
> + 
> ++#if !defined(__clang__) && defined(__GNUC__) && defined(__arm__) && \
> ++    __GNUC__ == 4 && __GNUC_MINOR__ == 8
> ++#warning "OPUS library causes an internal compiler error for gcc-4.8 based toolchain in arm"

btw, the gcc bug report [1] suggests this compiler bug was fixed in upstream gcc 4.8.2, so you might be able to amend this version check to include `__GNUC_PATCHLEVEL__ >= 2` so this deoptimization automatically disables itself when compiling with a future, fixed gcc 4.8.x. :)

Also, a patch [2] on bug 1104663 suggests -Os may avoid the compiler bug without falling back to -O0.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58394
[2] https://bugzilla.mozilla.org/attachment.cgi?id=8528890&action=diff
Depends on: 1108228
Has there been any progress on this lately?  I can't wait to see this awesome change make its way into the trees! :-)
Yeah sorry! I've been very busy these days :(
Right now, I'm trying to figure out why one of the cpp unit tests is failing witch gcc-4.8 (and gcc-4.7): test_tone (gecko/media/libcubeb/tests/)
As bug 1108228 shows, Bionic is segfaulting when the program finally exits.
I've tried with Valgrind as well but nothing wrong seems to be happening...
I'm running out of ideas!! So any help would be appreciated :)
If you're hung up on a single C++ unit test, I'd recommend just disabling it for B2G and investigating afterwards. It's not fantastic, but this change is blocking a lot of other work so I think it's a worthwhile tradeoff.
Attachment #8539831 - Flags: review?(mwu) → review+
Keywords: checkin-needed
Please, checkin the last patch only (attachment 8539831 [details] [review]) :)
Hi Juan,

is the try failure in cpp unitests realted to this checkin ?
Flags: needinfo?(atilag)
Keywords: checkin-needed
Yes, this is the cppunit test that fails. Bug 1108228 was filled to find out what's happening.
Flags: needinfo?(atilag)
I agree with Ted in disabling this one last test and figuring out what's happening there in bug 1108228.  Wonderful work here, Juan!
As Ted mentioned in bug 1108228 - comment #6 , this patch will disable test_tone for B2G/ICS devices. Thanks Ted!

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=79c553afe212

Happy Holidays btw! ;)
Attachment #8541522 - Flags: review?(mwu)
Attachment #8541522 - Flags: review?(mwu) → review?(kinetik)
Attachment #8541522 - Flags: review?(kinetik) → review+
I'm about to close bug 1108228, so this patch won't need to land.
So can we now reland everything here?
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f9a41dd22da
Ok, tests passed for the emulator.
We can land patch:  B2G ICS Emulator only changes to upgrade the toolchain (gcc-4.8).
Keywords: checkin-needed
Wow, that's great news! Thanks so much for your hard work on this!

<breaks out the champagne>
(In reply to Juan Gomez [:_AtilA_] from comment #84)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f9a41dd22da
> Ok, tests passed for the emulator.
> We can land patch:  B2G ICS Emulator only changes to upgrade the toolchain
> (gcc-4.8).

landed this in https://github.com/mozilla-b2g/b2g-manifest/commit/5bce19fd07b01f2f66cd64f5387764b07842ac48
Thanks for seeing this through!
And a clobber touch (since at least the first ICS emulator opt job I've seen on b-i still shows 4.4 being used).
https://hg.mozilla.org/integration/b2g-inbound/rev/30e4856f503e
Flags: needinfo?(gal)
Target Milestone: 2.1 S8 (7Nov) → 2.2 S3 (9jan)
https://hg.mozilla.org/mozilla-central/rev/30e4856f503e
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Device image builds haven't made the change yet. Reopening to avoid confusion.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This PR modifies the Hamachi manifest only. I cannot launch try-server jobs for devices, so if it burns the tree, just backit out and if I'm not online, please paste/send me the result.
I sucessfully build in my local machine with a fresh build.
Attachment #8546802 - Flags: review?(mwu)
Comment on attachment 8546802 [details] [review]
Update Hamachi manifest to use new gcc-4.8 based toolchain

Ok, but don't close this bug until every ICS device is switched over.
Attachment #8546802 - Flags: review?(mwu) → review+
Please land only this very last patch.
So, as said. If this patch fails (most probably), backit out and send me the errors. :)
Master: https://github.com/mozilla-b2g/b2g-manifest/commit/d038fbc528a1326da53a75f3d829a0834f017b46

To those playing from home, we're still waiting for a patch to change over all the other device image builds still. This only changes Hamachi builds.
Keywords: checkin-needed
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #94)

Reverted for Hamachi bustage.
Master: https://github.com/mozilla-b2g/b2g-manifest/commit/0ebf1a21491c1c323e828a405b2a0ce543a7b77a

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=1132723&repo=b2g-inbound

Looks like it was attempting to use gcc-4.8 :(
Depends on: 1119997
As per IRC conversation, this is the PR to migrate all important ICS devices.
Attachment #8546920 - Flags: review?(mwu)
Attachment #8546920 - Flags: review?(mwu) → review+
Note, I wonder, does this change mean that people building on mac get to download an additional linux toolchain they don't care about?
On ICS, you always had to download every version of every toolchain on every platform. All the toolchains plus some other things were stuffed into the prebuilt repo. So yes, but the addition of this toolchain doesn't hurt much on ICS.

The prebuilt repo was removed after ICS and replaced with individual toolchain repos in the prebuilts directory.
And we're green on all ICS-based device image builds \m/
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Juan, thanks for your work. It looks like Keon build is currently broken. I get that you want to move Peak and Keon devices to GCC 4.8 also, do you have any ETA on this?
Flags: needinfo?(atilag)
Hi Alex, Geeksphone has it's own fork of platform_build, so they must merge my changes in order to build. Anyway, I'm going to facilitate their job and PR they repos with my changes (I did it time ago, but they didn't respond). As soon as they merge the changes, I'll update the manifests (Keon and Peak).
Flags: needinfo?(atilag)
I've had issues contacting GP in the past so I got in touch with them now through the same channel that worked for me last time.

And now as it happens, their platform_build repo has been updated with your patch.
Flags: needinfo?(atilag)
Oh! ok, they didn't cherry-pick my commit hehehe.
There's also a bug compiling Peak (I don't know about Keon, but probably) that I'll fix here: bug 1120239 ASAP.
Flags: needinfo?(atilag)
Duplicate of this bug: 1104663
You need to log in before you can comment on or make changes to this bug.