Skip to content

Commit

Permalink
CMake: require CMAKE_BUILD_TYPE
Browse files Browse the repository at this point in the history
Summary:
Instead of choosing defaults, require a build type to be explicitly
specified.

Differential Revision: D66795928
  • Loading branch information
Tzvetan Mikov authored and facebook-github-bot committed Dec 5, 2024
1 parent 48653e7 commit eea666f
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 10 deletions.
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ jobs:
# Tests can take a long time to run, since we don't need to attach a debugger
# a Debug build with with -O2 lets us avoid timeouts and get results sooner.
cmake -S hermes -B build -GNinja -DHERMES_ENABLE_INTL=ON \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_CXX_FLAGS=-O2 -DCMAKE_C_FLAGS=-O2 \
-DHERMESVM_SANITIZE_HANDLES=<< parameters.sanitize >> \
-DHERMES_ENABLE_UNDEFINED_BEHAVIOR_SANITIZER=<< parameters.sanitize >> \
Expand Down Expand Up @@ -67,6 +68,6 @@ jobs:
- run:
name: Run Hermes regression tests
command: |
cmake -S hermes -B build
cmake -S hermes -B build -DCMAKE_BUILD_TYPE=Debug
cmake --build build -j 2
cmake --build build --target check-hermes -j 4
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,5 @@ jobs:
path: hermes
- name: Run Hermes regression tests
run: |-
cmake -S hermes -B build
cmake -S hermes -B build -DCMAKE_BUILD_TYPE=Debug
cmake --build build --target check-hermes all -j 4
10 changes: 5 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ if (POLICY CMP0026)
cmake_policy(SET CMP0026 OLD)
endif()

# Require a build type
if ((NOT GENERATOR_IS_MULTI_CONFIG) AND (NOT DEFINED CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL ""))
message(FATAL_ERROR "Please set CMAKE_BUILD_TYPE")
endif()

# Has to be set before `project` as per documentation
# https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_SYSROOT.html
set(CMAKE_OSX_SYSROOT ${HERMES_APPLE_TARGET_PLATFORM})
Expand Down Expand Up @@ -337,11 +342,6 @@ if (HERMESVM_INTERNAL_JAVASCRIPT_NATIVE)
add_definitions(-DHERMESVM_INTERNAL_JAVASCRIPT_NATIVE)
endif()

# Enable debug mode by default
if ((NOT GENERATOR_IS_MULTI_CONFIG) AND CMAKE_BUILD_TYPE STREQUAL "")
set(CMAKE_BUILD_TYPE Debug)
endif()

if (HERMES_STATIC_LINK)
append("-static" CMAKE_EXE_LINKER_FLAGS)
set(HERMES_USE_STATIC_ICU ON)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ To build a local debug version of the Hermes CLI tools the following steps shoul
mkdir hermes_workingdir
cd hermes_workingdir
git clone https://github.com/facebook/hermes.git
cmake -S hermes -B build -G Ninja
cmake -S hermes -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug
cmake --build ./build
```

Expand Down
4 changes: 2 additions & 2 deletions doc/BuildingAndRunning.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Create a base directory to work in, e.g. `~/workspace`, and cd into it.
After `cd`ing, follow the steps below to generate the Hermes build system:

git clone https://github.com/facebook/hermes.git
cmake -S hermes -B build -G Ninja
cmake -S hermes -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug

The build system has now been generated in the `build` directory. To perform the build:

Expand Down Expand Up @@ -100,7 +100,7 @@ simply run:
The `-HERMES_ENABLE_ADDRESS_SANITIZER=ON` flag will create a ASan build:

git clone https://github.com/facebook/hermes.git
cmake -S hermes -B asan_build -G Ninja -D HERMES_ENABLE_ADDRESS_SANITIZER=ON
cmake -S hermes -B asan_build -G Ninja -D HERMES_ENABLE_ADDRESS_SANITIZER=ON -DCMAKE_BUILD_TYPE=Debug
cmake --build ./asan_build

You can verify the build by looking for `asan` symbols in the `hermes` binary:
Expand Down

0 comments on commit eea666f

Please sign in to comment.