Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #16078 - add type to CMakePresets cacheVariables #16079

Closed
wants to merge 4 commits into from
Closed

Fix #16078 - add type to CMakePresets cacheVariables #16079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b56582b
Fix #16078 - add type to cmakepresets cacheVariables
jmarrec Apr 15, 2024
dbbb1c3
adjust test to include the type
jmarrec Apr 15, 2024
3730b75
Merge branch 'develop2' into 16078_cacheVariables_types
memsharded Apr 16, 2024
11c127b
fix test
memsharded Apr 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 15 additions & 5 deletions conan/tools/cmake/presets.py
View file
Open in desktop
Expand Up @@ -50,7 +50,7 @@ def generate(conanfile, toolchain_file, generator, cache_variables, preset_prefi

if "BUILD_TESTING" not in cache_variables:
if conanfile.conf.get("tools.build:skip_test", check_type=bool):
cache_variables["BUILD_TESTING"] = "OFF"
cache_variables["BUILD_TESTING"] = {"value": "OFF", "type": "BOOL"}

preset_path = os.path.join(conanfile.generators_folder, "CMakePresets.json")
multiconfig = is_multi_configuration(generator)
Expand Down Expand Up @@ -122,7 +122,7 @@ def _configure_preset(conanfile, generator, cache_variables, toolchain_file, mul
if preset_prefix:
name = f"{preset_prefix}-{name}"
if not multiconfig and build_type:
cache_variables["CMAKE_BUILD_TYPE"] = build_type
cache_variables["CMAKE_BUILD_TYPE"] = {"value": build_type, "type": "STRING"}
ret = {
"name": name,
"displayName": "'{}' config".format(name),
Expand Down Expand Up @@ -164,12 +164,22 @@ def _configure_preset(conanfile, generator, cache_variables, toolchain_file, mul
ret["binaryDir"] = conanfile.build_folder

def _format_val(val):
return f'"{val}"' if type(val) == str and " " in val else f"{val}"
return f'"{val}"' if isinstance(val, str) and " " in val else f"{val}"

def _format_var(var, value_or_dict):
type_str = ""
if isinstance(value_or_dict, dict):
value = value_or_dict["value"]
type_str = f":{value_or_dict['type']}"
else:
value = value_or_dict

return f"-D{var}{type_str}={_format_val(value)}"

# https://github.com/conan-io/conan/pull/12034#issuecomment-1253776285
cache_variables_info = " ".join(
[f"-D{var}={_format_val(value)}" for var, value in cache_variables.items()])
add_toolchain_cache = f"-DCMAKE_TOOLCHAIN_FILE={toolchain_file} " \
[_format_var(var, value) for var, value in cache_variables.items()])
add_toolchain_cache = f"-DCMAKE_TOOLCHAIN_FILE:FILEPATH={toolchain_file} " \
Comment on lines +167 to +182
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Properly format the message that is printed as well:

conanfile.py: If your CMake version is not compatible
with CMakePresets (<3.23) call cmake like: 
'cmake <path> -G Ninja 
 -DCMAKE_TOOLCHAIN_FILE:FILEPATH=/media/path/to/conan_toolchain.cmake
 -DMY_BOOL:BOOL=ON 
 -DMY_FILE_PATH:FILEPATH=/path/to/file 
 -DMY_DIR_PATH:PATH=/path/to/dir

if "CMAKE_TOOLCHAIN_FILE" not in cache_variables_info else ""

try:
Expand Down
14 changes: 11 additions & 3 deletions conan/tools/cmake/toolchain/toolchain.py
View file
Open in desktop
@@ -1,4 +1,5 @@
import os
from pathlib import Path
import textwrap
from collections import OrderedDict

Expand Down Expand Up @@ -240,14 +241,21 @@ def generate(self):
cache_variables = {}
for name, value in self.cache_variables.items():
if isinstance(value, bool):
cache_variables[name] = "ON" if value else "OFF"
cache_variables[name] = {"value": "ON" if value else "OFF", "type": "BOOL"}
elif isinstance(value, Path):
cache_variables[name] = {
"value": str(value),
"type": "FILEPATH" if value.is_file() else 'PATH'
}
elif isinstance(value, str):
cache_variables[name] = {"value": value, "type": "STRING"}
elif isinstance(value, _PackageOption):
if str(value).lower() in ["true", "false", "none"]:
cache_variables[name] = "ON" if bool(value) else "OFF"
cache_variables[name] = {"value": "ON" if bool(value) else "OFF", "type": "BOOL"}
elif str(value).isdigit():
cache_variables[name] = int(value)
else:
cache_variables[name] = str(value)
cache_variables[name] = {"value": str(value), "type": "STRING"}
Comment on lines +244 to +258
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace cache_variables with an object with both value and type where appropriate

Test example:

tc = CMakeToolchain(self)
tc.cache_variables["MY_BOOL"] = False
tc.cache_variables["MY_FILE_PATH"] = Path("/path/to/file")   # assert Path("/path/to/file").is_file()
tc.cache_variables["MY_FILE_PATH"] = Path("/path/to/dir")   # assert Path("/path/to/file").is_dir()
tc.generate()

else:
cache_variables[name] = value

Expand Down
3 changes: 2 additions & 1 deletion conans/test/integration/configuration/profile_test.py
View file
Open in desktop
Expand Up @@ -524,7 +524,8 @@ def configure(self):
# Verify the cmake toolchain takes Debug
assert "I'm dep and my shared is False" in client.out
presets = json.loads(client.load("CMakePresets.json"))
assert presets["configurePresets"][0]["cacheVariables"]['CMAKE_BUILD_TYPE'] == "Debug"
expected = {"value": "Debug", "type": "STRING"}
assert presets["configurePresets"][0]["cacheVariables"]['CMAKE_BUILD_TYPE'] == expected


def test_create_and_priority_of_consumer_specific_setting():
Expand Down
37 changes: 26 additions & 11 deletions conans/test/integration/toolchains/cmake/test_cmaketoolchain.py
View file
Open in desktop
@@ -1,5 +1,6 @@
import json
import os
from pathlib import Path
import platform
import textwrap

Expand Down Expand Up @@ -663,6 +664,7 @@ def test_cmake_presets_singleconfig():
def test_toolchain_cache_variables():
client = TestClient()
conanfile = textwrap.dedent("""
from pathlib import Path
from conan import ConanFile
from conan.tools.cmake import CMakeToolchain

Expand All @@ -681,6 +683,8 @@ def generate(self):
toolchain.cache_variables["NUMBER"] = self.options.number
toolchain.cache_variables["CMAKE_SH"] = "THIS VALUE HAS PRIORITY"
toolchain.cache_variables["CMAKE_POLICY_DEFAULT_CMP0091"] = "THIS VALUE HAS PRIORITY"
toolchain.cache_variables["MY_FILE_PATH"] = Path(__file__).resolve()
toolchain.cache_variables["MY_DIR_PATH"] = Path(__file__).resolve().parent
toolchain.cache_variables["CMAKE_MAKE_PROGRAM"] = "THIS VALUE HAS NO PRIORITY"
toolchain.generate()
""")
Expand All @@ -692,23 +696,34 @@ def generate(self):

presets = json.loads(client.load("CMakePresets.json"))
cache_variables = presets["configurePresets"][0]["cacheVariables"]
assert cache_variables["foo"] == 'ON'
assert cache_variables["foo2"] == 'OFF'
assert cache_variables["var"] == '23'
assert cache_variables["CMAKE_SH"] == "THIS VALUE HAS PRIORITY"
assert cache_variables["CMAKE_POLICY_DEFAULT_CMP0091"] == "THIS VALUE HAS PRIORITY"
assert cache_variables["foo"] == {'value': 'ON', 'type': 'BOOL'}
assert cache_variables["foo2"] == {'value': 'OFF', 'type': 'BOOL'}
assert cache_variables["var"] == {'value': '23', 'type': 'STRING'}
assert cache_variables["CMAKE_SH"] == {'value': "THIS VALUE HAS PRIORITY", 'type': 'STRING'}
assert cache_variables["CMAKE_POLICY_DEFAULT_CMP0091"] == {"value": "THIS VALUE HAS PRIORITY", 'type': 'STRING'}
assert cache_variables["CMAKE_MAKE_PROGRAM"] == "MyMake"
assert cache_variables["BUILD_TESTING"] == 'OFF'
assert cache_variables["ENABLE_FOOBAR"] == 'ON'
assert cache_variables["QUX"] == 'baz'
assert cache_variables["BUILD_TESTING"] == {'value': 'OFF', 'type': 'BOOL'}
assert cache_variables["ENABLE_FOOBAR"] == {'value': 'ON', 'type': 'BOOL'}
assert cache_variables["QUX"] == {'value': 'baz', 'type': 'STRING'}
assert cache_variables["NUMBER"] == 1
assert cache_variables["MY_FILE_PATH"] == {'value': str(Path(client.current_folder).resolve() / 'conanfile.py'), 'type': 'FILEPATH'}
assert cache_variables["MY_DIR_PATH"] == {'value': str(Path(client.current_folder).resolve()), 'type': 'PATH'}

def _format_val(val):
return f'"{val}"' if type(val) == str and " " in val else f"{val}"
return f'"{val}"' if isinstance(val, str) and " " in val else f"{val}"

def _format_var(var, value_or_dict):
type_str = ""
if isinstance(value_or_dict, dict):
value = value_or_dict["value"]
type_str = f":{value_or_dict['type']}"
else:
value = value_or_dict
return f"-D{var}{type_str}={_format_val(value)}"

for var, value in cache_variables.items():
assert f"-D{var}={_format_val(value)}" in client.out
assert "-DCMAKE_TOOLCHAIN_FILE=" in client.out
assert _format_var(var, value) in client.out
assert "-DCMAKE_TOOLCHAIN_FILE:FILEPATH=" in client.out
assert f"-G {_format_val('MinGW Makefiles')}" in client.out

client.run("install . --name=mylib --version=1.0 -c tools.gnu:make_program='MyMake'")
Expand Down