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.

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

[question] cacheVariables data type mismatch in generated CMakePresets.json #14568

Open
1 task done
PengZheng opened this issue Aug 25, 2023 · 2 comments
Open
1 task done

Comments

@PengZheng
Copy link

PengZheng commented Aug 25, 2023

What is your question?

If I define CMake Cache like the following in conanfile.py, CLion's CMake Presets Integration will not work.

         tc.cache_variables["CELIX_ERR_BUFFER_SIZE"] = self.options.celix_err_buffer_size

         v = Version(self.version)
        tc.cache_variables["CELIX_MAJOR"] = v.major.value
        tc.cache_variables["CELIX_MINOR"] = v.minor.value
        tc.cache_variables["CELIX_MICRO"] = v.patch.value

The root cause is analyzed by JetBrains engineer in https://youtrack.jetbrains.com/issue/CPP-34818:

pure technically the project generates preset file which violates cmake-presets specification:

cacheVariables
An optional map of cache variables. The key is the variable name (which may not be an empty string), and the value is either null, a boolean (which is equivalent to a value of "TRUE" or "FALSE" and a type of BOOL), a string representing the value of the variable (which supports macro expansion), or an object with the following fields:

Field might be either null or boolean or string or an object, though in generated presets file the value is a number

"CELIX_ERR_BUFFER_SIZE": 512,

A fix is relatively straight forward:

        tc.cache_variables["CELIX_ERR_BUFFER_SIZE"] = str(self.options.celix_err_buffer_size)

        v = Version(self.version)
        tc.cache_variables["CELIX_MAJOR"] = str(v.major.value)
        tc.cache_variables["CELIX_MINOR"] = str(v.minor.value)
        tc.cache_variables["CELIX_MICRO"] = str(v.patch.value)

I notice Conan's documentation does not mention this data type mismatch issue.
It would be better to avoid this kind of issue at the first place.
That is, Conan guarantees to generate CMakePresets.json compliant to the specification.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded
Copy link
Member

Hi @PengZheng

Conan tries to respect the type of the input. In that regard, it is totally expected and correct to have to str() something that is not a string to be correct. A different story is the management of other types, like bool and int. In that case, it is true that it could benefit from the explicit storage of types inside the generated presets (it doesn't seem that critical, it seems to work with booleans as strings True/False, for example), so lets try to improve that. The plan:

  • Store the type, at least for bools and ints in the generated presets
  • Make sure the CMake command line wrapper is using the right type

@memsharded
Copy link
Member

This will be implemented by #16079

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants