Conversation
|
AUI boot already has branch support |
There was a problem hiding this comment.
Code Review
This pull request introduces new CMake utility functions, aui_git_get_latest_tag and aui_git_get_latest_commit, which allow querying remote Git repositories for tags and commit SHAs without cloning. The review feedback identifies several critical improvements: securing git commands against argument injection by using the -- separator, ensuring correct prefix stripping for tags using anchored regex instead of global replacement, safely truncating commit SHAs to avoid potential CMake errors with string(SUBSTRING), and simplifying the Git requirement macro by removing redundant checks after find_package.
| # git ls-remote --tags --sort=version:refname <url> <pattern> | ||
| # --sort may not be available on older Git; we fall back to plain listing. | ||
| execute_process( | ||
| COMMAND "${GIT_EXECUTABLE}" ls-remote --tags "${repo_url}" "${ARG_PATTERN}" |
| endif() | ||
|
|
||
| execute_process( | ||
| COMMAND "${GIT_EXECUTABLE}" ls-remote "${repo_url}" "${_ref}" |
| endif() | ||
|
|
||
| execute_process( | ||
| COMMAND "${GIT_EXECUTABLE}" ls-remote --symref "${repo_url}" HEAD |
| macro(_git_remote_require_git) | ||
| if(NOT GIT_EXECUTABLE) | ||
| find_package(Git QUIET REQUIRED) | ||
| endif() | ||
| if(NOT GIT_EXECUTABLE) | ||
| message(FATAL_ERROR "[GitRemoteInfo] Git executable not found. " | ||
| "Install Git or set GIT_EXECUTABLE manually.") | ||
| endif() | ||
| endmacro() |
There was a problem hiding this comment.
The check for GIT_EXECUTABLE after find_package(Git REQUIRED) is redundant because the REQUIRED keyword already causes CMake to stop with a fatal error if Git is not found. The macro can be simplified.
macro(_git_remote_require_git)
if(NOT GIT_EXECUTABLE)
find_package(Git QUIET REQUIRED)
endif()
endmacro()
| string(REGEX REPLACE "^[0-9a-f]+\t" "" _ref "${_line}") | ||
|
|
||
| # Strip the "refs/tags/" prefix to get the bare tag name | ||
| string(REPLACE "refs/tags/" "" _tag_name "${_ref}") |
There was a problem hiding this comment.
Using string(REPLACE) to strip the refs/tags/ prefix is risky because it will replace all occurrences of that string within the tag name. It is safer to use string(REGEX REPLACE) with an anchor to only strip the prefix at the beginning of the string.
string(REGEX REPLACE "^refs/tags/" "" _tag_name "${_ref}")
| endif() | ||
|
|
||
| if(ARG_SHORT) | ||
| string(SUBSTRING "${_commit_sha}" 0 7 _commit_sha) |
There was a problem hiding this comment.
|
aui.boot.cmake is a bloatware |
solves: #119
add custom functions for aui.build.cmake like this
maybe integrate these functions for
auib_import?