-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
pcre2: Remove unused sjlit files after last update #89495
pcre2: Remove unused sjlit files after last update #89495
Conversation
58579a6
to
e0617af
Compare
@@ -104,6 +104,7 @@ static SLJIT_INLINE void apple_update_wx_flags(sljit_s32 enable_exec) | |||
#else | |||
#define SLJIT_MAP_JIT (0) | |||
#endif | |||
#define SLJIT_UPDATE_WX_FLAGS(from, to, enable_exec) |
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 don't know if it's a correct fix, or why it doesn't fail on GitHub Actions with Xcode in the first place.
With ioscross, we don't fulfill the TARGET_OS_MAC && !TARGET_OS_IPHONE
condition above (which sounds logical), but then SLJIT_UPDATE_WX_FLAGS
is never defined.
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 right, as to why it was not detected by GHA - no idea.
We might want to file an issue @ https://github.com/PCRE2Project/pcre2 just in case - if it's a bug it'll be (hopefully) fixed, if not - we might learn what we did wrong (I suspect it's more likely to be option № 1).
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 is incorrect; iOS needs an implementation for this function and so the build breaking (or showing a warning that says one is missing) is more accurate.
So maybe not right after all 😩
Source: PCRE2Project/pcre2#251 (comment)
So we should likely try to disable building the jit compiler altogether on iOS. Edit: #89507 does that. I'll repurpose this PR to just remove the unused files. |
e0617af
to
739fcd1
Compare
Thanks, and sorry for the regression 😬 |
Follow-up to #89371.
CC @Chubercik