Skip to content

Commit 8b95d9e

Browse files
authored
Fix returning too early when setting options (#11156)
### Summary Fixes a bug where we return too early since we are using a macro. This is only apparent if `set_overridable_option` is used outside of a top-level CMake file (i.e. different scope) ### Test plan unittest + CI
1 parent d9503e6 commit 8b95d9e

File tree

2 files changed

+30
-4
lines changed

2 files changed

+30
-4
lines changed

tools/cmake/common/preset.cmake

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,9 @@ endmacro()
8989
# Set an overridable option.
9090
macro(set_overridable_option NAME VALUE)
9191
# If the user has explitily set the option, do not override it.
92-
if(DEFINED ${NAME})
93-
return()
92+
if(NOT DEFINED ${NAME})
93+
set(${NAME} ${VALUE} CACHE STRING "")
9494
endif()
95-
96-
set(${NAME} ${VALUE} CACHE STRING "")
9795
endmacro()
9896

9997
# Detemine the build preset and load it.

tools/cmake/common/preset_test.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,34 @@ def test_set_overridable_option_after(self):
271271
self.run_cmake()
272272
self.assert_cmake_cache("EXECUTORCH_TEST_MESSAGE", "move fast", "STRING")
273273

274+
def test_set_overridable_option_loaded_from_file(self):
275+
_cmake_lists_txt = """
276+
cmake_minimum_required(VERSION 3.24)
277+
project(test_preset)
278+
include(${PROJECT_SOURCE_DIR}/preset.cmake)
279+
include(${PROJECT_SOURCE_DIR}/build/my_preset.cmake)
280+
include(${PROJECT_SOURCE_DIR}/build/default.cmake)
281+
"""
282+
_my_preset_txt = """
283+
set_overridable_option(EXECUTORCH_FOO "hello world")
284+
"""
285+
_default_preset_txt = """
286+
define_overridable_option(EXECUTORCH_TEST_MESSAGE "test message" STRING "move fast")
287+
define_overridable_option(EXECUTORCH_FOO "another test message" STRING "break things")
288+
"""
289+
self.create_workspace(
290+
{
291+
"CMakeLists.txt": _cmake_lists_txt,
292+
"build": {
293+
"my_preset.cmake": _my_preset_txt,
294+
"default.cmake": _default_preset_txt,
295+
},
296+
}
297+
)
298+
self.run_cmake(cmake_args=["-DEXECUTORCH_TEST_MESSAGE='from the cli'"])
299+
self.assert_cmake_cache("EXECUTORCH_TEST_MESSAGE", "from the cli", "STRING")
300+
self.assert_cmake_cache("EXECUTORCH_FOO", "hello world", "STRING")
301+
274302
def test_set_overridable_option_with_cli_override(self):
275303
_cmake_lists_txt = """
276304
cmake_minimum_required(VERSION 3.24)

0 commit comments

Comments
 (0)