B2G Emulator-x86: build error in android-signal-defs.h, need newer NDK or platform version

RESOLVED FIXED in mozilla23

Status

RESOLVED FIXED
6 years ago
8 months ago

People

(Reporter: vicamo, Assigned: vicamo)

Tracking

unspecified
mozilla23
x86
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

6 years ago
In android-signal-defs.h, we have:

   9 // All NDK platform versions have asm/sigcontext.h for ARM
  10 // Only NDK >= 6, platform >= 9 have asm/sigcontext.h for x86
  11 // Only NDK >= 8, platform >= 9 have asm/sigcontext.h for MIPS
  12 #if defined(__arm__) || defined(__thumb__) || ANDROID_VERSION >= 9
  13 #include <asm/sigcontext.h>

Gonk has api level 10 defined in bionic/libc/include/android/api-level.h. However we don't include it but rely on an autoconf variable ANDROID_VERSION, which is not assigned when we're configured with |--with-gonk=DIR|.
(Assignee)

Comment 1

6 years ago
After digging more automake configs, that's related to multiple components, not Gecko Profiler only. I suggest we always include api-level.h and deprecate ANDROID_VERSION because api-level.h is always available in both NDK and PDK.
Component: Gecko Profiler → Build Config
(Assignee)

Comment 2

6 years ago
Created attachment 735217 [details] [diff] [review]
patch
Attachment #735217 - Flags: feedback?(mh+mozilla)
Attachment #735217 - Flags: feedback?(blassey.bugs)
(Assignee)

Comment 3

6 years ago
Created attachment 735281 [details] [diff] [review]
patch

fix typo. https://tbpl.mozilla.org/?tree=Try&rev=c69a9645ee3a
Attachment #735217 - Attachment is obsolete: true
Attachment #735217 - Flags: feedback?(mh+mozilla)
Attachment #735217 - Flags: feedback?(blassey.bugs)
Attachment #735281 - Flags: feedback?(mh+mozilla)
Attachment #735281 - Flags: feedback?(blassey.bugs)
Comment on attachment 735281 [details] [diff] [review]
patch

Review of attachment 735281 [details] [diff] [review]:
-----------------------------------------------------------------

For nsprpub/configure.in, you'll want a separate patch, and do the corresponding change in nsprpub/configure.
Attachment #735281 - Flags: feedback?(mh+mozilla) → feedback+
Comment on attachment 735281 [details] [diff] [review]
patch

Ted would have a more informed opinion on this than I
Attachment #735281 - Flags: feedback?(blassey.bugs) → feedback?(ted)
(Assignee)

Updated

6 years ago
Assignee: nobody → vyang
Comment on attachment 735281 [details] [diff] [review]
patch

Review of attachment 735281 [details] [diff] [review]:
-----------------------------------------------------------------

The NSPR changes look fine. If you can give me a patch against https://hg.mozilla.org/projects/nspr/ I can land it there for you.
Attachment #735281 - Flags: feedback?(ted) → feedback+
(Assignee)

Comment 7

6 years ago
Created attachment 735737 [details] [diff] [review]
nspr patch
Attachment #735737 - Flags: review?(ted)
Attachment #735737 - Flags: review?(mh+mozilla)
(Assignee)

Comment 8

6 years ago
Created attachment 735740 [details] [diff] [review]
m-c patch

Should I keep nspr changes in this patch as well? Sorry I don't really know nspr landing rules.
Attachment #735281 - Attachment is obsolete: true
Attachment #735740 - Flags: review?(ted)
Attachment #735740 - Flags: review?(mh+mozilla)
Comment on attachment 735740 [details] [diff] [review]
m-c patch

Review of attachment 735740 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ -9333,5 @@
>      _SUBDIR_CONFIG_ARGS="`echo $_SUBDIR_CONFIG_ARGS | sed -e 's/--enable-wrap-malloc *//'`"
>  fi
>  
>  if test -z "$MOZ_NATIVE_NSPR"; then
> -    ac_configure_args="$_SUBDIR_CONFIG_ARGS --with-dist-prefix=$MOZ_BUILD_ROOT/dist --with-mozilla --with-android-version=$ANDROID_VERSION"

While you're here, could you move this --with-android-version in android.m4, using NSPR_CONFIGURE_ARGS, like in https://bug860184.bugzilla.mozilla.org/attachment.cgi?id=735641 ?
Attachment #735740 - Flags: review?(ted)
Attachment #735740 - Flags: review?(mh+mozilla)
Attachment #735740 - Flags: review+
Comment on attachment 735737 [details] [diff] [review]
nspr patch

Review of attachment 735737 [details] [diff] [review]:
-----------------------------------------------------------------

I'll leave this one to ted.
Attachment #735737 - Flags: review?(mh+mozilla)
Comment on attachment 735737 [details] [diff] [review]
nspr patch

Review of attachment 735737 [details] [diff] [review]:
-----------------------------------------------------------------

Typically we only land NSPR changes in the NSPR repo, and then import snapshots of that into m-c. I'll land this in NSPR for you shortly.
Attachment #735737 - Flags: review?(ted) → review+
(Assignee)

Updated

6 years ago
Blocks: 860594
(Assignee)

Comment 12

6 years ago
Created attachment 736145 [details] [diff] [review]
nspr patch

Update r=ted only.
Attachment #735737 - Attachment is obsolete: true
Attachment #736145 - Flags: review+
(Assignee)

Comment 13

6 years ago
Created attachment 736244 [details] [diff] [review]
m-c patch : v2

Address review comment.
Attachment #735740 - Attachment is obsolete: true
Attachment #736244 - Flags: review+
(Assignee)

Comment 14

6 years ago
Wait for bug 860184.
Depends on: 860184
Comment on attachment 736145 [details] [diff] [review]
nspr patch

Pushed to NSPR:
https://hg.mozilla.org/projects/nspr/rev/59246254cba9

You don't really need this to land as a prerequisite to your other patches, right? Nothing in NSPR actually uses this define.
Attachment #736145 - Flags: checkin+
(Assignee)

Comment 16

6 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #15)
> You don't really need this to land as a prerequisite to your other patches,
> right? Nothing in NSPR actually uses this define.

Yes. I had tested 1) with m-c only, 2) with nspr only, 3) with both patches, and all builds successfully in both b2g and fennec. I'm actually waiting for m-c part because it will need a rebase.
(Assignee)

Comment 17

6 years ago
Created attachment 736660 [details] [diff] [review]
m-c patch : v3

Rebased.
Attachment #736244 - Attachment is obsolete: true
Attachment #736660 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ee95ad865eab
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23

Updated

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.