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

cmake: restore cmake args list in buildinfo.txt #15563

Closed
wants to merge 3 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 13, 2024

This feature was recently dropped because of a bad side-effect of
silencing unused cmake command-line option warnings.

Fix this issue by retrieving variable values using get_property(),
instead of accessing the variables directly. It allows restoring
this feature without the bad side-effect.

Also limit the logic to CI runs.

Follow-up to 96edb5f #15501

This feature was recently dropped because of a bad side-effect of
silencing unused cmake command-line warnings.

Fix this issue by retrieving variable values using `get_property()`,
instead of accessing the variables directly. It allows restoring
this feature without the bad side-effect.

Follow-up to 96edb5f curl#15501
@vszakats vszakats added the cmake label Nov 13, 2024
@vszakats
Copy link
Member Author

/cc @dg0yt

@github-actions github-actions bot added the build label Nov 13, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Nov 13, 2024

Did you test how much this writes on CMake re-run? I would assume it includes all variables which are also in CMakeCache.txt, i.e. all the cache variables from find_library and friends.

@vszakats
Copy link
Member Author

vszakats commented Nov 13, 2024

The only place the result appears is when CI is detected, and re-runs are typically not done there. The unnecessary work is still bad, so how about I'd limit this to CI runs like already done for buildinfo.txt generation?

If still not good we might limit it further to curl CI runs.

@vszakats
Copy link
Member Author

vszakats commented Nov 13, 2024

Running an initial out-of-tree cmake -B _bld <options>, then re-running cmake -B _bld shows a subset of the original command-line option, but not the whole cache:

initial:
-- | -DBUILD_LIBCURL_DOCS="OFF" -DBUILD_MISC_DOCS="OFF" -DCMAKE_TRY_COMPILE_TARGET_TYPE="STATIC_LIBRARY" -DCMAKE_UNITY_BUILD="ON" -DCURL_BROTLI="ON" -DCURL_CA_BUNDLE="none" -DCURL_CA_SEARCH_SAFE="ON" -DCURL_DISABLE_LDAP="ON" -DENABLE_CURL_MANUAL="OFF" -DOPENSSL_ROOT_DIR="/usr/local/opt/openssl@3" -DUNUSED_VARIABLE="FOOBAR"|

re-run (manually aligned with the initial run):
-- |                                                    -DCMAKE_TRY_COMPILE_TARGET_TYPE="STATIC_LIBRARY" -DCMAKE_UNITY_BUILD="ON"                                            -DCURL_CA_SEARCH_SAFE="ON"                                                     -DOPENSSL_ROOT_DIR="/usr/local/opt/openssl@3" -DUNUSED_VARIABLE="FOOBAR"|

@vszakats
Copy link
Member Author

I plan to merge this, unless there is some remaining concern.

@vszakats vszakats closed this in 9eb5c7c Nov 14, 2024
@vszakats vszakats deleted the cm-restore-args-detection branch November 14, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants