-
Notifications
You must be signed in to change notification settings - Fork 5
fix set VERSION_OUT_DIR not work #7
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
Open
hxf0223
wants to merge
4
commits into
BareCpper:master
Choose a base branch
from
hxf0223:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ cmake_install.cmake | |
| install_manifest.txt | ||
| compile_commands.json | ||
| CTestTestfile.cmake | ||
| /build/ | ||
| _deps | ||
| /.vs | ||
| /out | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,89 +5,102 @@ | |||||||||||||||||||||||||||||||||
| # @note 3.20 required for `GENERATED` attribute to be project-wide i.e. Version.h isn't build until build-time | ||||||||||||||||||||||||||||||||||
| cmake_minimum_required(VERSION 3.20) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #TODO? if ( DEFINED VERSION_SEMANTIC ) | ||||||||||||||||||||||||||||||||||
| #return() | ||||||||||||||||||||||||||||||||||
| #endif() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # TODO? if ( DEFINED VERSION_SEMANTIC ) | ||||||||||||||||||||||||||||||||||
| # return() | ||||||||||||||||||||||||||||||||||
| # endif() | ||||||||||||||||||||||||||||||||||
| message(CHECK_START "Version.cmake") | ||||||||||||||||||||||||||||||||||
| list(APPEND CMAKE_MESSAGE_INDENT " ") | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| set(VERSION_OUT_DIR "${CMAKE_BINARY_DIR}" CACHE PATH "Destination directory into which `Version.cmake` shall genrate Versioning header files") | ||||||||||||||||||||||||||||||||||
| if(NOT VERSION_OUT_DIR) | ||||||||||||||||||||||||||||||||||
| set(VERSION_OUT_DIR "${CMAKE_BINARY_DIR}" CACHE PATH "Destination directory into which `Version.cmake` shall genrate Versioning header files") | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| set(VERSION_OUT_DIR "${CMAKE_BINARY_DIR}" CACHE PATH "Destination directory into which `Version.cmake` shall genrate Versioning header files") | |
| set(VERSION_OUT_DIR "${CMAKE_BINARY_DIR}" CACHE PATH "Destination directory into which `Version.cmake` shall generate Versioning header files") |
Copilot
AI
Apr 4, 2026
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.
message(STATUS "Git Datetime: ${VERSION_DATETIME}") is misleading: VERSION_DATETIME is the current configure-time timestamp (from string(TIMESTAMP ...)), not a Git-derived datetime. Consider renaming the label (e.g., "Configure datetime"/"Build datetime") or extracting the commit date if you intend it to be Git time.
Suggested change
| message(STATUS "Git Datetime: ${VERSION_DATETIME}") | |
| message(STATUS "Configure datetime: ${VERSION_DATETIME}") |
Copilot
AI
Apr 4, 2026
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.
gitversion_configure_file(${VERSION_H_TEMPLATE} ${VERSION_H}) passes unquoted paths; if either path contains spaces/semicolons, argument splitting will break and configure_file() won’t receive the intended filenames. Quote both arguments at the call site.
Suggested change
| gitversion_configure_file(${VERSION_H_TEMPLATE} ${VERSION_H}) | |
| gitversion_configure_file("${VERSION_H_TEMPLATE}" "${VERSION_H}") |
Comment on lines
+186
to
+191
Copilot
AI
Apr 4, 2026
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.
if(NOT EXISTS ${VERSION_H_TEMPLATE}) / file(WRITE ${VERSION_H_TEMPLATE} ...) use unquoted paths. If the module lives under a directory with spaces (common on Windows), this will mis-parse and can error. Quote ${VERSION_H_TEMPLATE} (and other file paths) anywhere they’re used as single arguments to CMake commands/conditions.
Comment on lines
+220
to
+227
Copilot
AI
Apr 4, 2026
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.
In the custom target invocation, -D CMAKE_MODULE_PATH=${CMAKE_MODULE_PATH} (and other -D ...=${...} args) are unquoted. If CMAKE_MODULE_PATH contains multiple entries (semicolon-separated) or any value contains spaces, CMake will split the argument list and the script will receive truncated/incorrect -D values. Pass these -D values as a single argument (quote/escape), or serialize list variables explicitly before passing them.
Suggested change
| -D VERSION_GENERATE_NOW=YES | |
| -D VERSION_H_TEMPLATE=${VERSION_H_TEMPLATE} | |
| -D VERSION_H=${VERSION_H} | |
| -D VERSION_PREFIX=${VERSION_PREFIX} | |
| -D GIT_EXECUTABLE=${GIT_EXECUTABLE} | |
| -D CMAKE_MODULE_PATH=${CMAKE_MODULE_PATH} | |
| -P ${CMAKE_CURRENT_LIST_FILE} | |
| WORKING_DIRECTORY ${VERSION_SOURCE_DIR} | |
| "-DVERSION_GENERATE_NOW=YES" | |
| "-DVERSION_H_TEMPLATE=${VERSION_H_TEMPLATE}" | |
| "-DVERSION_H=${VERSION_H}" | |
| "-DVERSION_PREFIX=${VERSION_PREFIX}" | |
| "-DGIT_EXECUTABLE=${GIT_EXECUTABLE}" | |
| "-DCMAKE_MODULE_PATH=${CMAKE_MODULE_PATH}" | |
| -P "${CMAKE_CURRENT_LIST_FILE}" | |
| WORKING_DIRECTORY "${VERSION_SOURCE_DIR}" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The guard
if(NOT VERSION_OUT_DIR)checks truthiness, so values like...-NOTFOUND/OFF/0will be treated as false and silently replaced. Since this is a PATH option, it’s safer to check whether it is defined (and optionally whether it’s an empty string) rather than boolean-evaluating the path value.