Skip to content

Fix: 1. typo 2. wrong usage of string(REPLACE) 3. Fix CI errors 4. Bump builtin xmake to 3.0.3 #43

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

Merged
waruqi merged 18 commits into from
Sep 28, 2025
Merged

Fix: 1. typo 2. wrong usage of string(REPLACE) 3. Fix CI errors 4. Bump builtin xmake to 3.0.3 #43

Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5028581
fix(docs): Correct typos in comments
yingmanwumen Sep 28, 2025
72074f3
fix(cmake): correct argument order in string(REPLACE) for compiler name
yingmanwumen Sep 28, 2025
7ef6cab
fix(ci): update github actions workflows for cmake and gcc
yingmanwumen Sep 28, 2025
a2d6686
docs: fix typos in README and example CMakeLists
yingmanwumen Sep 28, 2025
15f79a0
ci(workflows): add workflow_dispatch trigger to CI workflows
yingmanwumen Sep 28, 2025
1873304
ci: upgrade CMake and GCC versions in CI workflows
yingmanwumen Sep 28, 2025
97d784c
ci(workflow): Remove explicit gcc-13 installation from Linux CI
yingmanwumen Sep 28, 2025
000b4be
ci(workflow): Update CMake version in CI to 3.22.0
yingmanwumen Sep 28, 2025
92b3682
ci: Update CMake download paths in CI workflows
yingmanwumen Sep 28, 2025
247941f
fix(zlib): Apply macOS compatibility patch
yingmanwumen Sep 28, 2025
09be063
feat(zlib): Apply macOS patch version-specifically and relocate
yingmanwumen Sep 28, 2025
bff6488
fix(xmake): Resolve zlib_macos.patch not found issue
yingmanwumen Sep 28, 2025
dc5e943
refactor(xmake): use os.scriptdir instead of deprecated API
yingmanwumen Sep 28, 2025
d00f2f2
ci: try to fix patch path
yingmanwumen Sep 28, 2025
29e29ed
ci: try to fix patch path
yingmanwumen Sep 28, 2025
956858e
ci: fix zlib
yingmanwumen Sep 28, 2025
020fbe6
feat(ci): add Visual Studio v144 toolset install step for Windows
yingmanwumen Sep 28, 2025
bb4ed5a
build: Update xmake and streamline Windows CI
yingmanwumen Sep 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions xrepo.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ set(XREPO_XMAKEFILE "" CACHE STRING "Xmake script file of Xrepo package")
# `target_link_directories` to use the package.
# - User should figure out what library to use for `target_link_libraries`.
# - If `DIRECTORY_SCOPE` is specified, execute following code so the package
# can be used in cmake's direcotry scope:
# can be used in cmake's directory scope:
# include_directories(foo_INCLUDE_DIRS)
# link_directories(foo_LIBRARY_DIRS)
# 3. Append package install directory to `CMAKE_PREFIX_PATH`.
Expand Down Expand Up @@ -217,8 +217,8 @@ function(_detect_toolchain)
get_filename_component(_compiler_name "${CMAKE_C_COMPILER}" NAME_WLE)
elseif(DEFINED CMAKE_CXX_COMPILER)
get_filename_component(_compiler_name "${CMAKE_CXX_COMPILER}" NAME_WLE)
string(REPLACE "g++" "gcc" "${_compiler_name}" _compiler_name)
string(REPLACE "clang++" "clang" "${_compiler_name}" _compiler_name)
string(REPLACE "g++" "gcc" _compiler_name "${_compiler_name}")
string(REPLACE "clang++" "clang" _compiler_name "${_compiler_name}")
Comment on lines 218 to +221

@gemini-code-assist gemini-code-assist Bot Sep 28, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

While this change correctly fixes the string(REPLACE) syntax, the overall approach of parsing the compiler executable name to determine the toolchain is fragile. This method can easily break when compiler wrappers like ccache are used (e.g., CMAKE_CXX_COMPILER="ccache g++") or with unusually named compiler executables.

A more robust approach would be to leverage CMake's built-in compiler identification variables. For example:

if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
    set(_compiler_name "gcc")
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
    set(_compiler_name "clang")
endif()

If preserving the version from the compiler name (e.g., gcc-11) is important for xrepo, CMAKE_CXX_COMPILER_VERSION could be appended, or a more careful regex could be used to extract it from the executable name. Relying on CMAKE_<LANG>_COMPILER_ID is generally safer than string manipulation on file paths.

else()
# Shouldn't reach here because cmake will try to detect compiler and set
# corresponding variables.
Expand Down Expand Up @@ -375,7 +375,7 @@ function(xrepo_package package)

_xrepo_finish_package_setup(${package_name})

# Store xrepo command and arguments for furture comparison.
# Store xrepo command and arguments for future comparison.
set(_cache_xrepo_cmdargs_${package_name} "${_xrepo_cmdargs_${package_name}}" CACHE INTERNAL "")
endfunction()

Expand Down Expand Up @@ -421,7 +421,7 @@ function(xrepo_target_packages target)
endforeach()
endfunction()

# Append parent directorie of include directory to CMAKE_PREFIX_PATH.
# Append parent directory of include directory to CMAKE_PREFIX_PATH.
macro(_xrepo_set_cmake_prefix_path package_name)
# CMake looks for quite a few directories under each prefix directory for config-file.cmake.
# Thus Using CMAKE_PREFIX_PATH is easier and more reliable for config-file packages to be found
Expand Down
Loading