| Message ID | 1554782710-27144-1-git-send-email-ruanzw@xiaopeng.com |
|---|---|
| State | Superseded |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [v1] include/mk/env_post.mk: enable __ANDROID__ definition for Android build | expand |
Hi Steve, Sandeep, > Enable the __ANDROID__ definition by default for Android build, otherwise we > should manually enable it by configure command. > Signed-off-by: Zhengwang Ruan <ruanzw@xiaopeng.com> > --- > include/mk/env_post.mk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk > index 913bdf5..7953e0a 100644 > --- a/include/mk/env_post.mk > +++ b/include/mk/env_post.mk > @@ -44,7 +44,7 @@ endif > ifeq ($(ANDROID),1) > # There are many undeclared functions, it's best not to accidentally overlook > # them. > -CFLAGS += -Werror-implicit-function-declaration > +CFLAGS += -Werror-implicit-function-declaration -D__ANDROID__ > LDFLAGS += -L$(top_builddir)/lib/android_libpthread > LDFLAGS += -L$(top_builddir)/lib/android_librt Please, when you have time, can you please have a look into this? IMHO the question is whether code guarded by __ANDROID__ (lib/tst_kernel.c, rt_tgsigqueueinfo01.c, etc.) works as expected in NDK build (as in aosp you use definitions in Android.bp anyway). According to [1] -D__ANDROID__ is defined by toolchain (I understand it by both aosp build and builds using NDK), so it shouldn't be needed. Not sure if it makes sense to build for android with non android toolchain (i.e. no aosp nor NDK, use generic arm cross compile toolchain or standard gcc/clang in case of intel platform). If yes, than this patch would be useful, as generic toolchain obviously don't define __ANDROID__. Kind regards, Petr [1] https://groups.google.com/forum/#!topic/android-ndk/cf9_f1SLXls
Hi Petr, Zhengwang, On 4/24/19 2:14 AM, Petr Vorel wrote: > Hi Steve, Sandeep, > ... > > Please, when you have time, can you please have a look into this? > > IMHO the question is whether code guarded by __ANDROID__ (lib/tst_kernel.c, > rt_tgsigqueueinfo01.c, etc.) works as expected in NDK build (as in aosp you use > definitions in Android.bp anyway). > > According to [1] -D__ANDROID__ is defined by toolchain (I understand it by both > aosp build and builds using NDK), so it shouldn't be needed. Using Sandeep's instructions and with your latest merged fixes I was able to build unmodified upstream with the NDK. I just tried running one of the test binaries on my device to sanity check it. I put in an #error if __ANDROID__ is defined and it does look to be set by the NDK compiler. FWIW I am using Android API level 26, the minimum which provides hasmntopt. This gets configured in the "Set up ndk toolchains for autoconf" step Sandeep mentioned by setting the CC and CXX env vars appropriately. Zhengwang can you try the steps Sandeep mentioned (with the API level tweak I mentioned above)? > Not sure if it makes sense to build for android with non android toolchain (i.e. > no aosp nor NDK, use generic arm cross compile toolchain or standard gcc/clang > in case of intel platform). If yes, than this patch would be useful, as generic > toolchain obviously don't define __ANDROID__. Zhengwang had said > In the case of cross-compilation, I think it still makes sense, since > we can use the same toolchain, such as clang, and build parameters > (the specify the include headers and libs, etc) as AOSP. but it's not clear to me since you need to have bionic around anyway, and at that point, why not just use the Android toolchain? thanks, Steve
Hi Steve, Zhengwang, > Using Sandeep's instructions and with your latest merged fixes I was able to > build unmodified upstream with the NDK. I just tried running one of the test > binaries on my device to sanity check it. > I put in an #error if __ANDROID__ is defined and it does look to be set by > the NDK compiler. FWIW I am using Android API level 26, the minimum which > provides hasmntopt. This gets configured in the "Set up ndk toolchains for > autoconf" step Sandeep mentioned by setting the CC and CXX env vars > appropriately. Thanks for testing. IMHO this is a simple proof that this patch (-D__ANDROID__) is not needed. ... > > In the case of cross-compilation, I think it still makes sense, since > > we can use the same toolchain, such as clang, and build parameters > > (the specify the include headers and libs, etc) as AOSP. > but it's not clear to me since you need to have bionic around anyway, and at > that point, why not just use the Android toolchain? No, you're right, that does not make sense. > thanks, > Steve Kind regards, Petr
Hi Steve, -------- Original Message -------- From: Steve Muckle Sent: Wed, 24 Apr 2019 12:23:01 -0700 To: Petr Vorel, Zhengwang Ruan Cc: Sandeep Patil, Ltp, Kernel-team Subject: Re: [LTP] [PATCH v1] include/mk/env_post.mk: enable __ANDROID__ definition for Android build > Hi Petr, Zhengwang, > > On 4/24/19 2:14 AM, Petr Vorel wrote: >> Hi Steve, Sandeep, >> > ... >> >> Please, when you have time, can you please have a look into this? >> >> IMHO the question is whether code guarded by __ANDROID__ >> (lib/tst_kernel.c, >> rt_tgsigqueueinfo01.c, etc.) works as expected in NDK build (as in >> aosp you use >> definitions in Android.bp anyway). >> >> According to [1] -D__ANDROID__ is defined by toolchain (I understand >> it by both >> aosp build and builds using NDK), so it shouldn't be needed. > > Using Sandeep's instructions and with your latest merged fixes I was > able to build unmodified upstream with the NDK. I just tried running > one of the test binaries on my device to sanity check it. > > I put in an #error if __ANDROID__ is defined and it does look to be > set by the NDK compiler. FWIW I am using Android API level 26, the > minimum which provides hasmntopt. This gets configured in the "Set up > ndk toolchains for autoconf" step Sandeep mentioned by setting the CC > and CXX env vars appropriately. > > Zhengwang can you try the steps Sandeep mentioned (with the API level > tweak I mentioned above)? Yes, I tried the steps as Sandeep mentioned and also put an "#error" if __ANDROID__ was defined, it proves NDK has defined it. Thanks you guys very much for letting me know this. :-) > >> Not sure if it makes sense to build for android with non android >> toolchain (i.e. >> no aosp nor NDK, use generic arm cross compile toolchain or standard >> gcc/clang >> in case of intel platform). If yes, than this patch would be useful, >> as generic >> toolchain obviously don't define __ANDROID__. > > Zhengwang had said > > > In the case of cross-compilation, I think it still makes sense, since > > we can use the same toolchain, such as clang, and build parameters > > (the specify the include headers and libs, etc) as AOSP. > > but it's not clear to me since you need to have bionic around anyway, > and at that point, why not just use the Android toolchain? Yes, NDK should always be the best in the situation of cross-compiling for android. Thanks again! Regards, Zhengwang > > > thanks, > Steve <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body smarttemplateinserted="true"> <div id="smartTemplate4-template"> <p>Hi Steve, <br> </p> </div> <br> <div id="smartTemplate4-quoteHeader">-------- Original Message --------<br> From: Steve Muckle<br> Sent: Wed, 24 Apr 2019 12:23:01 -0700<br> To: Petr Vorel, Zhengwang Ruan<br> Cc: Sandeep Patil, Ltp, Kernel-team<br> Subject: Re: [LTP] [PATCH v1] include/mk/env_post.mk: enable __ANDROID__ definition for Android build<br> </div> <blockquote type="cite" cite="mid:edc171bb-7351-1069-5a80-8b5c67547fa5@google.com">Hi Petr, Zhengwang, <br> <br> On 4/24/19 2:14 AM, Petr Vorel wrote: <br> <blockquote type="cite">Hi Steve, Sandeep, <br> <br> </blockquote> ... <br> <blockquote type="cite"> <br> Please, when you have time, can you please have a look into this? <br> <br> IMHO the question is whether code guarded by __ANDROID__ (lib/tst_kernel.c, <br> rt_tgsigqueueinfo01.c, etc.) works as expected in NDK build (as in aosp you use <br> definitions in Android.bp anyway). <br> <br> According to [1] -D__ANDROID__ is defined by toolchain (I understand it by both <br> aosp build and builds using NDK), so it shouldn't be needed. <br> </blockquote> <br> Using Sandeep's instructions and with your latest merged fixes I was able to build unmodified upstream with the NDK. I just tried running one of the test binaries on my device to sanity check it. <br> <br> I put in an #error if __ANDROID__ is defined and it does look to be set by the NDK compiler. FWIW I am using Android API level 26, the minimum which provides hasmntopt. This gets configured in the "Set up ndk toolchains for autoconf" step Sandeep mentioned by setting the CC and CXX env vars appropriately. <br> <br> Zhengwang can you try the steps Sandeep mentioned (with the API level tweak I mentioned above)? <br> </blockquote> <br> <p>Yes, I tried the steps as Sandeep mentioned and also put an "#error" if __ANDROID__ was defined, it proves NDK has defined it.</p> <p>Thanks you guys very much for letting me know this. :-)</p> <p><br> </p> <blockquote type="cite" cite="mid:edc171bb-7351-1069-5a80-8b5c67547fa5@google.com"> <br> <blockquote type="cite">Not sure if it makes sense to build for android with non android toolchain (i.e. <br> no aosp nor NDK, use generic arm cross compile toolchain or standard gcc/clang <br> in case of intel platform). If yes, than this patch would be useful, as generic <br> toolchain obviously don't define __ANDROID__. <br> </blockquote> <br> Zhengwang had said <br> <br> > In the case of cross-compilation, I think it still makes sense, since <br> > we can use the same toolchain, such as clang, and build parameters <br> > (the specify the include headers and libs, etc) as AOSP. <br> <br> but it's not clear to me since you need to have bionic around anyway, and at that point, why not just use the Android toolchain? <br> </blockquote> <p>Yes, NDK should always be the best in the situation of cross-compiling for android. Thanks again!</p> <p>Regards,</p> <p>Zhengwang<br> </p> <blockquote type="cite" cite="mid:edc171bb-7351-1069-5a80-8b5c67547fa5@google.com"> <br> <br> thanks, <br> Steve <br> </blockquote> </body> </html>
diff --git a/include/mk/env_post.mk b/include/mk/env_post.mk index 913bdf5..7953e0a 100644 --- a/include/mk/env_post.mk +++ b/include/mk/env_post.mk @@ -44,7 +44,7 @@ endif ifeq ($(ANDROID),1) # There are many undeclared functions, it's best not to accidentally overlook # them. -CFLAGS += -Werror-implicit-function-declaration +CFLAGS += -Werror-implicit-function-declaration -D__ANDROID__ LDFLAGS += -L$(top_builddir)/lib/android_libpthread LDFLAGS += -L$(top_builddir)/lib/android_librt
Enable the __ANDROID__ definition by default for Android build, otherwise we should manually enable it by configure command. Signed-off-by: Zhengwang Ruan <ruanzw@xiaopeng.com> --- include/mk/env_post.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)