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

RESOLVED FIXED

Status

Firefox OS
GonkIntegration
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Enrico, Assigned: Enrico)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

a year ago
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.
(Assignee)

Comment 1

a year ago
Created attachment 8786765 [details] [review]
Link to PR

Patch provided by <https://android-review.googlesource.com/#/c/223100/> and reported by <https://oopsmonk.github.io/blog/2016/06/07/android-build-error-on-ubuntu-16-04-lts>
Attachment #8786765 - Flags: review?(lissyx+mozillians)
(Assignee)

Comment 2

a year ago
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!
(Assignee)

Comment 5

a year ago
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.
(Assignee)

Updated

a year ago
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 :)
(Assignee)

Comment 7

a year ago
(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.
(Assignee)

Comment 8

a year ago
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)
(Assignee)

Comment 10

a year ago
(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 :)
(Assignee)

Comment 12

a year ago
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?
(Assignee)

Comment 13

a year ago
Created attachment 8787345 [details]
patch on the upstream gecko side

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)
Comment hidden (mozreview-request)
(Assignee)

Comment 17

a year ago
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 ...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8438c685137b
Comment hidden (mozreview-request)
(Assignee)

Comment 24

a year ago
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.
Attachment #8787764 - Flags: review?(lissyx+mozillians)
(Assignee)

Comment 25

a year ago
(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 27

a year ago
mozreview-review
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+
https://github.com/mozilla-b2g/platform_build/commit/63ae12cb8ca32cbbd7ef8292582f2ac2a1587a0b
Enrico, your MozReview patch queue is broken, please fix it: one change, remove the one that adds your remote.
Flags: needinfo?(enrico.ghiorzi)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc6d08f7ca4e
Comment hidden (mozreview-request)
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 33

a year ago
mozreview-review
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 34

a year ago
mozreview-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-
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8788134 - Attachment is obsolete: true
(Assignee)

Comment 36

a year ago
(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 37

a year ago
mozreview-review
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)

Comment 39

a year ago
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

Comment 40

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/91a0bc34de8a
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 41

a year ago
That's great, thanks Alexandre for your help!
You need to log in before you can comment on or make changes to this bug.