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

[vcpkg-cmake] Add check for unused cmake variables #18201

Closed

Conversation

JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented May 31, 2021

In cmake, unused variables are reported as warnings, but in vcpkg, this most likely means that the option names were not updated after the port update.
So I check them and report them here.

If some options only take effect on a specific platform, such as:

if (APPLE)
    options(WITH_XXX_SUPPORT "with xxx support" OFF)
endif()

Use OPTIONS_CHECK_SKIP to ignore them, and cmake regular expressions are supported:

vcpkg_cmake_configure(
    ...
    OPTIONS_CHECK_SKIP "WITH_XXX_SUPPORT"
)

@JackBoosY JackBoosY added info:internal This PR or Issue was filed by the vcpkg team. category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels May 31, 2021
@JackBoosY
Copy link
Contributor Author

Set FATAL_ERROR to WARNING temporary to check all ports.

@JackBoosY JackBoosY requested a review from strega-nil-ms May 31, 2021 07:48
@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label May 31, 2021
@Neumann-A
Copy link
Contributor

Neumann-A commented May 31, 2021

Please exclude VCPKG_* and Z_VCPKG_* variables. I mean the complete set.

@Neumann-A
Copy link
Contributor

Also this doesn't work if a variable is guarded by an if statement and a vcpkg feature.

@JackBoosY
Copy link
Contributor Author

Also this doesn't work if a variable is guarded by an if statement and a vcpkg feature.

This function is only for the internal options in the source.

@JackBoosY JackBoosY marked this pull request as ready for review June 1, 2021 07:00
@JackBoosY JackBoosY requested a review from ras0219-msft June 1, 2021 07:00
@JackBoosY
Copy link
Contributor Author

I've checked the pipeline log, and most of the variables in the warnings are indeed out of date.

@PhoebeHui PhoebeHui added info:reviewed Pull Request changes follow basic guidelines and removed info:reviewed Pull Request changes follow basic guidelines labels Jun 1, 2021
@JackBoosY
Copy link
Contributor Author

Parts of the reported info:

poco: ENABLE_SAMPLES
apr: INSTALL_PRIVATE_H
apr-util: APU_DECLARE_EXPORT;APU_DECLARE_STATIC
libpng: PNG_ARM_NEON
openal-soft: ALSOFT_AMBDEC_PRESETS;ALSOFT_BACKEND_ALSA;ALSOFT_BACKEND_COREAUDIO;ALSOFT_BACKEND_JACK;ALSOFT_BACKEND_OPENSL;ALSOFT_BACKEND_OSS;ALSOFT_BACKEND_PORTAUDIO;ALSOFT_BACKEND_PULSEAUDIO;ALSOFT_BACKEND_QSA;ALSOFT_BACKEND_SNDIO;ALSOFT_BACKEND_SOLARIS;ALSOFT_CONFIG;ALSOFT_CPUEXT_NEON;ALSOFT_HRTF_DEFS;ALSOFT_TESTS
allegro5: WANT_CURL_EXAMPLE;WANT_GLES2
angle: ANGLE_IS_64_BIT_CPU
thrift: WITH_STDTHREADS
arrow: uriparser_SOURCE
assimp: ASSIMP_BUILD_ASSIMP_VIEW
tiff: WITH_ZSTD
leptonica: STATIC
tesseract: USE_SYSTEM_ICU
aubio: HAVE_LIBAV;HAVE_SNDFILE;HAVE_SWRESAMPLE;HAVE_WAVREAD;HAVE_WAVWRITE
zeromq: DISABLE_WS
azure-uamqp-c: build_as_dynamic
azure-uhttp-c: build_as_dynamic
azure-umqtt-c: build_as_dynamic
gtest: BUILD_GTEST
cpprestsdk: WEBSOCKETPP_CONFIG_VERSION
azure-storage-cpp: GETTEXT_LIB_DIR
bond: BOND_ENABLE_COMM
box2d: BUILD_SAMPLES;BUILD_TESTS
leveldb: INSTALL_HEADERS
bullet3: BUILD_DEMOS
caf: CAF_ENABLE_UTILITY_TARGETS;CAF_NO_BENCHMARKS;CAF_NO_CURL_EXAMPLES;CAF_NO_PROTOBUF_EXAMPLES;CAF_NO_QT_EXAMPLES
capstone: CAPSTONE_X86_ONLY
suitesparse: BUILD_METIS;METIS_SOURCE_DIR;SUITESPARSE_INSTALL_PREFIX
lua: SKIP_INSTALL_HEADERS
cartographer: gtest_disable_pthreads
ccfits: CFITSIO_INCLUDE_DIR
celero: CELERO_RUN_EXAMPLE_ON_BUILD
cgns: HDF5_NEEDS_MPI
pybind11: Python3_EXECUTABLE
corrade: WITH_
librdkafka: ENABLE_SHAREDPTR_DEBUG
vtk-m: BUILD_TESTING;VTKm_USE_DEFAULT_TYPES_FOR_VTK
debug-assert: DEBUG_ASSERT_INSTALL
discord-rpc: RAPIDJSONTEST
dlfcn-win32: ENABLE_DEFAULT_PRECISION

@JackBoosY
Copy link
Contributor Author

Depends on #18202.

Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do these changes; the same changes are also necessary for vcpkg_configure_cmake.

Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes requested.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jul 9, 2021
@JackBoosY
Copy link
Contributor Author

Ping for review again and merge this PR.

@strega-nil-ms
Copy link
Contributor

I'll pull this into the next rollup

@strega-nil-ms strega-nil-ms added info:next-rollup and removed category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Jul 13, 2021
@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Jul 15, 2021
@BillyONeal
Copy link
Member

Dropping 'reviewed' since it's marked 'next rollup'

strega-nil-ms pushed a commit to strega-nil/vcpkg that referenced this pull request Jul 19, 2021
[vcpkg-cmake] Add check for unused cmake variables
@strega-nil-ms
Copy link
Contributor

Closed as part of rollup PR #19001

strega-nil-ms added a commit that referenced this pull request Jul 20, 2021
* [rollup:2021-07-16 1/7] PR #18201 (@JackBoosY)

[vcpkg-cmake] Add check for unused cmake variables

* [rollup:2021-07-16 2/7] PR #18397 (@strega-nil)

[vcpkg_list] add new function

* [rollup:2021-07-16 3/7] PR #18782 (@strega-nil)

[scripts-audit] vcpkg_build_ninja

* [rollup:2021-07-16 4/7] PR #18784 (@strega-nil)

[scripts-audit] vcpkg_minimum_required

* [rollup:2021-07-16 5/7] PR #18785 (@strega-nil)

[scripts-audit] vcpkg_replace_string

* [rollup:2021-07-16 6/7] PR #18786 (@strega-nil)

[scripts-audit] windows scripts

* [rollup:2021-07-16 7/7] PR #18945 (@strega-nil)

[many ports] remove deprecated vcpkg_check_features call [1/5]

Co-authored-by: nicole mazzuca <[email protected]>
Co-authored-by: PhoebeHui <[email protected]>
@JackBoosY JackBoosY deleted the dev/jack/check-cmake-ununsed-vars branch July 23, 2021 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants