-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(main/luvi): fix incompatible bytecode error #22034
Conversation
From PR: #21722 |
a1f0146
to
3739b98
Compare
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.
Thanks for the fix! Please see comments/questions
packages/luvi/build.sh
Outdated
@@ -17,36 +17,29 @@ TERMUX_PKG_EXTRA_CONFIGURE_ARGS=" | |||
-DWithPCRE2=On | |||
-DWithSharedPCRE2=On | |||
-DWithLPEG=On | |||
-DLIBLUV_INCLUDE_DIR=${TERMUX_PKG_SRCDIR}/deps/luv/src | |||
-DLIBLUV_LIBRARIES=${TERMUX_PREFIX}/lib/libluv.so |
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.
Generally speaking the headers and the library should come from the same place/the same version. Why use -DLIBLUV_INCLUDE_DIR=${TERMUX_PKG_SRCDIR}/deps/luv/src
and not -DLIBLUV_INCLUDE_DIR=${TERMUX_PREFIX/include/luv
?
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.
I've tried to do that when trying to submit this package, it somewhat failed and I have no idea why. But I'll try to add it back.
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.
I think we risk issues if version that is stored in luvi's deps/luv/src/ deviates (or starts to deviate in the future) from version that is stored in $PREFIX/, would be great if we could use $TERMUX_PREFIX/include/luv instead, so please check what error it gives
765b2e1
to
80bd292
Compare
Welp I tried to fix the branch being messed up, and messed it up even more by forgetting to commit. |
I might be able to fix it by reopening the PR. Which I can't do because I re-pushed the branch with 0 commits... |
Superseded by #22053 |
This PR also bumps the commit used because previously the commit it was referencing did not exist.