Skip to content

优化cmake支持#70

Merged
mingkuang-Chuyu merged 5 commits intoChuyu-Team:Fea/PendingPRfrom
HIllya51:master
Nov 19, 2024
Merged

优化cmake支持#70
mingkuang-Chuyu merged 5 commits intoChuyu-Team:Fea/PendingPRfrom
HIllya51:master

Conversation

@HIllya51
Copy link
Copy Markdown
Contributor

用了好久,感觉有几个地方不是很舒服,希望能优化一下:
1、vscode cmaketool按那个按钮编译是小写开头的"win32",匹配不上
2、有时候希望没有install的时候也能编译(这个不重要,您觉得不好的话可以删掉)
3、这个比较重要,就是一个大的项目下面有几个子项目,include会导致整个项目全都链接vcltl,但其实只希望其中某几个链接vcltl

@MouriNaruto
Copy link
Copy Markdown
Member

你看看能否改进成无视大小写的字符串比较,这样的话其他人也能写 ARM ARM64 啥的

毛利

@mingkuang-Chuyu mingkuang-Chuyu added 类型:新功能/建议(enhancement) New feature or request 处置:正在讨论(Review) 我们正在讨论如何解决,怎么实现更好。 labels Nov 11, 2024
@mingkuang-Chuyu
Copy link
Copy Markdown
Collaborator

mingkuang-Chuyu commented Nov 14, 2024

@xspeed1989 请帮忙review下。

  1. 帮忙评估一下更改是否影响早期VC-LTL版本的兼容性
  2. 额外添加的 INTERFACE是否存在一种不生效或者优先级比编译器内置CRT库低的可能性?
  3. option(VC_LTL_EnableCMakeInterface "VC_LTL_EnableCMakeInterface" OFF) 这种方式 CMake 3.5.2是否支持?

@HIllya51
Copy link
Copy Markdown
Contributor Author

好的

@HIllya51
Copy link
Copy Markdown
Contributor Author

  1. option(VC_LTL_EnableCMakeInterface "VC_LTL_EnableCMakeInterface" OFF) 这种方式 CMake 3.5.2是否支持?

3.5.2是肯定支持option的,option很早的版本就支持了 https://cmake.org/cmake/help/v3.5/command/option.html
1&2我就不太清楚了,等大佬评估。

if(VC_LTL_EnableCMakeInterface)
add_library(VC_LTL5 INTERFACE)
target_include_directories(VC_LTL5 SYSTEM BEFORE INTERFACE ${VC_LTL_Include})
target_link_directories(VC_LTL5 INTERFACE ${VC_LTL_Library})
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

根据大胸review,target_link_directories需要3.13开始才支持,目前正在评估cmake minversion提升到3.13,是否会对大家造成困扰。

cmake.org/cmake/help/v3.13/command/target_link_directories.html

Copy link
Copy Markdown
Contributor Author

@HIllya51 HIllya51 Nov 14, 2024

Choose a reason for hiding this comment

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

其实还好,因为这个选项默认是关闭的,只要不开它即使3.5版也不会报错
不过其实可以glob遍历这个目录下的文件,然后用target_link_libraries来做,这样就不需要提升cmake版本了

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

我写的时候没注意cmake版本要求。如果需要的话,我可以改一下。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

不过感觉3.5确实也有点老了,cmake版本也不是像目标系统那样不方便升级,所以要是决定提升版本的话那就更好了

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

其实还好,因为这个选项默认是关闭的,只要不开它即使3.5版也不会报错 不过其实可以glob遍历这个目录下的文件,然后用target_link_libraries来做,这样就不需要提升cmake版本了

跟大胸沟通了下,早期的CMake遇到target_link_directories 应该会直接报错,又这个VC_LTL_EnableCMakeInterface判断也没用。一旦决定用target_link_directories,或许只能提升Cmake版本

@mingkuang-Chuyu mingkuang-Chuyu added 处置:等待发布(Publishing) 问题已经解决,等待新版本发布。 and removed 处置:正在讨论(Review) 我们正在讨论如何解决,怎么实现更好。 labels Nov 19, 2024
@mingkuang-Chuyu mingkuang-Chuyu changed the base branch from master to Fea/PendingPR November 19, 2024 06:17
@mingkuang-Chuyu mingkuang-Chuyu merged commit 35ba201 into Chuyu-Team:Fea/PendingPR Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

类型:新功能/建议(enhancement) New feature or request 处置:等待发布(Publishing) 问题已经解决,等待新版本发布。

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants