Closed Bug 1299497 Opened 8 years ago Closed 8 years ago

[b2g os, build] Build fails with "unsupported reloc 43" and clang error

Categories

(Firefox OS Graveyard :: GonkIntegration, defect)

Product:
Firefox OS Graveyard
Component:
GonkIntegration
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox51 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox51 --- fixed

People

(Reporter: enrico.ghiorzi, Assigned: enrico.ghiorzi)

Details

Attachments

(2 files, 4 obsolete files)

Link to PR
54 bytes, text/x-github-pull-request
gerard-majax
: review+
Details | Review
Bug 1299497 - Build fails with "unsupported reloc 43" errors.
58 bytes, text/x-review-board-request
gerard-majax
: review+
Details 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160829102229

Steps to reproduce:

Build B2G OS as per instructions on MDN.


Actual results:

The build fails, producing multiple errors like:

libnativehelper/JniInvocation.cpp:165: error: unsupported reloc 43
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Expected results:

Build should succeed without errors.
As noticed by <https://oopsmonk.github.io/blog/2016/06/07/android-build-error-on-ubuntu-16-04-lts> an alternative solution to the proposed patch is simply to "Use AOSP after May 7, 2016". Is the AOSP version we use going to be updated soon? Should we apply the patch or update AOSP?
Flags: needinfo?(lissyx+mozillians)
(In reply to Enrico from comment #2)
> As noticed by
> <https://oopsmonk.github.io/blog/2016/06/07/android-build-error-on-ubuntu-16-
> 04-lts> an alternative solution to the proposed patch is simply to "Use AOSP
> after May 7, 2016". Is the AOSP version we use going to be updated soon?
> Should we apply the patch or update AOSP?

Can you verify if the "AOSP after May 7, 2016" includes the 5.1.1 branch ? Said otherwise, do they backported this to L ?

If so, we should just update that. If that means moving to M or N, I prefer we just apply the patch
Flags: needinfo?(lissyx+mozillians) → needinfo?(enrico.ghiorzi)
In the extent we have to include this, the patch looks ok, but I would prefer if you did a cherry-pick of upstream directly, rather than doing the same patch. It makes it easier to track, thanks!
I investigated it and apparently our 5.1.1 file is identical to the upstream one in lollipop branches, just as our 6.0.0 is identical to the marshmellow-release upstream file. The marshmellow file features a few extra commits. In particular, all flags `-no-integrated-as` have been removed for linux (and `-integrated-as` has been added for OS X). I don't know whether removing the flags would work in our case.
Flags: needinfo?(enrico.ghiorzi)
(In reply to Enrico from comment #5)
> I investigated it and apparently our 5.1.1 file is identical to the upstream
> one in lollipop branches, just as our 6.0.0 is identical to the
> marshmellow-release upstream file. The marshmellow file features a few extra
> commits. In particular, all flags `-no-integrated-as` have been removed for
> linux (and `-integrated-as` has been added for OS X). I don't know whether
> removing the flags would work in our case.

So it would be only fixed on N branches ? Guess you have to cherry-pick and update your PR then :)
(In reply to Alexandre LISSY :gerard-majax from comment #6)
> (In reply to Enrico from comment #5)
> > I investigated it and apparently our 5.1.1 file is identical to the upstream
> > one in lollipop branches, just as our 6.0.0 is identical to the
> > marshmellow-release upstream file. The marshmellow file features a few extra
> > commits. In particular, all flags `-no-integrated-as` have been removed for
> > linux (and `-integrated-as` has been added for OS X). I don't know whether
> > removing the flags would work in our case.
> 
> So it would be only fixed on N branches ? Guess you have to cherry-pick and
> update your PR then :)

It took me a while to work it out but I think I finally got it. That patch is very recent (May 2016) and has landed on android's master branch (you won't find it even in N) so as far as I can tell there is no alternative solution at the moment.
According to <https://android-review.googlesource.com/#/c/223100/> it landed on

Branches: 	master
Tags: 	        android-n-preview-3, android-n-preview-4, android-n-preview-5, android-wear-n-preview-1, android-wear-n-preview-2
(In reply to Enrico from comment #8)
> According to <https://android-review.googlesource.com/#/c/223100/> it landed
> on
> 
> Branches: 	master
> Tags: 	        android-n-preview-3, android-n-preview-4,
> android-n-preview-5, android-wear-n-preview-1, android-wear-n-preview-2

It's okay, cherry-pick that in your PR, I'm fine with it. Please make sure to push to try to ensure we are not breaking non AOSP builds.

Basically that means you have to dig into Gecko tree and hack around sources.xml files in b2g/configs/. If you need help, tell me!
Flags: needinfo?(enrico.ghiorzi)
(In reply to Alexandre LISSY :gerard-majax from comment #9)
> It's okay, cherry-pick that in your PR, I'm fine with it. Please make sure
> to push to try to ensure we are not breaking non AOSP builds.
> 
> Basically that means you have to dig into Gecko tree and hack around
> sources.xml files in b2g/configs/. If you need help, tell me!

So, if I got everything right: in gecko-b2g/b2g/config/aries-l/sources.xml (and /aries-l/ only, as others are not AOSP devices) I have to change the revision number of entry
<project name="platform_build" path="build" remote="b2g" revision="aee7ff3dba262a037559d360b62af429b62cb876">
to that of my patch, right? I should do that in master branch, right?
(In reply to Enrico from comment #10)
> (In reply to Alexandre LISSY :gerard-majax from comment #9)
> > It's okay, cherry-pick that in your PR, I'm fine with it. Please make sure
> > to push to try to ensure we are not breaking non AOSP builds.
> > 
> > Basically that means you have to dig into Gecko tree and hack around
> > sources.xml files in b2g/configs/. If you need help, tell me!
> 
> So, if I got everything right: in gecko-b2g/b2g/config/aries-l/sources.xml
> (and /aries-l/ only, as others are not AOSP devices) I have to change the
> revision number of entry
> <project name="platform_build" path="build" remote="b2g"
> revision="aee7ff3dba262a037559d360b62af429b62cb876">
> to that of my patch, right? I should do that in master branch, right?

Mostly, you are right. We will need to change the line as you said, but we need to change it on all sources.xml that are using the same right now.

There is no aries-l configuration in the tree. Only nexus-5-l would be relevant here.

And in fact, you need to do a real Gecko patch to update this, we do not need that just to play with push to try :)
Oh, so you mean patching the upstream Gecko (that at <https://github.com/mozilla/gecko-dev/tree/master/b2g/config>, even if that's the read-only mirror)? And nothing should be changed in the B2G fork?
Attached file patch on the upstream gecko side (obsolete) — 
Updates /b2g/config/aries/sources.xml to use the patched version of core/clang/HOST_x86_common.mk
Flags: needinfo?(enrico.ghiorzi) → needinfo?(lissyx+mozillians)
Attachment #8787345 - Flags: review?(lissyx+mozillians)
Flags: needinfo?(lissyx+mozillians)
Your patch looks not good technically, it's an invalid HG patch I guess. How did you made it? Maybe you should want to use MozReview ?

See http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html

Otherwise, yes, this is what I meant: any in-tree configuration that uses the same platform_build commit should be updated so that we have build/test coverage :)
Also, Enrico, can you send a try request ? FYI you can do that with |./mach try| and we are interested in aries-eng and nexus-5l-eng platforms :)

If you use MozReview you can schedule a try push from there also. We really need this to be sure you are not breaking anything else :)
Flags: needinfo?(enrico.ghiorzi)
I think I managed to PR and ask for review correctly this time. I still have to figure how to run the tests out yet, I'll try tomorrow.
(In reply to Enrico from comment #17)
> I think I managed to PR and ask for review correctly this time. I still have
> to figure how to run the tests out yet, I'll try tomorrow.

I triggered them for you :)
Flags: needinfo?(enrico.ghiorzi)
And this is going to fail:
> error.GitError: platform_build update-ref: fatal: 76946dd709234cf4bdb860672ec66d6af8f2ea3c^0: not a valid SHA1
Hell it is probably because we don't have that in the cache :/
Ho Enrico, sorry, you need to do a slightly different commit to be able to push to try: you need to add a remote that targets your repo and then change the platform_build project line to pick from your repo/branch ...
Attached file `./mach try` log (obsolete) — 
This is the log I got from `./mach try -b d -p nexus-5l-eng b2g/test`. Something looks amiss, but I can't tell whether the `try` attempt fails, or it works but it's not targeted on the right tests, or it works and detect issues with the patch.
Attachment #8787764 - Flags: review?(lissyx+mozillians)
(In reply to Alexandre LISSY :gerard-majax from comment #21)
> Ho Enrico, sorry, you need to do a slightly different commit to be able to
> push to try: you need to add a remote that targets your repo and then change
> the platform_build project line to pick from your repo/branch ...

I think I did that alright. I got a kind of result from the `try` command (attached) but it doesn't look particularly significative to me. Most probably that didn't work.
(In reply to Enrico from comment #24)
> Created attachment 8787764 [details]
> `./mach try` log
> 
> This is the log I got from `./mach try -b d -p nexus-5l-eng b2g/test`.
> Something looks amiss, but I can't tell whether the `try` attempt fails, or
> it works but it's not targeted on the right tests, or it works and detect
> issues with the patch.

No, that is not what you want. Well I'll merge everything and we will see if that is a problem :/
Attachment #8787752 - Attachment is obsolete: true
Attachment #8787752 - Flags: review?(lissyx+mozillians)
Comment on attachment 8787436 [details]
Bug 1299497 - Build fails with "unsupported reloc 43" errors.

https://reviewboard.mozilla.org/r/76188/#review74776
Attachment #8787436 - Flags: review?(lissyx+mozillians) → review+
Attachment #8787345 - Attachment is obsolete: true
Attachment #8787345 - Flags: review?(lissyx+mozillians)
Attachment #8786765 - Flags: review?(lissyx+mozillians) → review+
Enrico, your MozReview patch queue is broken, please fix it: one change, remove the one that adds your remote.
Flags: needinfo?(enrico.ghiorzi)
That is not good Enrico, I don't know what you do, just rebase your branch on top of current central, and only keep this patch: https://reviewboard.mozilla.org/r/76188/diff/1#index_header

Remove the others, it will be good (try is green)
Flags: needinfo?(enrico.ghiorzi)
Flags: needinfo?(enrico.ghiorzi)
Attachment #8787764 - Attachment is obsolete: true
Attachment #8787764 - Flags: review?(lissyx+mozillians)
Comment on attachment 8788134 [details]
Backed out changeset d96a723b7e52;

https://reviewboard.mozilla.org/r/76718/#review74802

Dont need this one either
Attachment #8788134 - Flags: review?(lissyx+mozillians) → review-
Comment on attachment 8787752 [details]
Bug 1299497 - Target custom repo as remote for `platform_build`;

https://reviewboard.mozilla.org/r/76440/#review74800

We don't need this one
Attachment #8787752 - Flags: review-
Attachment #8788134 - Attachment is obsolete: true
(In reply to Alexandre LISSY :gerard-majax from comment #32)
> That is not good Enrico, I don't know what you do, just rebase your branch
> on top of current central, and only keep this patch:
> https://reviewboard.mozilla.org/r/76188/diff/1#index_header
> 
> Remove the others, it will be good (try is green)

Sorry, hg is a mystery to me. Though, I think this time it should be alright.
Comment on attachment 8787436 [details]
Bug 1299497 - Build fails with "unsupported reloc 43" errors.

https://reviewboard.mozilla.org/r/76188/#review74814
That's great, pushing!
Flags: needinfo?(enrico.ghiorzi)
Pushed by alissy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91a0bc34de8a
Build fails with "unsupported reloc 43" errors. r=gerard-majax
Assignee: nobody → enrico.ghiorzi
Status: UNCONFIRMED → ASSIGNED
Component: General → GonkIntegration
Ever confirmed: true
https://hg.mozilla.org/mozilla-central/rev/91a0bc34de8a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
That's great, thanks Alexandre for your help!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: