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

Create target "format" by default only if yaml-cpp is main cmake project #1021

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

OlivierLDff
Copy link

Use Case

I included yaml-cpp with FetchContent.
I don't want to have in my targets, a format target that do not format my main project.

Fix

I introduce cmake option YAML_CPP_ENABLE_FORMAT that is ON by default when yaml-cpp is root project, and OFF otherwise.
Then target format is created if (YAML_CPP_ENABLE_FORMAT AND YAML_CPP_CLANG_FORMAT_EXE)

@jbeder
Copy link
Owner

jbeder commented Apr 1, 2022

Solved by 0e6e98e?

@OlivierLDff
Copy link
Author

Yes but don't you think

cmake_dependent_option(YAML_CPP_FORMAT_SOURCE 
  "Enable format target" ON
  "CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR" OFF)

would make more sense?

It would ease integration with FetchContent, CPM, etc...

@jbeder
Copy link
Owner

jbeder commented Apr 3, 2022

Is that right? There are some other PRs (e.g. #1007) that seem to prefer option instead of cmake_dependent_option.

@OlivierLDff
Copy link
Author

OlivierLDff commented Apr 4, 2022

Oh yes I didn't know cmake_dependent_option couldn't be overridable.

Then following code is the best option:

option(YAML_CPP_FORMAT_SOURCE "Format source" ${YAML_CPP_MAIN_PROJECT})

That would also be consistent with what have been done for: YAML_CPP_INSTALL.

To push it a little bit further even

cmake_dependent_option(YAML_CPP_BUILD_TESTS
  "Enable yaml-cpp tests" ON
  "BUILD_TESTING;CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR" OFF)

should behave that way. Let's say if you want to run yaml-cpp tests along with your project unit test using ctest for example


And don't you think YAML_CPP_ENABLE_FORMAT should become dependent on YAML_CPP_CLANG_FORMAT_EXE

cmake_dependent_option(YAML_CPP_ENABLE_FORMAT 
  "Format source" ${YAML_CPP_MAIN_PROJECT}
  "YAML_CPP_CLANG_FORMAT_EXE" OFF)

or something similar? I'm not sure this would work, I haven't tested this code

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

Successfully merging this pull request may close these issues.

2 participants