Make sure generic options are always inserted. by Redbeanw44602 · Pull Request #6867 · xmake-io/xmake

Redbeanw44602 · 2025-09-28T08:40:22Z

Currently, the package.tools.cmake has not yet inserted generic options for appleos/mingw (such as CMAKE_BUILD_TYPE, BUILD_SHARED_LIBS, etc). I think this may be a bug.

This patch also makes _get_configs_for_cross call _get_configs_for_generic and removes the duplicate logic.

gemini-code-assist · 2025-09-28T08:40:37Z

Summary of Changes

Hello @Redbeanw44602, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a bug and significantly improves code maintainability within the CMake package tool module. It ensures that essential generic CMake configuration options are always correctly applied across various build environments, specifically fixing an oversight for Apple OS and MinGW. The changes also refactor the configuration retrieval logic, centralizing the application of these generic options and eliminating duplicate code, leading to a more robust and cleaner codebase.

Highlights

Centralized Generic CMake Options: Ensures that common CMake options like CMAKE_BUILD_TYPE and BUILD_SHARED_LIBS are consistently applied across different build configurations by centralizing their insertion through _get_configs_for_generic.
Bug Fix for Apple OS and MinGW: Corrects an issue where generic CMake options were not being inserted for Apple OS and MinGW build environments, leading to potentially incorrect or incomplete configurations.
Code Refactoring and Duplication Removal: Consolidates logic by making _get_configs_for_cross, _get_configs_for_appleos, _get_configs_for_mingw, and _get_configs_for_host_toolchain functions call _get_configs_for_generic, eliminating redundant code for setting generic options and position-independent code (PIC).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

Redbeanw44602 · 2025-09-28T08:41:16Z

xmake-io/xmake-repo#7824

gemini-code-assist

Code Review

This pull request aims to refactor CMake configuration generation by centralizing generic options, which is a good goal. However, the current implementation introduces a critical bug across several platform-specific configuration functions (_get_configs_for_appleos, _get_configs_for_mingw, _get_configs_for_wasm, _get_configs_for_cross, _get_configs_for_host_toolchain). In each case, the refactoring causes the platform-specific CMake environment variables to be discarded, which will likely break compilation for those platforms.

The root cause is that _get_configs_for_generic creates its own envs table and doesn't use the one from its caller. My review comments provide a consistent fix: refactor _get_configs_for_generic to accept an envs table, populate it with generic options, and then use it to generate the CMake arguments. This will correctly merge platform-specific and generic options.

xmake/modules/package/tools/cmake.lua

waruqi · 2025-09-28T14:36:24Z

当初这个接口里，还有对 flags 的处理，所以不同平台，有的调了，cross 里没用。

不过 dev 中 flags 的处理已经剥离掉了，只剩一些通用配置，那这个接口，你可以在 _get_configs 里面统一调用，不用每个平台里去加

xmake/xmake/modules/package/tools/cmake.lua

Lines 402 to 436 in 0db4fe6

    
           function _get_configs_for_generic(package, configs, opt) 
        
               local envs = {} 
        
               local cflags = _get_cflags(package, opt) 
        
               if cflags then 
        
                   envs.CMAKE_C_FLAGS = cflags 
        
               end 
        
               local cxxflags = _get_cxxflags(package, opt) 
        
               if cxxflags then 
        
                   envs.CMAKE_CXX_FLAGS = cxxflags 
        
               end 
        
               local asflags = _get_asflags(package, opt) 
        
               if asflags then 
        
                   envs.CMAKE_ASM_FLAGS = asflags 
        
               end 
        
               local ldflags = _get_ldflags(package, opt) 
        
               if ldflags then 
        
                   envs.CMAKE_EXE_LINKER_FLAGS = ldflags 
        
               end 
        
               local shflags = _get_shflags(package, opt) 
        
               if shflags then 
        
                   envs.CMAKE_SHARED_LINKER_FLAGS = shflags 
        
                   envs.CMAKE_MODULE_LINKER_FLAGS = shflags 
        
               end 
        
               if not package:is_plat("windows", "mingw") and package:config("pic") ~= false then 
        
                   envs.CMAKE_POSITION_INDEPENDENT_CODE = "ON" 
        
               end 
        
               if not package:use_external_includes() then 
        
                   envs.CMAKE_NO_SYSTEM_FROM_IMPORTED = "ON" 
        
               end 
        
               envs.CMAKE_BUILD_TYPE = package:is_debug() and "Debug" or "Release" 
        
               if package:is_library() then 
        
                   envs.BUILD_SHARED_LIBS = package:config("shared") and "ON" or "OFF" 
        
               end 
        
               _insert_configs_from_envs(configs, envs, opt) 
        
           end

Issues-translate-bot · 2025-09-28T15:21:25Z

Bot detected the issue body's language is not English, translate it automatically.

In the original interface, there was also the processing of flags, so some of them were adjusted on different platforms, but they were useless in cross.

However, the processing of flags in dev has been stripped and adjusted. You can call this interface in _get_configs without adding it to each platform.

xmake/xmake/modules/package/tools/cmake.lua

Lines 402 to 436 in 0db4fe6

    
           function _get_configs_for_generic(package, configs, opt) 
        
               local envs = {} 
        
               local cflags = _get_cflags(package, opt) 
        
               if cflags then 
        
                   envs.CMAKE_C_FLAGS = cflags 
        
               end 
        
               local cxxflags = _get_cxxflags(package, opt) 
        
               if cxxflags then 
        
                   envs.CMAKE_CXX_FLAGS = cxxflags 
        
               end 
        
               local asflags = _get_asflags(package, opt) 
        
               if asflags then 
        
                   envs.CMAKE_ASM_FLAGS = asflags 
        
               end 
        
               local ldflags = _get_ldflags(package, opt) 
        
               if ldflags then 
        
                   envs.CMAKE_EXE_LINKER_FLAGS = ldflags 
        
               end 
        
               local shflags = _get_shflags(package, opt) 
        
               if shflags then 
        
                   envs.CMAKE_SHARED_LINKER_FLAGS = shflags 
        
                   envs.CMAKE_MODULE_LINKER_FLAGS = shflags 
        
               end 
        
               if not package:is_plat("windows", "mingw") and package:config("pic") ~= false then 
        
                   envs.CMAKE_POSITION_INDEPENDENT_CODE = "ON" 
        
               end 
        
               if not package:use_external_includes() then 
        
                   envs.CMAKE_NO_SYSTEM_FROM_IMPORTED = "ON" 
        
               end 
        
               envs.CMAKE_BUILD_TYPE = package:is_debug() and "Debug" or "Release" 
        
               if package:is_library() then 
        
                   envs.BUILD_SHARED_LIBS = package:config("shared") and "ON" or "OFF" 
        
               end 
        
               _insert_configs_from_envs(configs, envs, opt) 
        
           end

Redbeanw44602 · 2025-09-29T05:49:51Z

当初这个接口里，还有对 flags 的处理，所以不同平台，有的调了，cross 里没用。

不过 dev 中 flags 的处理已经剥离掉了，只剩一些通用配置，那这个接口，你可以在 _get_configs 里面统一调用，不用每个平台里去加

xmake/xmake/modules/package/tools/cmake.lua

Lines 402 to 436 in 0db4fe6

function _get_configs_for_generic(package, configs, opt)

local envs = {}

local cflags = _get_cflags(package, opt)

if cflags then

envs.CMAKE_C_FLAGS = cflags

end

local cxxflags = _get_cxxflags(package, opt)

if cxxflags then

envs.CMAKE_CXX_FLAGS = cxxflags

end

local asflags = _get_asflags(package, opt)

if asflags then

envs.CMAKE_ASM_FLAGS = asflags

end

local ldflags = _get_ldflags(package, opt)

if ldflags then

envs.CMAKE_EXE_LINKER_FLAGS = ldflags

end

local shflags = _get_shflags(package, opt)

if shflags then

envs.CMAKE_SHARED_LINKER_FLAGS = shflags

envs.CMAKE_MODULE_LINKER_FLAGS = shflags

end

if not package:is_plat("windows", "mingw") and package:config("pic") ~= false then

envs.CMAKE_POSITION_INDEPENDENT_CODE = "ON"

end

if not package:use_external_includes() then

envs.CMAKE_NO_SYSTEM_FROM_IMPORTED = "ON"

end

envs.CMAKE_BUILD_TYPE = package:is_debug() and "Debug" or "Release"

if package:is_library() then

envs.BUILD_SHARED_LIBS = package:config("shared") and "ON" or "OFF"

end

_insert_configs_from_envs(configs, envs, opt)

end

OK了

Issues-translate-bot · 2025-09-29T05:50:01Z

Bot detected the issue body's language is not English, translate it automatically.

In the original interface, there was also the processing of flags, so some of them were adjusted on different platforms, but they were useless in cross.

However, the processing of flags in dev has been stripped away, and only some common configurations are left. You can call this interface in _get_configs without adding it to each platform.

xmake/xmake/modules/package/tools/cmake.lua

Lines 402 to 436 in 0db4fe6

function _get_configs_for_generic(package, configs, opt)

local envs = {}

local cflags = _get_cflags(package, opt)

if cflags then

envs.CMAKE_C_FLAGS = cflags

end

local cxxflags = _get_cxxflags(package, opt)

if cxxflags then

envs.CMAKE_CXX_FLAGS = cxxflags

end

local asflags = _get_asflags(package, opt)

if asflags then

envs.CMAKE_ASM_FLAGS = asflags

end

local ldflags = _get_ldflags(package, opt)

if ldflags then

envs.CMAKE_EXE_LINKER_FLAGS = ldflags

end

local shflags = _get_shflags(package, opt)

if shflags then

envs.CMAKE_SHARED_LINKER_FLAGS = shflags

envs.CMAKE_MODULE_LINKER_FLAGS = shflags

end

if not package:is_plat("windows", "mingw") and package:config("pic") ~= false then

envs.CMAKE_POSITION_INDEPENDENT_CODE = "ON"

end

if not package:use_external_includes() then

envs.CMAKE_NO_SYSTEM_FROM_IMPORTED = "ON"

end

envs.CMAKE_BUILD_TYPE = package:is_debug() and "Debug" or "Release"

if package:is_library() then

envs.BUILD_SHARED_LIBS = package:config("shared") and "ON" or "OFF"

end

_insert_configs_from_envs(configs, envs, opt)

end

OK

make sure generic options are always inserted.

a9a7ebc

Redbeanw44602 changed the title ~~make sure generic options are always inserted.~~ Make sure generic options are always inserted. Sep 28, 2025

gemini-code-assist bot reviewed Sep 28, 2025

View reviewed changes

fix.

1adb1f5

update.

f2ee698

waruqi merged commit a8232d9 into xmake-io:dev Sep 29, 2025
22 checks passed

waruqi added this to the v3.0.4 milestone Sep 29, 2025

Redbeanw44602 deleted the fix/cmake-generic-configs branch September 29, 2025 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make sure generic options are always inserted.#6867

Make sure generic options are always inserted.#6867
waruqi merged 3 commits intoxmake-io:devfrom
Redbeanw44602:fix/cmake-generic-configs

Redbeanw44602 commented Sep 28, 2025

Uh oh!

gemini-code-assist bot commented Sep 28, 2025

Uh oh!

Redbeanw44602 commented Sep 28, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waruqi commented Sep 28, 2025 •

edited

Loading

Uh oh!

Issues-translate-bot commented Sep 28, 2025

Uh oh!

Redbeanw44602 commented Sep 29, 2025

Uh oh!

Issues-translate-bot commented Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Redbeanw44602 commented Sep 28, 2025

Uh oh!

gemini-code-assist bot commented Sep 28, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

Redbeanw44602 commented Sep 28, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waruqi commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Issues-translate-bot commented Sep 28, 2025

Uh oh!

Redbeanw44602 commented Sep 29, 2025

Uh oh!

Issues-translate-bot commented Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

waruqi commented Sep 28, 2025 •

edited

Loading