1585470 - Android cpu-features.c included twice when building GeckoView with --disable-webrtc

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P3)

Description

[Matthew Finkel [:sysrqb]]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

Build Firefox for Android with |--disable-webrtc|

Actual results:

Multiple definitions due to cpu-features.c inclusion in two libraries.

https://searchfox.org/mozilla-central/source/media/libaom/moz.build#92
https://searchfox.org/mozilla-central/source/media/libvpx/moz.build#102

Expected results:

It should compile without error.

Comment 1

[Matthew Finkel [:sysrqb]]

36:05.43 /home/android/.mozbuild/android-ndk-r15c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: error: /home/andr
oid/tor-browser/obj-arm-linux-androideabi/toolkit/library/../../media/libaom/cpu-features.o: multiple definition of 'android_getCpuCount'                                                                   
36:05.43 /home/android/.mozbuild/android-ndk-r15c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: /home/android/tor
-browser/obj-arm-linux-androideabi/toolkit/library/../../media/libvpx/cpu-features.o: previous definition here
36:05.43 /home/android/.mozbuild/android-ndk-r15c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: error: /home/andr
oid/tor-browser/obj-arm-linux-androideabi/toolkit/library/../../media/libaom/cpu-features.o: multiple definition of 'android_getCpuFamily'
36:05.43 /home/android/.mozbuild/android-ndk-r15c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: /home/android/tor
-browser/obj-arm-linux-androideabi/toolkit/library/../../media/libvpx/cpu-features.o: previous definition here
[...]

We found excluding cpu-features.c from libaom was enough for solving this because that is conditional on not CONFIG['MOZ_WEBRTC'] where as libvpx is unconditional:
https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-68.1.0esr-9.0-2&id=d09248948db5152475ee419d6f07e0f971b5817d

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Moving this to the GeckoView product for triage.

Comment 3

[C.M.Chang[:chunmin]]

Set to P3 for now, feel free to change it.

Comment 4

[Matthew Finkel [:sysrqb]]

Attachment: Bug 1585470 - Remove duplicate cpu-features.c definition when building GV without webrtc

Comment 5

[Matthew Finkel [:sysrqb]]

https://firefoxci.taskcluster-artifacts.net/RwHGwwZTSE2U358shSgJZg/0/public/logs/live_backing.log

[task 2020-04-24T20:08:05.012Z] 20:08:05     INFO -  /builds/worker/fetches/android-ndk/toolchains/x86-4.9/prebuilt/linux-x86_64/lib/gcc/i686-linux-android/4.9.x/../../../../i686-linux-android/bin/ld: error: /builds/worker/workspace/obj-build/toolkit/library/build/../../../media/libaom/cpu-features.o: multiple definition of 'android_getCpuCount'

I ran two try builds. One with the patch and webrtc included, and another with the patch and webrtc excluded. Both of them built and passed cppunit, mochitests, and web-platform-tests for android-x86, android-api-16, and linux-x64.

With patch and webrtc: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e1ce5092bac373b5f331b738b6b33a4636aaaf3
With patch and without webrtc: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98c6ec425736de48c6573264e210a9dd7618ce7a

I submitted the patch for review (I'm not sure who should review this): https://phabricator.services.mozilla.com/D73085

Comment 6

[Matthew Finkel [:sysrqb]]

Hi Ralph, I saw you're the last person who touched this config block. If you're not a good reviewer for this patch then can you suggest someone else? Thanks.

Comment 7

[Matthew Finkel [:sysrqb]]

Hi glandium, do you know who I can flag for r?

I'll rebase and re-run try builds, too.

Comment 8

[Mike Hommey [:glandium]]

Flag me.

Comment 9

[Matthew Finkel [:sysrqb]]

Attachment: Bug 1585470 - Remove duplicate cpu-features.c definition when building GV without webrtc

Comment 10

[Matthew Finkel [:sysrqb]]

Finally getting back to this.

Try build with the libaom moz.build patch on top of m-c tip.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcab497de768bd76334b2e0eb197b640589aac09

Try build with libaom moz.build patch and disabling webrtc in mozconfig.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3863c6ef779b46567fbecda580fb3d5e049c00d6

Comment 11

[Matthew Finkel [:sysrqb]]

And a Try build showing m-c is still broken without the suggested patch.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bc0fe986e4290f89726321247ec20d84449a402

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/847a2421d079
Remove duplicate cpu-features.c definition when building GV without webrtc r=glandium

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/847a2421d079