-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Static link for hermes-inspector #32694
Static link for hermes-inspector #32694
Conversation
cc @cortinico |
|
Base commit: 2bb91ae |
Base commit: 2bb91ae |
@ShikaSD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@@ -40,7 +40,7 @@ LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(call find-no | |||
|
|||
LOCAL_CPP_FEATURES := exceptions | |||
|
|||
LOCAL_STATIC_LIBRARIES := libjsireact libhermes-executor-common-debug libhermes-inspector | |||
LOCAL_STATIC_LIBRARIES := libjsireact libhermes-executor-common-debug |
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 believe removing it here is incorrect, as hermes-inspector is only used by debug flavour of executor
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.
From my understanding libhermes-inspector
is linked into libhermes-executor-common-debug
already (
LOCAL_STATIC_LIBRARIES := libjsireact libhermes-inspector |
libhermes-inspector
directly (no <hermes/inspector/*>
in ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor
).
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.
Ah, I see, makes sense :)
This pull request was successfully merged by @janicduplessis in 20b0eba. When will my fix make it into a release? | Upcoming Releases |
Summary
Follow up to #32683 to also link hermes-inspector statically.
Changelog
[Android] [Fix] - Static link for hermes-inspector
Test Plan
Tested a clean build and made sure hermes-inspector.so doesn't exist anymore.