Skip to content
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

package.tools.cmake should use add_compile_options in preference to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS #5826

Open
xq114 opened this issue Nov 14, 2024 · 32 comments

Comments

@xq114
Copy link
Contributor

xq114 commented Nov 14, 2024

你在什么场景下需要该功能?

目前由于cmake的bug https://gitlab.kitware.com/cmake/cmake/-/issues/15427 CMAKE_CXX_FLAGS 不仅会出现在编译过程,还会出现在链接过程中(视generator不同而行为不同),虽然大部分flag会被链接器忽略,但仍然留下了隐患;并且这个变量会在项目中被修改、覆盖,不一定能起到期待的效果。根据文档 https://cmake.org/cmake/help/latest/command/add_compile_options.html add_compile_options是新的加编译选项方式,更加安全、灵活(使用generator expression而不是硬编码语言)。使用add_compile_options的好处有三:

  1. 不会出现在链接过程中,保证在不同generator之间行为一致;
  2. 不容易在项目中被修改,导致预先指定的flags被覆盖;
  3. 更符合xmake中cxflags的设计(可以利用generator expression同时为C/C++指定flags;不用单独为RELEASE/DEBUG重复加flag)

-- shrink cmake arguments, fix too long arguments
-- @see https://github.com/xmake-io/xmake-repo/pull/5247#discussion_r1780302212
local cmake_argv = {}
local long_options = hashset.of(
"CMAKE_C_FLAGS",
"CMAKE_CXX_FLAGS",
"CMAKE_ASM_FLAGS",
"CMAKE_EXE_LINKER_FLAGS",
"CMAKE_SHARED_LINKER_FLAGS",
"CMAKE_C_FLAGS_RELEASE",
"CMAKE_CXX_FLAGS_RELEASE",
"CMAKE_ASM_FLAGS_RELEASE",
"CMAKE_EXE_LINKER_FLAGS_RELEASE",
"CMAKE_SHARED_LINKER_FLAGS_RELEASE",
"CMAKE_C_FLAGS_DEBUG",
"CMAKE_CXX_FLAGS_DEBUG",
"CMAKE_ASM_FLAGS_DEBUG",
"CMAKE_EXE_LINKER_FLAGS_DEBUG",
"CMAKE_SHARED_LINKER_FLAGS_DEBUG")
local shrink = false
table.remove_if(argv, function (idx, value)
local k, v = value:match("%-D(.*)=(.*)")
if k and v and long_options:has(k) and #v > 128 then
table.insert(cmake_argv, ("set(%s \"%s\")"):format(k, tostring(v)))
shrink = true
return true
end
end)
if shrink then
local cmakefile = path.join(opt.curdir and opt.curdir or oldir, "CMakeLists.txt")
io.insert(cmakefile, 1, table.concat(cmake_argv, "\n"))
end
table.insert(argv, oldir)

目前xmake的shrink功能也为使用 add_compile_options 留下了空间。缺点仅仅是要求cmake 3.13,但目前cmake已经到3.31了,要求cmake 3.13并不过分。

此外,链接时也有更加灵活的add_link_options可用 https://cmake.org/cmake/help/latest/command/add_link_options.html 但不支持为静态库指定flag,并且现有方案没有显著bug。唯一问题是package.tools.cmake中缺少了CMAKE_MODULE_LINKER_FLAGS,这个变量指定为和shflags一样就行。

描述可能的解决方案

  1. 在shrink过程中将set(CMAKE_CXX_FLAGS ...)等替换成add_compile_options($<$<COMPILE_LANGUAGE:CXX>:...>);
  2. 加一行 set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS}) 或者改用新的add_link_options API

描述你认为的候选方案

GPT 回复使用add_compile_options同时为C/C++指定compile option的方式:

在CMake中,使用add_compile_options配合生成表达式$<OR:...>$<COMPILE_LANGUAGE:...>,可以根据不同的编程语言来指定编译选项,同时适用于C和C++。

用法示例

假设我们希望在整个项目中为C和C++代码都添加某个编译选项,比如-Wall(开启所有警告),而-std=c99仅应用于C代码:

add_compile_options(
    $<$<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-Wall>
    $<$<COMPILE_LANGUAGE:C>:-std=c99>
)

解释

  • $<OR:$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:CXX>>:-Wall>:指定-Wall选项适用于C和C++两种语言。
  • $<COMPILE_LANGUAGE:C>:-std=c99>:仅当编译C代码时才使用-std=c99选项。

这种写法使得CMake在编译时动态应用编译选项,并确保不同语言有适用的选项,提升了灵活性。

其他信息

CMAKE_CXX_FLAGS出现在链接中
image

@waruqi
Copy link
Member

waruqi commented Nov 15, 2024

不用单独为RELEASE/DEBUG重复加flag)

如果是 add_requires/cmake.install 里面传递的 cflags 等参数,原本就没区分 debug/release,关键是现在内部还有个 get default cmake flags 的逻辑,会根据 package 的 mode ,去从 cmake 中取 DEBUG/RELEASE 的默认参数。这块还是要区分 mode 的。

所以这个区分逻辑还是要加的

另外目前这个 shrink 逻辑,只对 value string > 128 生效,并不是全开的,因为我暂时不确定它的稳定性和兼容性,如果要支持这个,就得把这个 length 限制去掉才行 ,稳定性有待验证。

add_compile_options 使用上似乎也有些限制,分语言似乎无法直接一次传入多值,否则编译时候会报错,例如这种 add_compile_options($<$<COMPILE_LANGUAGE:C>:-Wall -DXXX>)

我暂时只能拆分 flags 挨个设置,另外即使拆分了,flag 里面带有空格,也会又问题,需要额外的 \ 转义处理。。

坑也挺多,我暂时实现了初版,但我没把握它一定可靠。

add_link_options 无法区分 binary/shared ,暂时没用,改成这种,就直接对所有 binary/shared 生效了,会 break 。

#5827

@waruqi
Copy link
Member

waruqi commented Nov 15, 2024

目前的 patch ,ci 上测了些基础包 没啥问题,你这也可以测下,比如测一下带有各种 flags 的包

@Issues-translate-bot
Copy link

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


In the current patch, some basic packages have been tested on ci. There is no problem. You can also test it, such as testing packages with various flags.

@xq114
Copy link
Contributor Author

xq114 commented Nov 15, 2024

不用单独为RELEASE/DEBUG重复加flag)

如果是 add_requires/cmake.install 里面传递的 cflags 等参数,原本就没区分 debug/release,关键是现在内部还有个 get default cmake flags 的逻辑,会根据 package 的 mode ,去从 cmake 中取 DEBUG/RELEASE 的默认参数。这块还是要区分 mode 的。

generator expression 也可以区分mode的: $<CMAKE_BUILD_TYPE:Debug> 这样

add_compile_options 使用上似乎也有些限制,分语言似乎无法直接一次传入多值,否则编译时候会报错,例如这种 add_compile_options($<$<COMPILE_LANGUAGE:C>:-Wall -DXXX>)

我暂时只能拆分 flags 挨个设置,另外即使拆分了,flag 里面带有空格,也会又问题,需要额外的 \ 转义处理。。

cmake指定多个flag需要引号引起来,比如set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /utf-8" 试试add_compile_options($<$<COMPILE_LANGUAGE:C>:"-Wall -DXXX">)

@Issues-translate-bot
Copy link

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


No need to add flag repeatedly for RELEASE/DEBUG separately)

If it is cflags and other parameters passed in add_requires/cmake.install, there is no distinction between debug/release. The key is that there is now an internal logic of get default cmake flags, which will get DEBUG/from cmake according to the mode of the package. Default parameters for RELEASE. Here we still have to distinguish mode.

Generator expression can also distinguish modes: $<CMAKE_BUILD_TYPE:Debug> like this

There seem to be some restrictions on the use of add_compile_options. Depending on the language, it seems that multiple values ​​cannot be passed in directly at once, otherwise an error will be reported during compilation, such as this add_compile_options($<$<COMPILE_LANGUAGE:C>:-Wall -DXXX>)

For the time being, I can only split the flags and set them one by one. In addition, even if the flags are split, there will be problems if there are spaces in the flags, and additional \ escaping processing is required. .

cmake needs to specify multiple flags in quotation marks, such as set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /utf-8". Try add_compile_options($<$<COMPILE_LANGUAGE:C>:"-Wall -DXXX">)

@xq114
Copy link
Contributor Author

xq114 commented Nov 15, 2024

如果不使用新的 add_linker_options API的话,希望能加上 set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS}) 目前Module用不了packagedeps

@Issues-translate-bot
Copy link

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


If you don’t use the new add_linker_options API, I hope you can add set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS}). Currently, Module cannot use packagedeps.

@waruqi
Copy link
Member

waruqi commented Nov 15, 2024

cmake指定多个flag需要引号引起来

引号刚就试过了,会挂,你可以试试

@Issues-translate-bot
Copy link

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


cmake specifies multiple flags that need to be enclosed in quotes

I just tried using quotation marks and it failed. You can try it.

@waruqi
Copy link
Member

waruqi commented Nov 15, 2024

如果不使用新的 add_linker_options API的话,希望能加上 set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS}) 目前Module用不了packagedeps

这里的 CMAKE_MODULE_LINKER_FLAGS 是指啥? c++ module?为啥用的是 shared flags

@xq114
Copy link
Contributor Author

xq114 commented Nov 16, 2024

如果不使用新的 add_linker_options API的话,希望能加上 set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS}) 目前Module用不了packagedeps

这里的 CMAKE_MODULE_LINKER_FLAGS 是指啥? c++ module?为啥用的是 shared flags

https://cmake.org/cmake/help/latest/command/add_library.html

MODULE A plugin that may not be linked by other targets, but may be dynamically loaded at runtime using dlopen-like functionality.

通俗来讲就是python里.pyd之类的动态库,不用来链接只用来LoadLibrary动态加载的

@waruqi
Copy link
Member

waruqi commented Nov 18, 2024

这样?再试试呢 1e9846e

通俗来讲就是python里.pyd之类的动态库

这种,cmake 不会使用 shared linker flags?

@waruqi waruqi added this to the v2.9.7 milestone Nov 18, 2024
@waruqi
Copy link
Member

waruqi commented Nov 18, 2024

现在的 patch 可以了么

@Issues-translate-bot
Copy link

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


Is the current patch OK?

@xq114
Copy link
Contributor Author

xq114 commented Nov 18, 2024

这样?再试试呢 1e9846e

通俗来讲就是python里.pyd之类的动态库

这种,cmake 不会使用 shared linker flags?

是的,现在这样可以了

@xq114 xq114 closed this as completed Nov 18, 2024
@xq114 xq114 reopened this Nov 19, 2024
@xq114
Copy link
Contributor Author

xq114 commented Nov 19, 2024

image
include flag只有第一个加了-isystem

@Issues-translate-bot
Copy link

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


image
Only the first include flag is added -isystem

@xq114
Copy link
Contributor Author

xq114 commented Nov 19, 2024

https://zhuanlan.zhihu.com/p/437404485

但是使用生成器表达式可以简化成:
add_compile_options("$<$CONFIG:Debug:-g;-O0>")
add_compile_options($<$CONFIG:Release:-O2>)
如果需要指定多个编译选项,必须使用双引号把生成器表达式包含起来,且选项之间使用分号。

@waruqi
Copy link
Member

waruqi commented Nov 19, 2024

暂时没空看,你先调调,可以直接来个 pr 过来

@Issues-translate-bot
Copy link

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


I don’t have time to watch it at the moment. You can adjust it first. You can send me a PR directly.

@waruqi
Copy link
Member

waruqi commented Nov 20, 2024

再试试 b6a04f8

@Issues-translate-bot
Copy link

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


Try again b6a04f8

@waruqi
Copy link
Member

waruqi commented Nov 20, 2024

还不行,有点问题,等这个 patch #5850

@xq114
Copy link
Contributor Author

xq114 commented Nov 20, 2024

应该是 local flags = v:replace(" ", ";") 吧?

@Issues-translate-bot
Copy link

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


It should be local flags = v:replace(" ", ";"), right?

@waruqi
Copy link
Member

waruqi commented Nov 21, 2024

应该是 local flags = v:replace(" ", ";") 吧?

不行,还有 -I/tmp/xxx with spaces 这种带空格的路径,替换了 不就出错了?还有 -I /tmp 这种。

@waruqi
Copy link
Member

waruqi commented Nov 21, 2024

改成 ; 后,编译直接报错了

add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-I;"/tmp/xx;x";-I;/tmp/bar>)
"\$<0:-I" \"/tmp/xx x\" -I "/tmp/bar>" "\$<1:-I"

@Issues-translate-bot
Copy link

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


After changing to ;, the compilation directly reported an error

add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-I;"/tmp/xx;x";-I;/tmp/bar>)
"\$<0:-I" \"/tmp/xx x\" -I "/tmp/bar>" "\$<1:-I"

@waruqi
Copy link
Member

waruqi commented Nov 21, 2024

感觉 add_compile_options 这个坑也挺多,加 flags 比直接设置 CMAKE_CXX_FLAGS 还麻烦,目前没啥好办法,如果拆成多个 add_compile_options 配置,-I /tmp/foo -I /tmp/bar 就会被拆开,导致出现两个 -I 设置,会被 add_compile_options 自动去重。

不行就只能先暂时禁用 add_compile_options 了。 你可以试试 有啥解决办法。

@Issues-translate-bot
Copy link

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


I feel that add_compile_options has a lot of pitfalls. Adding flags is more troublesome than directly setting CMAKE_CXX_FLAGS. There is currently no good solution. If it is split into multiple add_compile_options configurations, -I /tmp/foo -I /tmp/bar will be split. , resulting in two -I settings, which will be automatically deduplicated by add_compile_options.

If not, you can only temporarily disable add_compile_options. You can try to find any solutions.

@waruqi
Copy link
Member

waruqi commented Nov 21, 2024

我暂时先 revert 了,等回头有空再改进

@Issues-translate-bot
Copy link

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


I'll revert it for now, and I'll improve it later when I have time.

@waruqi waruqi removed this from the v2.9.7 milestone Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants