-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat: Support compile on Rocky Linux #417
Conversation
Walkthrough此次更新改善了PikiwiDB项目的CMake配置,主要集中在安装目录的灵活性和外部项目的管理上。引入了 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BuildSystem
participant ExternalLibrary
User->>BuildSystem: 触发构建
BuildSystem->>ExternalLibrary: 拉取外部库
ExternalLibrary->>BuildSystem: 返回库文件
BuildSystem->>User: 提供构建完成消息
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
cmake/brpc.cmake (1)
25-25
: 移除URL_HASH
可能影响完整性验证在
brpc.cmake
中移除URL_HASH
可能会影响下载文件的完整性验证。其他 CMake 文件仍在使用URL_HASH
进行完整性检查,但未发现替代机制。
- 请确保在
brpc.cmake
中实施其他完整性验证机制。Analysis chain
验证:移除 URL_HASH 可能影响项目完整性。
移除
URL_HASH
可能会影响下载项目的完整性验证。建议确保有其他机制来验证下载的文件。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the presence of integrity checks for the brpc project. # Test: Search for integrity checks in the CMake configuration files. Expect: Alternative integrity checks or comments explaining the removal. rg --type cmake 'URL_HASH|integrity|checksum' -A 3Length of output: 545
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- CMakeLists.txt (4 hunks)
- README.md (2 hunks)
- README_CN.md (2 hunks)
- cmake/braft.cmake (1 hunks)
- cmake/brpc.cmake (1 hunks)
- cmake/fmt.cmake (1 hunks)
- cmake/gflags.cmake (2 hunks)
- cmake/gtest.cmake (1 hunks)
- cmake/leveldb.cmake (2 hunks)
- cmake/llhttp.cmake (1 hunks)
- cmake/lz4.cmake (2 hunks)
- cmake/openssl.cmake (1 hunks)
- cmake/protobuf.cmake (1 hunks)
- cmake/rocksdb.cmake (1 hunks)
- cmake/snappy.cmake (1 hunks)
- cmake/spdlog.cmake (1 hunks)
- cmake/zlib.cmake (1 hunks)
- cmake/zstd.cmake (1 hunks)
Files skipped from review due to trivial changes (1)
- README_CN.md
Additional context used
Learnings (1)
cmake/zlib.cmake (1)
Learnt from: dingxiaoshuai123 PR: OpenAtomFoundation/pikiwidb#378 File: cmake/zlib.cmake:0-0 Timestamp: 2024-07-20T02:20:32.550Z Learning: Ensure that comments in the CMake configuration files accurately reflect the corresponding library or component they refer to.
Markdownlint
README.md
13-13: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Additional comments not posted (28)
cmake/llhttp.cmake (3)
15-17
: 移除 URL_HASH 可能影响文件完整性移除
URL_HASH
参数可能会导致下载的文件未经过完整性验证。建议确保文件来源可信,或者考虑使用其他方式进行文件验证。
15-15
: 添加 CMAKE_INSTALL_LIBDIR 参数添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
参数有助于更好地组织库文件的安装路径,提升跨平台兼容性。
17-17
: 使用多核编译提升性能使用
BUILD_COMMAND make -j${CPU_CORE}
允许并行编译,可以显著提升构建性能。确保CPU_CORE
变量已正确设置。cmake/zlib.cmake (2)
17-17
: 添加 CMAKE_INSTALL_LIBDIR 参数添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
参数提高了库文件安装路径的配置灵活性。
20-20
: 使用多核编译提升性能使用
BUILD_COMMAND make -j${CPU_CORE}
以支持并行编译,能够显著提高构建效率。请确保CPU_CORE
变量已正确配置。cmake/lz4.cmake (1)
16-16
: 添加 CMAKE_INSTALL_LIBDIR 参数添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
参数有助于更好地管理库文件的安装路径,增强项目的可配置性。cmake/fmt.cmake (1)
21-21
: 更改已批准:改进了库安装配置通过添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
,增强了库安装路径的灵活性。这一更改使得用户可以自定义库的安装目录,提升了构建系统的灵活性。cmake/zstd.cmake (1)
16-16
: 更改已批准:改进了库安装配置通过添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
,明确了库的安装目录,提升了安装配置的灵活性和兼容性。cmake/snappy.cmake (1)
17-17
: 更改已批准:改进了库安装配置通过添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
,增强了库安装路径的灵活性和一致性,使得用户可以自定义库的安装目录。cmake/gtest.cmake (1)
17-17
: 更改已批准:增强安装路径配置添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
提高了库安装路径的灵活性,有助于跨平台兼容性。cmake/spdlog.cmake (1)
24-24
: 更改已批准:增强安装路径配置添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
提高了库安装路径的灵活性,有助于跨平台兼容性。cmake/leveldb.cmake (1)
8-8
: 更改已批准:改进安装路径配置通过使用
${CMAKE_INSTALL_LIBDIR}
和添加-DCMAKE_INSTALL_PREFIX
以及-DCMAKE_INSTALL_LIBDIR
,增强了安装路径的动态配置能力,提高了构建系统的灵活性。Also applies to: 19-20
cmake/gflags.cmake (2)
13-15
: 变量格式化改动这些行的变量格式化改动没有影响功能,但提高了可读性。
24-24
: 添加静态库构建参数添加
-DGFLAGS_BUILD_STATIC_LIBS=ON
参数明确要求构建静态库,这可能会影响构建配置。请确保这符合项目需求。cmake/protobuf.cmake (1)
33-33
: 添加库目录参数添加
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
参数提高了库安装路径的灵活性。这是一个有益的改进。cmake/openssl.cmake (1)
20-22
: 动态库目录引用将
--libdir
参数更改为引用CMAKE_INSTALL_LIBDIR
变量,提高了安装目录的灵活性。这是一个很好的改进。cmake/rocksdb.cmake (1)
26-26
: 更改已批准:添加了库安装目录配置选项。在
ExternalProject_Add
中添加-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
提供了更大的灵活性来指定库的安装目录,这对于需要自定义安装路径的环境尤为有用。cmake/braft.cmake (1)
30-30
: 更改已批准:添加了库安装目录配置选项。在
ExternalProject_Add
中添加-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
提供了更大的灵活性来指定库的安装目录,这对于需要自定义安装路径的环境尤为有用。cmake/brpc.cmake (1)
25-25
: 更改已批准:添加了库安装目录配置选项。在
ExternalProject_Add
中添加-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
提供了更大的灵活性来指定库的安装目录。README.md (4)
11-22
: 编译说明更新正确编译说明更新为推荐使用 Rocky Linux,并使用
dnf
代替yum
,这符合现代化的包管理实践。Tools
Markdownlint
13-13: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
38-38
: 运行说明格式化改进格式化的改进提升了文档的一致性和可读性。
Line range hint
5-7
:Tools
Markdownlint
13-13: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Line range hint
203-205
:Tools
Markdownlint
13-13: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
CMakeLists.txt (5)
8-8
: 引入 GNUInstallDirs 改善跨平台兼容性引入
GNUInstallDirs
模块标准化了安装目录变量,增强了跨平台兼容性。
83-83
: 链接器标志更新链接器标志从
-static-libstdc++
更新为-lstdc++
,这可能解决某些平台上的静态链接问题。
116-118
: 输出目录设置改进新的输出目录设置集中管理构建工件,提升了构建过程的组织性和清晰度。
109-113
: 库安装路径更新库安装路径更新为使用标准化变量,增强了对不同环境的适应性。
169-172
: 新增 CMake 包含文件新增的 gflags、zlib 和 protobuf 包含文件确保了这些库的正确集成。
CMakeLists.txt
Outdated
SET(LIB_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/${EP_BASE_SUFFIX}/Source) | ||
|
||
SET(CMAKE_ARCHIVE_OUTPUT_DIRECTORY_DEBUG ${LIB_INSTALL_DIR}) | ||
SET(CMAKE_ARCHIVE_OUTPUT_DIRECTORY_RLEASE ${LIB_INSTALL_DIR}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个是 ”CMAKE_ARCHIVE_OUTPUT_DIRECTORY_RLEASE“ 应该是 “CMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELEASE” ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
啊是的,这里是我打错了,抱歉哈
SET(CMAKE_ARCHIVE_OUTPUT_DIRECTORY_RLEASE ${LIB_INSTALL_DIR}) | ||
SET(CMAKE_INSTALL_PREFIX ${LIB_INSTALL_DIR}) | ||
SET(LIBDIR ${CMAKE_INSTALL_LIBDIR}) | ||
SET(FIND_LIBRARY_USE_LIB32_PATHS true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“LIBDIR” 的作用是?
需要包含 32 位库的搜索路径么?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
上面提到了,这个参数就是引发编译不成功的原因(cmake 在一部分操作系统上会设置为 lib,一部分是 lib64,结果因为这种不统一,使得不同操作系统的编译行为不一样,进而不能移植,我就直接手动统一参数了)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那这个需要包含 32 位库的搜索路径么
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那这个需要包含 32 位库的搜索路径么
需要,32位库的是 lib 路径
为什么把 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
因为在测试的时候发现,很多 URL HASH 所填写的内容是错误的,导致每一次都需要重新下载,所以直接删除了 |
Because during the test, it was found that the contents filled in many URL HASH were wrong, which required re-downloading every time, so I deleted them directly. |
https://cmake.org/cmake/help/latest/module/ExternalProject.html 如果没有指定 hash 值 每次都会去下载, 我记得如果 填的hash值有问题, 会提示下载错误, 不能编译的, 如果是我填的, 这些值是我手动下载压缩文件后 , 用命令计算的, 应该不会有错 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- CMakeLists.txt (4 hunks)
- cmake/braft.cmake (1 hunks)
- cmake/brpc.cmake (1 hunks)
- cmake/fmt.cmake (1 hunks)
- cmake/gflags.cmake (3 hunks)
- cmake/gtest.cmake (1 hunks)
- cmake/leveldb.cmake (2 hunks)
- cmake/llhttp.cmake (1 hunks)
- cmake/lz4.cmake (2 hunks)
- cmake/openssl.cmake (1 hunks)
- cmake/protobuf.cmake (1 hunks)
- cmake/rocksdb.cmake (2 hunks)
- cmake/snappy.cmake (1 hunks)
- cmake/spdlog.cmake (1 hunks)
- cmake/zlib.cmake (1 hunks)
- cmake/zstd.cmake (1 hunks)
Files skipped from review due to trivial changes (1)
- cmake/gflags.cmake
Files skipped from review as they are similar to previous changes (14)
- CMakeLists.txt
- cmake/braft.cmake
- cmake/brpc.cmake
- cmake/fmt.cmake
- cmake/gtest.cmake
- cmake/leveldb.cmake
- cmake/llhttp.cmake
- cmake/lz4.cmake
- cmake/openssl.cmake
- cmake/protobuf.cmake
- cmake/rocksdb.cmake
- cmake/snappy.cmake
- cmake/zlib.cmake
- cmake/zstd.cmake
Additional comments not posted (1)
cmake/spdlog.cmake (1)
24-24
: 增强灵活性的安装目录配置在
ExternalProject_Add
的CMAKE_ARGS
中添加-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR}
增强了安装过程的灵活性。这允许在非标准位置管理库时指定库安装目录,适用于多平台构建环境。
fix #407
完成针对于 Rocky Linux/Fedora Linux/CentOS 的移植工作,统一了编译的路径,同时删除了哈希验证(目前cmake文件的哈希设置是有问题的,每一次都会全部重新下载,因此删除)。
同时加入设置 git 使用 HTTP 1.1 的内容,规避 error: RPC failed; curl 92 HTTP/2 stream 0 was not closed cleanly: PROTOCOL_ERROR (err 1) 错误。
(在 Fedora Linux 上面的编译截图,顺利通过)
Summary by CodeRabbit
Summary by CodeRabbit