Skip to content

Commit

Permalink
Export public directory from Hermes API targets (facebook#694)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#694

The headers under the `public` directory are not exported by our API
targets, which means that something taking a dependency on those
targets won't automatically get the headers.

This diff creates a new `hermesPublic` header-only target to get the
headers exported correctly.

An alternative approach could have been to continue including those
headers in our other targets normally, and then only exporting them at
the API level. This would be more consistent with how the rest of our
build works, since we don't usually rely on include directories
exported from targets. However, this approach would have the drawback
of being  harder to test and maintain, since we don't have testing for
which headers are exported from the API level.

Reviewed By: jpporto

Differential Revision: D34673711

fbshipit-source-id: 5da258ad3ecd7ea72e7c293143f1e5debcfa34a7
  • Loading branch information
neildhar authored and facebook-github-bot committed Mar 26, 2022
1 parent c852306 commit 82cc24d
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 6 deletions.
2 changes: 1 addition & 1 deletion API/hermes/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ add_hermes_library(synthTraceParser SynthTraceParser.cpp LINK_LIBS hermesSupport
set(HERMES_ENABLE_EH OFF)

# compileJS uses neither exceptions nor RTTI
add_hermes_library(compileJS STATIC CompileJS.cpp)
add_hermes_library(compileJS STATIC CompileJS.cpp LINK_LIBS hermesPublic)

# Restore EH and RTTI (Note: At the time of writing, there is no usage of
# add_hermes_library either after this line in this file or in a sub directory.
Expand Down
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,6 @@ include_directories(
include_directories(BEFORE
${CMAKE_CURRENT_BINARY_DIR}/include
${CMAKE_CURRENT_SOURCE_DIR}/include
${CMAKE_CURRENT_SOURCE_DIR}/public
${CMAKE_CURRENT_SOURCE_DIR}/external/flowparser/include
${CMAKE_CURRENT_SOURCE_DIR}/external
)
Expand Down
2 changes: 2 additions & 0 deletions lib/BCGen/HBC/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ add_hermes_library(hermesHBCBackend
hermesInst
hermesSourceMap
hermesAST
hermesPublic
)

add_hermes_library(hermesHBCBackendLean
Expand All @@ -46,6 +47,7 @@ add_hermes_library(hermesHBCBackendLean
UniquingFilenameTable.cpp
LINK_LIBS
hermesSupport
hermesPublic
)

target_compile_definitions(hermesHBCBackendLean PUBLIC HERMESVM_LEAN)
5 changes: 2 additions & 3 deletions lib/Platform/Intl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ add_hermes_library(hermesBCP47Parser STATIC BCP47Parser.cpp)
if(HERMES_ENABLE_INTL)
if(HERMES_IS_ANDROID)
add_hermes_library(hermesPlatformIntl STATIC PlatformIntlAndroid.cpp
LINK_LIBS
fbjni::fbjni
LINK_LIBS fbjni::fbjni hermesPublic
)
target_compile_options(hermesPlatformIntl PRIVATE -frtti -fexceptions)
elseif(APPLE)
Expand All @@ -21,6 +20,6 @@ if(HERMES_ENABLE_INTL)
# Work around a bug in unity builds where it tries to build Obj-C as C++.
set_target_properties(hermesPlatformIntl PROPERTIES UNITY_BUILD false)
else()
add_hermes_library(hermesPlatformIntl STATIC PlatformIntlDummy.cpp)
add_hermes_library(hermesPlatformIntl STATIC PlatformIntlDummy.cpp LINK_LIBS hermesPublic)
endif()
endif()
1 change: 1 addition & 0 deletions lib/VM/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ set(link_libs
hermesSupport
hermesPlatform
hermesInternalBytecode
hermesPublic
dtoa
)

Expand Down
3 changes: 3 additions & 0 deletions public/hermes/Public/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,8 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

add_library(hermesPublic INTERFACE)
target_include_directories(hermesPublic INTERFACE ../../)

install(DIRECTORY "${PROJECT_SOURCE_DIR}/public/hermes/Public" DESTINATION include/hermes
FILES_MATCHING PATTERN "*.h")
2 changes: 1 addition & 1 deletion unittests/API/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ set(SamplingProfilerSources
set(HERMES_LINK_COMPONENTS LLVHSupport)

# Build SegmentTestCompile without EH and RTTI
add_hermes_library(SegmentTestCompile STATIC ${APISegmentTestCompileSources})
add_hermes_library(SegmentTestCompile STATIC ${APISegmentTestCompileSources} LINK_LIBS hermesHBCBackend)

# Turn on EH and RTTI for APITests
set(HERMES_ENABLE_EH ON)
Expand Down
1 change: 1 addition & 0 deletions unittests/Support/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ add_hermes_unittest(HermesSupportTests
target_link_libraries(HermesSupportTests
hermesPlatform
hermesSupport
hermesPublic
dtoa
)

0 comments on commit 82cc24d

Please sign in to comment.