From df653a9dc2341548c2677deb4ce04df16f418196 Mon Sep 17 00:00:00 2001 From: James Ide Date: Mon, 14 Oct 2019 19:18:03 -0700 Subject: [PATCH] More robust hermes-engine lookup logic in makefiles (#26820) Summary: The Android makefiles had hard-coded paths to hermes-engine, sometimes looking in `node_modules` and other times looking in `..`. This commit implements the Node module resolution algorithm (see common.mk), which handles both of these cases and also looks further up the root if necessary, handling the case when the `hermes-engine` npm package is hoisted. This commit does three things: - Defines `find-node-module` and uses it in the makefiles to find `hermes-engine` - Removes the unused `/path/to/hermes-engine/include` paths since this directory does not exist and should be `/path/to/hermes-engine/android/include` (`android`) - Moves the definition of `REACT_NATIVE` in the makefiles to the top. It was defined after every `$(CLEAR_VARS)` invocation but was not actually cleared anyway. `$(CLEAR_VARS)` resets only `LOCAL_*` variables in this list: https://android.googlesource.com/platform/build/+/7dc45a8/core/clear_vars.mk ## Changelog [Internal] [Changed] - Android Makefiles look for hermes-engine using Node's module resolution algorithm Pull Request resolved: https://github.com/facebook/react-native/pull/26820 Test Plan: Run `./gradlew :ReactAndroid:installArchives`, which requires the hermes-engine paths to be correct in order to find the headers. Differential Revision: D17923671 Pulled By: cpojer fbshipit-source-id: 9238b8718a94080db1abbba6375a6a1d484c871d --- .../hermes/instrumentation/Android.mk | 8 ++--- .../facebook/hermes/reactexecutor/Android.mk | 10 +++---- ReactCommon/common.mk | 29 +++++++++++++++++++ ReactCommon/hermes/inspector/Android.mk | 7 +++-- 4 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 ReactCommon/common.mk diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/instrumentation/Android.mk b/ReactAndroid/src/main/java/com/facebook/hermes/instrumentation/Android.mk index 12687995885..8f33762493a 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/instrumentation/Android.mk +++ b/ReactAndroid/src/main/java/com/facebook/hermes/instrumentation/Android.mk @@ -5,15 +5,16 @@ LOCAL_PATH := $(call my-dir) - -include $(CLEAR_VARS) REACT_NATIVE := $(LOCAL_PATH)/../../../../../../../.. +include $(REACT_NATIVE)/ReactCommon/common.mk +include $(CLEAR_VARS) + LOCAL_MODULE := jsijniprofiler LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp) -LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(REACT_NATIVE)/node_modules/hermes-engine/android/include $(REACT_NATIVE)/../hermes-engine/android/include $(REACT_NATIVE)/../node_modules/hermes-engine/include +LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(call find-node-module,$(LOCAL_PATH),hermes-engine)/android/include LOCAL_CPP_FEATURES := exceptions @@ -24,4 +25,3 @@ include $(BUILD_SHARED_LIBRARY) include $(CLEAR_VARS) -REACT_NATIVE := $(LOCAL_PATH)/../../../../../../../.. diff --git a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/Android.mk b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/Android.mk index 50f85946c66..2bf9d26bda3 100644 --- a/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/Android.mk +++ b/ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/Android.mk @@ -4,15 +4,16 @@ # LICENSE file in the root directory of this source tree. LOCAL_PATH := $(call my-dir) - -include $(CLEAR_VARS) REACT_NATIVE := $(LOCAL_PATH)/../../../../../../../.. +include $(REACT_NATIVE)/ReactCommon/common.mk +include $(CLEAR_VARS) + LOCAL_MODULE := hermes-executor-release LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp) -LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(REACT_NATIVE)/node_modules/hermes-engine/android/include $(REACT_NATIVE)/../hermes-engine/android/include $(REACT_NATIVE)/../node_modules/hermes-engine/include +LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(call find-node-module,$(LOCAL_PATH),hermes-engine)/android/include LOCAL_CPP_FEATURES := exceptions @@ -23,14 +24,13 @@ include $(BUILD_SHARED_LIBRARY) include $(CLEAR_VARS) -REACT_NATIVE := $(LOCAL_PATH)/../../../../../../../.. LOCAL_MODULE := hermes-executor-debug LOCAL_CFLAGS := -DHERMES_ENABLE_DEBUGGER=1 LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp) -LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(REACT_NATIVE)/node_modules/hermes-engine/android/include $(REACT_NATIVE)/../hermes-engine/android/include $(REACT_NATIVE)/../node_modules/hermes-engine/include +LOCAL_C_INCLUDES := $(LOCAL_PATH) $(REACT_NATIVE)/ReactCommon/jsi $(call find-node-module,$(LOCAL_PATH),hermes-engine)/android/include LOCAL_CPP_FEATURES := exceptions diff --git a/ReactCommon/common.mk b/ReactCommon/common.mk new file mode 100644 index 00000000000..a7b873a689d --- /dev/null +++ b/ReactCommon/common.mk @@ -0,0 +1,29 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +## +# Returns the absolute path to the specified npm package, searching from the +# given base directory. This function uses Node's module resolution algorithm +# searching "node_modules" in the base directory and its ancestors. If no +# matching package is found, this function returns an empty string. +# +# The first argument to this function is the base directory from which to begin +# searching. The second argument is the name of the npm package. +# +# Ex: $(call find-node-module,$(LOCAL_PATH),hermes-engine) +### +define find-node-module +$(strip \ + $(eval _base := $(strip $(1))) \ + $(eval _package := $(strip $(2))) \ + $(eval _candidate := $(abspath $(_base)/node_modules/$(_package))) \ + $(if $(realpath $(_candidate)), \ + $(_candidate), \ + $(if $(_base), \ + $(call find-node-module,$(patsubst %/,%,$(dir $(_base))),$(_package)) \ + ) \ + ) \ +) +endef diff --git a/ReactCommon/hermes/inspector/Android.mk b/ReactCommon/hermes/inspector/Android.mk index f8fdd43b6bd..a4e66a5076f 100644 --- a/ReactCommon/hermes/inspector/Android.mk +++ b/ReactCommon/hermes/inspector/Android.mk @@ -4,10 +4,11 @@ # LICENSE file in the root directory of this source tree. LOCAL_PATH := $(call my-dir) - -include $(CLEAR_VARS) REACT_NATIVE := $(LOCAL_PATH)/../../.. +include $(REACT_NATIVE)/ReactCommon/common.mk +include $(CLEAR_VARS) + LOCAL_MODULE := hermes-inspector LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp $(LOCAL_PATH)/detail/*.cpp $(LOCAL_PATH)/chrome/*.cpp) @@ -15,7 +16,7 @@ LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp $(LOCAL_PATH)/detail/*.cpp $(L LOCAL_C_ROOT := $(LOCAL_PATH)/../.. LOCAL_CFLAGS := -DHERMES_ENABLE_DEBUGGER=1 -LOCAL_C_INCLUDES := $(LOCAL_C_ROOT) $(REACT_NATIVE)/ReactCommon/jsi $(REACT_NATIVE)/node_modules/hermes-engine/android/include $(REACT_NATIVE)/../hermes-engine/android/include $(REACT_NATIVE)/../node_modules/hermes-engine/include +LOCAL_C_INCLUDES := $(LOCAL_C_ROOT) $(REACT_NATIVE)/ReactCommon/jsi $(call find-node-module,$(LOCAL_PATH),hermes-engine)/android/include LOCAL_EXPORT_C_INCLUDES := $(LOCAL_C_ROOT) LOCAL_CPP_FEATURES := exceptions