-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[rollup:2021-07-06] Rollup PR #18838
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
Changes from all commits
dba4871
f9a2aa7
27e7e57
00a7acd
4831206
524497f
959d5fa
8357b22
11c0a6c
ec62ce0
f7f9e48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # CMake Guidelines | ||
|
|
||
| We expect that all CMake scripts that are either: | ||
|
|
||
| - In the `scripts/` directory, or | ||
| - In a `vcpkg-*` port | ||
|
|
||
| should follow the guidelines laid out in this document. | ||
| Existing scripts may not follow these guidelines yet; | ||
| it is expected that we will continue to update old scripts | ||
| to fall in line with these guidelines. | ||
|
|
||
| These guidelines are intended to create stability in our scripts. | ||
| We hope that they will make both forwards and backwards compatibility easier. | ||
|
|
||
| ## The Guidelines | ||
|
|
||
| - Except for out-parameters, we always use `cmake_parse_arguments()` | ||
| rather than function parameters or referring to `${ARG<N>}`. | ||
| - This doesn't necessarily need to be followed for "script-local helper functions" | ||
| - In this case, positional parameters should be put in the function | ||
| declaration (rather than using `${ARG<N>}`), | ||
| and should be named according to local rules (i.e. `snake_case`). | ||
| - Exception: positional parameters that are optional should be | ||
| given a name via `set(argument_name "${ARG<N>}")`, after checking `ARGC`. | ||
| - Out-parameters should be the first parameter to a function. Example: | ||
| ```cmake | ||
| function(format out_var) | ||
| cmake_parse_arguments(PARSE_ARGV 1 "arg" ...) | ||
| # ... set(buffer "output") | ||
| set("${out_var}" "${buffer}" PARENT_SCOPE) | ||
| endfunction() | ||
| ``` | ||
| - There are no unparsed or unused arguments. | ||
| Always check for `ARGN` or `arg_UNPARSED_ARGUMENTS`. | ||
| `FATAL_ERROR` when possible, `WARNING` if necessary for backwards compatibility. | ||
| - All `cmake_parse_arguments` must use `PARSE_ARGV`. | ||
| - All `foreach` loops must use `IN LISTS` and `IN ITEMS`. | ||
| - The variables `${ARGV}` and `${ARGN}` are unreferenced, | ||
| except in helpful messages to the user. | ||
| - (i.e., `message(FATAL_ERROR "blah was passed extra arguments: ${ARGN}")`) | ||
| - We always use functions, not macros or top level code. | ||
| - Exception: "script-local helper macros". It is sometimes helpful to define a small macro. | ||
| This should be done sparingly, and functions should be preferred. | ||
| - Exception: `vcpkg.cmake`'s `find_package`. | ||
| - Scripts in the scripts tree should not be expected to need observable changes | ||
| as part of normal operation. | ||
| - Example violation: `vcpkg_acquire_msys()` has hard-coded packages and versions that need updating over time due to the MSYS project dropping old packages. | ||
| - Example exception: `vcpkg_from_sourceforge()` has a list of mirrors which needs maintenance but does not have an observable behavior impact on the callers. | ||
| - All variable expansions are in quotes `""`, | ||
| except those which are intended to be passed as multiple arguments. | ||
| - Example: | ||
| ```cmake | ||
| set(working_directory "") | ||
| if(DEFINED arg_WORKING_DIRECTORY) | ||
| set(working_directory "WORKING_DIRECTORY" "${arg_WORKING_DIRECTORY}") | ||
| endif() | ||
| # calls do_the_thing() if NOT DEFINED arg_WORKING_DIRECTORY, | ||
| # else calls do_the_thing(WORKING_DIRECTORY "${arg_WORKING_DIRECTORY}") | ||
| do_the_thing(${working_directory}) | ||
| ``` | ||
| - There are no "pointer" or "in-out" parameters | ||
| (where a user passes a variable name rather than the contents), | ||
| except for simple out-parameters. | ||
| - Variables are not assumed to be empty. | ||
| If the variable is intended to be used locally, | ||
| it must be explicitly initialized to empty with `set(foo "")`. | ||
| - All variables expected to be inherited from the parent scope across an API boundary (i.e. not a file-local function) should be documented. Note that all variables mentioned in triplets.md are considered documented. | ||
| - Out parameters are only set in `PARENT_SCOPE` and are never read. | ||
| See also the helper `z_vcpkg_forward_output_variable()` to forward out parameters through a function scope. | ||
| - `CACHE` variables are used only for global variables which are shared internally among strongly coupled | ||
| functions and for internal state within a single function to avoid duplicating work. | ||
| These should be used extremely sparingly and should use the `Z_VCPKG_` prefix to avoid | ||
| colliding with any local variables that would be defined by any other code. | ||
| - Examples: | ||
| - `vcpkg_cmake_configure`'s `Z_VCPKG_CMAKE_GENERATOR` | ||
| - `z_vcpkg_get_cmake_vars`'s `Z_VCPKG_GET_CMAKE_VARS_FILE` | ||
| - `include()`s are only allowed in `ports.cmake` or `vcpkg-port-config.cmake`. | ||
| - `foreach(RANGE)`'s arguments _must always be_ natural numbers, | ||
| and `<start>` _must always be_ less than or equal to `<stop>`. | ||
| - This must be checked by something like: | ||
| ```cmake | ||
| if(start LESS_EQUAL end) | ||
| foreach(RANGE start end) | ||
| ... | ||
| endforeach() | ||
| endif() | ||
| ``` | ||
| - All port-based scripts must use `include_guard(GLOBAL)` | ||
| to avoid being included multiple times. | ||
| - `set(var)` should not be used. Use `unset(var)` to unset a variable, | ||
| and `set(var "")` to set it to the empty value. _Note: this works for use as a list and as a string_ | ||
|
|
||
| ### CMake Versions to Require | ||
|
|
||
| - All CMake scripts, except for `vcpkg.cmake`, | ||
| may assume the version of CMake that is present in the | ||
| `cmake_minimum_required` of `ports.cmake`. | ||
| - This `cmake_minimum_required` should be bumped every time a new version | ||
| of CMake is added to `vcpkgTools.xml`, as should the | ||
| `cmake_minimum_required` in all of the helper `CMakeLists.txt` files. | ||
| - `vcpkg.cmake` must assume a version of CMake back to 3.1 in general | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @strega-nil In the context of #18898, I created a test port for the minimum cmake version. $ git grep -l "string(APPEND" scripts/toolchains
scripts/toolchains/android.cmake
scripts/toolchains/freebsd.cmake
scripts/toolchains/ios.cmake
scripts/toolchains/linux.cmake
scripts/toolchains/mingw.cmake
scripts/toolchains/openbsd.cmake
scripts/toolchains/osx.cmake
scripts/toolchains/windows.cmake
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It even nobody complained that it is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, okay. This might argue for updating our minimum supported version to 3.4, since we haven't heard complaints.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I did. And I would have complained earlier if the vcpkg ports I need would be mature enough for usage on Ubuntu 18.04 / CMake 3.10. |
||
| - Specific functions and options may assume a greater CMake version; | ||
| if they do, make sure to comment that function or option | ||
| with the required CMake version. | ||
|
|
||
|
|
||
| ### Changing Existing Functions | ||
|
|
||
| - Never remove arguments in non-internal functions; | ||
| if they should no longer do anything, just take them as normal and warn on use. | ||
| - Never add a new mandatory argument. | ||
|
|
||
| ### Naming Variables | ||
|
|
||
| - `cmake_parse_arguments`: set prefix to `"arg"` | ||
| - Local variables are named with `snake_case` | ||
| - Internal global variable names are prefixed with `Z_VCPKG_`. | ||
| - External experimental global variable names are prefixed with `X_VCPKG_`. | ||
|
|
||
| - Internal functions are prefixed with `z_vcpkg_` | ||
| - Functions which are internal to a single function (i.e., helper functions) | ||
| are named `[z_]<func>_<name>`, where `<func>` is the name of the function they are | ||
| a helper to, and `<name>` is what the helper function does. | ||
| - `z_` should be added to the front if `<func>` doesn't have a `z_`, | ||
| but don't name a helper function `z_z_foo_bar`. | ||
| - Public global variables are named `VCPKG_`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # z_vcpkg_forward_output_variable | ||
|
|
||
| The latest version of this document lives in the [vcpkg repo](https://github.com/Microsoft/vcpkg/blob/master/docs/). | ||
|
|
||
| This macro helps with forwarding values from inner function calls, | ||
| through a local function scope, into pointer out parameters. | ||
|
|
||
| ```cmake | ||
| z_vcpkg_forward_output_variable(ptr_to_parent_var var_to_forward) | ||
| ``` | ||
|
|
||
| is equivalent to | ||
|
|
||
| ```cmake | ||
| if(DEFINED ptr_to_parent_var) | ||
| if(DEFINED value_var) | ||
| set("${ptr_to_parent_var}" "${value_var}" PARENT_SCOPE) | ||
| else() | ||
| unset("${ptr_to_parent_var}" PARENT_SCOPE) | ||
| endif() | ||
| endif() | ||
| ``` | ||
|
|
||
| Take note that the first argument should be a local variable that has a value of the parent variable name. | ||
| Most commonly, this local is the result of a pointer-out parameter to a function. | ||
| If the variable in the first parameter is not defined, this function does nothing, | ||
| simplifying functions with optional out parameters. | ||
| Most commonly, this should be used in cases like: | ||
|
|
||
| ```cmake | ||
| function(my_function out_var) | ||
| file(SHA512 "somefile.txt" local_var) | ||
| z_vcpkg_forward_output_variable(out_var local_var) | ||
| endfunction() | ||
| ``` | ||
|
|
||
| ## Source | ||
| [scripts/cmake/z\_vcpkg\_forward\_output\_variable.cmake](https://github.com/Microsoft/vcpkg/blob/master/scripts/cmake/z_vcpkg_forward_output_variable.cmake) |
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.
NB: we believe this is a bug. No change requested.