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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Enrico]

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.

Comment 1

[Enrico]

Attachment: 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>

Comment 2

[Enrico]

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?

Comment 3

[:gerard-majax]

(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

Comment 4

[:gerard-majax]

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!

Comment 5

[Enrico]

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.

Comment 6

[:gerard-majax]

(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 :)

Comment 7

[Enrico]

(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.

Comment 8

[Enrico]

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

Comment 9

[:gerard-majax]

(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!

Comment 10

[Enrico]

(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?

Comment 11

[:gerard-majax]

(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 :)

Comment 12

[Enrico]

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?

Comment 13

[Enrico]

Attachment: patch on the upstream gecko side

Updates /b2g/config/aries/sources.xml to use the patched version of core/clang/HOST_x86_common.mk

Comment 14

[:gerard-majax]

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 :)

Comment 15

[:gerard-majax]

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 :)

Comment 16

[Enrico]

Attachment: Bug 1299497 - Build fails with "unsupported reloc 43" errors.

Review commit: https://reviewboard.mozilla.org/r/76188/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76188/

Comment 17

[Enrico]

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.

Comment 18

[:gerard-majax]

(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 :)

Comment 19

[:gerard-majax]

And this is going to fail:
> error.GitError: platform_build update-ref: fatal: 76946dd709234cf4bdb860672ec66d6af8f2ea3c^0: not a valid SHA1

Comment 20

[:gerard-majax]

Hell it is probably because we don't have that in the cache :/

Comment 21

[:gerard-majax]

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 ...

Comment 22

[:gerard-majax]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8438c685137b

Comment 23

[Enrico]

Attachment: Bug 1299497 - Target custom repo as remote for `platform_build`;

briefing:  add a remote that targets your repo and then change the platform_build project line to pick from your repo/branch

Review commit: https://reviewboard.mozilla.org/r/76440/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76440/

Comment 24

[Enrico]

Attachment: `./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.

Comment 25

[Enrico]

(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.

Comment 26

[:gerard-majax]

(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 :/

Comment 27

[:gerard-majax]

Comment on attachment 8787436 [details]
Bug 1299497 - Build fails with "unsupported reloc 43" errors.

https://reviewboard.mozilla.org/r/76188/#review74776

Comment 28

[:gerard-majax]

https://github.com/mozilla-b2g/platform_build/commit/63ae12cb8ca32cbbd7ef8292582f2ac2a1587a0b

Comment 29

[:gerard-majax]

Enrico, your MozReview patch queue is broken, please fix it: one change, remove the one that adds your remote.

Comment 30

[:gerard-majax]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc6d08f7ca4e

Comment 31

[Enrico]

Attachment: Backed out changeset d96a723b7e52;

Review commit: https://reviewboard.mozilla.org/r/76718/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76718/

Comment 32

[:gerard-majax]

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)

Comment 33

[:gerard-majax]

Comment on attachment 8788134 [details]
Backed out changeset d96a723b7e52;

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

Dont need this one either

Comment 34

[:gerard-majax]

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

Comment 35

[Enrico]

Comment on attachment 8787436 [details]
Bug 1299497 - Build fails with "unsupported reloc 43" errors.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76188/diff/1-2/

Comment 36

[Enrico]

(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

[:gerard-majax]

Comment on attachment 8787436 [details]
Bug 1299497 - Build fails with "unsupported reloc 43" errors.

https://reviewboard.mozilla.org/r/76188/#review74814

Comment 38

[:gerard-majax]

That's great, pushing!

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/91a0bc34de8a
Build fails with "unsupported reloc 43" errors. r=gerard-majax

Comment 40

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/91a0bc34de8a

Comment 41

[Enrico]

That's great, thanks Alexandre for your help!