-
Notifications
You must be signed in to change notification settings - Fork 8
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
lib/lib-wamr: Move to musl and address compiler errors #8
Conversation
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.
Good job, @R0mbertus! 🔝
Please see my comments below.
return BHT_ERROR; | ||
|
||
- pthread_mutexattr_settype(&mattr, PTHREAD_MUTEX_RECURSIVE_NP); | ||
+ pthread_mutexattr_settype(&mattr, PTHREAD_MUTEX_RECURSIVE); |
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.
Why do you need this change?
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.
This change is needed as it would otherwise not compile as PTHREAD_MUTEX_RECURSIVE_NP
is not defined anywhere in musl pthread library, but PTHREAD_MUTEX_RECURSIVE
is.
|
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.
Could you rebase your 2 commits into a single one?
I will approve after that.
They've been rebased into one, and the offsetoff does work properly now. |
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.
@R0mbertus Please add a Co-authored-by
tag for the co-author (along with the present sign-off-bt's).
Also please update the commit message to state the changes that were necesarry in order for libwamr to build (and there is a weird S
letter at the end of the commit message, after the sign-off-by tags).
Changed the commit message to include a list of changes needed to build |
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.
Great work @R0mbertus, please wrap your commit message to ~80 characters per line, I'll approve it after that.
Move from the `LIBNEWLIBC` and `LIBPTHREAD_EMBEDDED` to `LIBMUSL`. Changed needed to build lib-wamr were: * Using the `uk/essentials.h` header for a definition of `offsetof` * Adding a new `0006` patch file to change `PTHREAD_MUTEX_RECURSIVE_NP` into `PTHREAD_MUTEX_RECURSIVE` as the former on isn't defined * Change the `0005` patch file to use modern working unikraft initrd loading code so it can load the `.wasm` files Signed-off-by: Robert Klink <roberthklink@gmail.com> Signed-off-by: Ricardo Mohamedhoesein <rmohamedhoesein3@gmail.com> Co-authored-by: Ricardo Mohamedhoesein <rmohamedhoesein3@gmail.com>
Commit messages for both this PR and the one for |
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.
All good, thanks.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com
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.
Looks good. Thanks @R0mbertus!
Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com
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.
All good now. Thanks a lot, @R0mbertus!
Reviewed-by: Radu Nichita radunichita99@gmail.com
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.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com
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.
Approved-by: Razvan Deaconescu razvand@unikraft.io
This PR addresses issues #3, #4, #6 and #7. It does this by:
offsetof
declaration to theinclude/bh_platform.h
(issue Buiding Error inwasm_interp.h
#3)0005-...
patch file to work with modern unikraft (issue magic header not detected #4)0006-...
patch file that removes the_NP
so that it works (issue Error onPTHREAD_MUTEX_RECURSIVE_NP
, not defined #6)