Closed Bug 721807 Opened 8 years ago Closed 8 years ago
.cpp fails to compile on X86 android
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20100101 Firefox/7.0.1 Build ID: 20110928224508 Steps to reproduce: I tried to build Fennec for an x86 Android tablet. Actual results: It failed to build APKOpen.cpp due to a chain of header dependencies ending in xpcom-config.h. Since xpcom hadn't been built yet, it couldn't find the header. Expected results: It should have known there was an stl dependency and built correctly. The fix is to add STL_FLAGS= to mozglue/android/Makefile.in.
You should ask review on someone. Brad, could you perhaps review this patch?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Thomas, why does clearing the STL_FLAGS fix this bug? And why x86 only?
(In reply to Brad Lassey [:blassey] from comment #3) > Thomas, why does clearing the STL_FLAGS fix this bug? And why x86 only? When i pulled the latest code some time after the mozglue portion was reorganized from other-liscenses, it built fine for ARM/Android, but not for x86/Android. The new code created a dependency on stl_wrappers for APKOpen. Specifically the cstring file. The cstring file has a dependency on xpcom and it hadn't been built yet when APKOpen was being built. The stl_wrappers dependency doesn't appear to be present in the ARM build. So I made it for x86 only so as to not affect the ARM build. As far as to why/how the solution works, I got the recommendation from Mike Hommey and don't know what further implications it has. "Thomas, You can probably add STL_FLAGS= in mozglue/android/Makefile.in. I think this would be the correct fix. Mike "
glandium, why do you use cstring and vector into mozglue? x86 compiler use stl-wrapper, but Zip.h uses cstring and vector. So cstring includes mozalloc, so this issue occurs.
Component: General → mozglue
Product: Fennec Native → Core
QA Contact: general → mozglue
comment #4's fix is hack. Correct fix is that mozglue dones't use STL (cstring, vector and etc).
(In reply to Makoto Kato from comment #6) > comment #4's fix is hack. Correct fix is that mozglue dones't use STL > (cstring, vector and etc). No, the correct fix is that mozglue does no use mozalloc, and "STL_FLAGS=" is the way to do that.
I'd say the ifeq ($(CPU_ARCH),x86) in the patch is unnecessary.
Or, if you want an even more correct way to do it, it would be to fold mozalloc into mozglue, and remove mozalloc dependency on xpcom-config.h. While folding mozalloc into mozglue is in my agenda, it's more work than just adding "STL_FLAGS=", which, again, is a right way to fix this issue.
Assignee: nobody → m_kato
Attachment #592177 - Attachment is obsolete: true
Attachment #597322 - Flags: review?(mh+mozilla)
Comment on attachment 597322 [details] [diff] [review] fix v2 Review of attachment 597322 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozglue/android/Makefile.in @@ +67,5 @@ > DEFINES += -DANDROID_ARM_LINKER > +else > +ifeq ($(CPU_ARCH),x86) > +DEFINES += -DANDROID_X86_LINKER > +endif You don't need that. ANDROID_ARM_LINKER is only needed in mozglue/android for cacheflush, which is arm-specific. Nothing requires ANDROID_X86_LINKER, and nothing will.
Attachment #597322 - Flags: review?(mh+mozilla) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.