Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

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

wants to merge 3 commits into from
+21 −0

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 13, 2024 •
edited
Loading

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

@vszakats
cmake: restore cmake args list in buildinfo.txt

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
Loading
Loading status checks…
2470408 
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 •
edited
Loading

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
replace unset(VAR) with set(VAR "")

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
Loading
Loading status checks…
0dabedf
@vszakats
Copy link
Member Author

vszakats commented Nov 13, 2024 •
edited
Loading

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
collect _cmake_args for CI runs only

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
Loading
Loading status checks…
d00f938 
For performance.
@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
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
@vszakats @pps83
cmake: restore cmake args list in buildinfo.txt

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
aedff44 
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 curl#15501
Closes curl#15563
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
build cmake
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@vszakats @dg0yt