-
Notifications
You must be signed in to change notification settings - Fork 647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Android wasi support. #590
Conversation
@@ -5,6 +5,10 @@ | |||
|
|||
#include "platform_api_vmcore.h" | |||
#include "platform_api_extension.h" | |||
#include "bh_log.h" | |||
|
|||
#define API_NOT_SUPPORT_ERROR(API, LEVEL) LOG_ERROR("%s() is only supported when compile option ANDROID_PLATFORM >= %s.", #API, #LEVEL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In WAMR's design, the platform layer is the lowest layer, it should not depend any upper layer, so here we should not include bh_log.h and call LOG_ERROR. Could you please call __android_log_print instead, how about:
#define API_NOT_SUPPORT_ERROR(API, VERSION) __android_log_print("%s() is only supported when __ANDROID_API__ >= %s.", #API, #VERSION)
@@ -33,3 +37,69 @@ int os_vprintf(const char *fmt, va_list ap) | |||
return __android_log_vprint(ANDROID_LOG_INFO, "wasm_runtime::", fmt, ap); | |||
} | |||
|
|||
#if __ANDROID_API__ < 19 | |||
|
|||
int futimens(int __dir_fd, const struct timespec __times[2]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coding style, please put '{' to the next line:
int futimens(int __dir_fd, const struct timespec __times[2])
{
...
}
|
||
#if __ANDROID_API__ < 21 | ||
|
||
int posix_fallocate(int __fd, off_t __offset, off_t __length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
return -1; | ||
} | ||
|
||
int linkat(int __old_dir_fd, const char *__old_path, int __new_dir_fd, const char *__new_path, int __flags) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coding style, the line is too long, please change it to
int linkat(int __old_dir_fd, const char *__old_path,
int __new_dir_fd, const char *__new_path, int __flags)
{
...
}
|
||
int posix_fadvise(int fd, off_t offset, off_t len, int advice); | ||
|
||
int linkat(int __old_dir_fd, const char *__old_path, int __new_dir_fd, const char *__new_path, int __flags); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, change to:
int linkat(int __old_dir_fd, const char *__old_path,
int __new_dir_fd, const char *__new_path, int __flags);
@@ -11,6 +11,7 @@ set (ANDROID_NDK $ENV{ANDROID_NDK_HOME}) | |||
set (ANDROID_SDK $ENV{ANDROID_SDK_HOME}) | |||
set (ANDROID_ABI "x86") | |||
set (ANDROID_LD lld) | |||
set (ANDROID_PLATFORM 24) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that it isn't used, in fact the macro ANDROID_API is used instead, could we remove this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some reasons make me think that we should "set (ANDROID_PLATFORM 24)" by default.
- refer to Android NDK ANDROID PLATFORM Doc,
seems that we should set "ANDROID_PLATFORM" to determine the value of macro ANDROID_API. - API level 24, equals Android system version 7.0, most of current android system version is larger that 7.0 .
In that situation, wamr libc WASI could supports most of APIs except getrandom().
(if we remove this line, instead we could tell developers that they must config "ANDROID_PLATFORM" if they want to support libc WASI in the WAMR developer Document).
If need, I can remove this line. How do you think? thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, but had better not hardcode it to 24, could you please modify it to
if (NOT DEFINED ANDROID_PLATFORM)
set (ANDROID_PLATFORM 24)
endif ()
So developer can use cmake -DANDROID_PLATFORM=xx
to set the version number if his version number is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll change and test it later.
Enable Android libc wasi support. (#590)
Some libc APIs required by wasi native lib are missing in some Android API versions, only when the version >= 24, all APIs are supported. Add the missing APIs in android platform layer, leave them empty, report error and return failed if they are called. Also update CMakeLists.txt to enable libc wasi by default. Co-authored-by: Wenyong Huang <wenyong.huang@intel.com>
Android WASI support
Hi, I am trying to adapt WASI for Android platform. But there is some build error on Android because some system APIS are
not well supported by some versions of Android system.
So, here is some points to adapt it:
we define some empty functions to avoid build error and also print some errors to notice users.