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
Set cmake compile options based on language #7600
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Some tests that check generated CMake code are broken as expected.
@@ -96,7 +97,8 @@ class CMakeFindPackageGenerator(Generator): | |||
set({{ pkg_name }}_{{ comp_name }}_RES_DIRS {{ comp.res_paths }}) | |||
set({{ pkg_name }}_{{ comp_name }}_DEFINITIONS {{ comp.defines }}) | |||
set({{ pkg_name }}_{{ comp_name }}_COMPILE_DEFINITIONS {{ comp.compile_definitions }}) | |||
set({{ pkg_name }}_{{ comp_name }}_COMPILE_OPTIONS_LIST "{{ comp.cxxflags_list }}" "{{ comp.cflags_list }}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing this variable when using components, as this feature is marked as experimental
@@ -15,6 +15,8 @@ | |||
) | |||
set({name}_COMPILE_DEFINITIONS{build_type_suffix} {deps.compile_definitions}) | |||
set({name}_COMPILE_OPTIONS{build_type_suffix}_LIST "{deps.cxxflags_list}" "{deps.cflags_list}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I kept the variable (although it is not used in targets) to avoid breaking
@@ -119,7 +127,8 @@ class CMakeFindPackageMultiGenerator(CMakeFindPackageGenerator): | |||
set({{ pkg_name }}_{{ comp_name }}_RES_DIRS_{{ build_type }} {{ comp.res_paths }}) | |||
set({{ pkg_name }}_{{ comp_name }}_DEFINITIONS_{{ build_type }} {{ comp.defines }}) | |||
set({{ pkg_name }}_{{ comp_name }}_COMPILE_DEFINITIONS_{{ build_type }} {{ comp.compile_definitions }}) | |||
set({{ pkg_name }}_{{ comp_name }}_COMPILE_OPTIONS_LIST_{{ build_type }} "{{ comp.cxxflags_list }}" "{{ comp.cflags_list }}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed variable in the components flavour
@@ -486,14 +487,6 @@ def build(self): | |||
client.out) | |||
|
|||
def cpp_info_filename_test(self): | |||
def add_to_conan_file(after, add_lines, spaces_to_indent): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed code not used
I see the following errors in Windows:
I think we cannot implement it this way |
After trying to continue with this using the project language as described here https://stackoverflow.com/questions/32389273/detect-project-language-in-cmake, implementing this feels hacky to me. First, we would need to extract the information of the language to a new variable with Then, using a generator expression with To conclude, I am not sure if we should support this flag differentiation while creating the targets as CMake does not provide a feature supported in the main compilers. We can, however, expose the flags in variables without mixing them and let the consumers use that information in the way they want. |
Changelog: Fix: Set CMake targets compile options based on language
Docs: omit
#tags: slow
develop
branch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.