Add guideline for VCPKG_<LANG>_FLAGS (#7751)

This commit is contained in:
Victor Romero 2019-08-19 12:28:30 -07:00 committed by GitHub
parent 55f574632b
commit 8e7ce6d91a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -1,26 +1,37 @@
# Maintainer Guidelines and Policies
This document lists a set of policies that you should apply when adding or updating a port recipe. It is intended to serve the role of [Debian's Policy Manual](https://www.debian.org/doc/debian-policy/), [Homebrew's Maintainer Guidelines](https://docs.brew.sh/Maintainer-Guidelines), and [Homebrew's Formula Cookbook](https://docs.brew.sh/Formula-Cookbook).
This document lists a set of policies that you should apply when adding or updating a port recipe.
It is intended to serve the role of
[Debian's Policy Manual](https://www.debian.org/doc/debian-policy/),
[Homebrew's Maintainer Guidelines](https://docs.brew.sh/Maintainer-Guidelines), and
[Homebrew's Formula Cookbook](https://docs.brew.sh/Formula-Cookbook).
## PR Structure
### Make separate Pull Requests per port
Whenever possible, separate changes into multiple PR's. This makes them significantly easier to review and prevents issues with one set of changes from holding up every other change.
Whenever possible, separate changes into multiple PRs.
This makes them significantly easier to review and prevents issues with one set of changes from holding up every other change.
### Avoid trivial changes in untouched files
For example, avoid reformatting or renaming variables in portfiles that otherwise have no reason to be modified for the issue at hand. However, if you need to modify the file for the primary purpose of the PR (updating the library), then obviously beneficial changes like fixing typos are appreciated!
For example, avoid reformatting or renaming variables in portfiles that otherwise have no reason to be modified for the issue at hand.
However, if you need to modify the file for the primary purpose of the PR (updating the library),
then obviously beneficial changes like fixing typos are appreciated!
### Check names against other repositories
A good service to check many at once is [Repology](https://repology.org/). If the library you are adding could be confused with another one, consider renaming to make it clear.
A good service to check many at once is [Repology](https://repology.org/).
If the library you are adding could be confused with another one,
consider renaming to make it clear.
### Use GitHub Draft PRs
GitHub Draft PRs are a great way to get CI or human feedback on work that isn't yet ready to merge. Most new PRs should be opened as drafts and converted to normal PRs once the CI passes.
GitHub Draft PRs are a great way to get CI or human feedback on work that isn't yet ready to merge.
Most new PRs should be opened as drafts and converted to normal PRs once the CI passes.
More information about GitHub Draft PRs: https://github.blog/2019-02-14-introducing-draft-pull-requests/
More information about GitHub Draft PRs:
https://github.blog/2019-02-14-introducing-draft-pull-requests/
## Portfiles
@ -28,29 +39,33 @@ More information about GitHub Draft PRs: https://github.blog/2019-02-14-introduc
At this time, the following helpers are deprecated:
1. `vcpkg_extract_archive()` should be replaced by `vcpkg_extract_archive_ex()`
2. `vcpkg_apply_patches()` should be replaced by the `PATCHES` arguments to the "extract" helpers (e.g. `vcpkg_from_github()`)
3. `vcpkg_build_msbuild()` should be replaced by `vcpkg_install_msbuild()`
1. `vcpkg_extract_archive()` should be replaced by [`vcpkg_extract_archive_ex()`](vcpkg_extract_archive_ex.md)
2. `vcpkg_apply_patches()` should be replaced by the `PATCHES` arguments to the "extract" helpers (e.g. [`vcpkg_from_github()`](vcpkg_from_github.md))
3. `vcpkg_build_msbuild()` should be replaced by [`vcpkg_install_msbuild()`](vcpkg_install_msbuild.md)
### Avoid excessive comments in portfiles
Ideally, portfiles should be short, simple, and as declarative as possible. Remove any boiler plate comments introduced by the `create` command before submitting a PR.
Ideally, portfiles should be short, simple, and as declarative as possible.
Remove any boiler plate comments introduced by the `create` command before submitting a PR.
## Build Techniques
### Do not use vendored dependencies
Do not use embedded copies of libraries. All dependencies should be split out and packaged separately so they can be updated and maintained.
Do not use embedded copies of libraries.
All dependencies should be split out and packaged separately so they can be updated and maintained.
### Prefer using CMake
When multiple buildsystems are available, prefer using CMake. Additionally, when appropriate, it can be easier and more maintainable to rewrite alternative buildsystems into CMake using `file(GLOB)` directives.
When multiple buildsystems are available, prefer using CMake.
Additionally, when appropriate, it can be easier and more maintainable to rewrite alternative buildsystems into CMake using `file(GLOB)` directives.
Examples: [abseil](../../ports/abseil/portfile.cmake)
### Choose either static or shared binaries
By default, `vcpkg_configure_cmake()` will pass in the appropriate setting for `BUILD_SHARED_LIBS`, however for libraries that don't respect that variable, you can switch on `VCPKG_LIBRARY_LINKAGE`:
By default, `vcpkg_configure_cmake()` will pass in the appropriate setting for `BUILD_SHARED_LIBS`,
however for libraries that don't respect that variable, you can switch on `VCPKG_LIBRARY_LINKAGE`:
```cmake
string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "static" KEYSTONE_BUILD_STATIC)
@ -67,19 +82,37 @@ vcpkg_configure_cmake(
### When defining features, explicitly control dependencies
When defining a feature that captures an optional dependency, ensure that the dependency will not be used accidentally when the feature is not explicitly enabled. For example:
When defining a feature that captures an optional dependency,
ensure that the dependency will not be used accidentally when the feature is not explicitly enabled.
```cmake
set(CMAKE_DISABLE_FIND_PACKAGE_ZLIB ON)
if("zlib" IN_LIST FEATURES)
set(CMAKE_DISABLE_FIND_PACKAGE_ZLIB OFF)
if ("zlib" IN_LIST FEATURES)
set(CMAKE_DISABLE_FIND_PACKAGE_ZLIB OFF)
else()
set(CMAKE_DISABLE_FIND_PACKAGE_ZLIB ON)
endif()
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA
OPTIONS
-CMAKE_DISABLE_FIND_PACKAGE_ZLIB=${CMAKE_DISABLE_FIND_PACKAGE_ZLIB}
)
```
The snippet below using `vcpkg_check_features()` is equivalent, [see the documentation](vcpkg_check_features.md).
```cmake
vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
INVERTED_FEATURES
"zlib" CMAKE_DISABLE_FIND_PACKAGE_ZLIB
)
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA
OPTIONS
-DCMAKE_DISABLE_FIND_PACKAGE_ZLIB=${CMAKE_DISABLE_FIND_PACKAGE_ZLIB}
${FEATURE_OPTIONS}
)
```
@ -125,6 +158,22 @@ Common options that allow avoiding patching:
2. [CMAKE] Calls to `find_package(XYz)` in CMake scripts can be disabled via [`-DCMAKE_DISABLE_FIND_PACKAGE_XYz=ON`](https://cmake.org/cmake/help/v3.15/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html)
3. [CMAKE] Cache variables (declared as `set(VAR "value" CACHE STRING "Documentation")` or `option(VAR "Documentation" "Default Value")`) can be overriden by just passing them in on the command line as `-DVAR:STRING=Foo`. One notable exception is if the `FORCE` parameter is passed to `set()`. See also the [CMake `set` documentation](https://cmake.org/cmake/help/v3.15/command/set.html)
### Prefer patching over overriding `VCPKG_<VARIABLE>` values
Some variables prefixed with `VCPKG_<VARIABLE>` have an equivalent `CMAKE_<VARIABLE>`.
However, not all of them are passed to the internal package build [(see implementation: Windows toolchain)](../../scripts/toolchains/windows.cmake).
Consider the following example:
```cmake
set(VCPKG_C_FLAGS "-O2 ${VCPKG_C_FLAGS}")
set(VCPKG_CXX_FLAGS "-O2 ${VCPKG_CXX_FLAGS}")
```
Using `vcpkg`'s built-in toolchains this works, because the value of `VCPKG_<LANG>_FLAGS` is forwarded to the appropriate `CMAKE_LANG_FLAGS` variable. But, a custom toolchain that is not aware of `vcpkg`'s variables will not forward them.
Because of this, it is preferable to patch the buildsystem directly when setting `CMAKE_<LANG>_FLAGS`.
### Minimize patches
When making changes to a library, strive to minimize the final diff. This means you should _not_ reformat the upstream source code when making changes that affect a region. Also, when disabling a conditional, it is better to add a `AND FALSE` or `&& 0` to the condition than to delete every line of the conditional.