Skip to content

build: cmake build optimize,avoid repeated compilation of dependent libraries #229

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cmake/braft.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ExternalProject_Add(
GIT_TAG v1.1.2-beta20250101
GIT_SHALLOW true
SOURCE_DIR ${BRAFT_SOURCES_DIR}
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
${EXTERNAL_PROJECT_CXX}
Expand Down
1 change: 1 addition & 0 deletions cmake/brpc.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ExternalProject_Add(
DOWNLOAD_DIR "${CMAKE_CURRENT_SOURCE_DIR}/download"
DOWNLOAD_NAME "brpc-1.8.0.tar.gz"
SOURCE_DIR ${BRPC_SOURCES_DIR}
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
${EXTERNAL_PROJECT_CXX}
Expand Down
1 change: 1 addition & 0 deletions cmake/fmt.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ ExternalProject_Add(
DOWNLOAD_DIR "${CMAKE_CURRENT_SOURCE_DIR}/download"
DOWNLOAD_NAME "fmt-10.1.1.zip"
SOURCE_DIR ${FMT_SOURCES_DIR}
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
${EXTERNAL_PROJECT_CXX}
Expand Down
1 change: 1 addition & 0 deletions cmake/gflags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ ExternalProject_Add(
DOWNLOAD_DIR "${CMAKE_CURRENT_SOURCE_DIR}/download"
DOWNLOAD_NAME "gflags-2.2.2.zip"
SOURCE_DIR ${GFLAGS_SOURCES_DIR}
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_GENERATOR}
${EXTERNAL_PROJECT_C}
Expand Down
1 change: 1 addition & 0 deletions cmake/gtest.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ExternalProject_Add(
GIT_TAG v1.14.0
GIT_SHALLOW true
SOURCE_DIR ${GTEST_SOURCES_DIR}
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_GENERATOR}
-DCMAKE_INSTALL_PREFIX=${LIB_INSTALL_PREFIX}
Expand Down
1 change: 1 addition & 0 deletions cmake/leveldb.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ExternalProject_Add(
GIT_TAG "1.23"
GIT_SHALLOW true
SOURCE_DIR ${LEVELDB_SOURCES_DIR}
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
${EXTERNAL_PROJECT_CXX}
Expand Down
1 change: 1 addition & 0 deletions cmake/lz4.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ ExternalProject_Add(
DOWNLOAD_NO_PROGRESS 1
${EXTERNAL_PROJECT_LOG_ARGS}
SOURCE_SUBDIR build/cmake
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
${EXTERNAL_PROJECT_CXX}
Expand Down
1 change: 1 addition & 0 deletions cmake/openssl.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ ExternalProject_Add(
DOWNLOAD_NAME "openssl-3.2.1.tar.gz"
SOURCE_DIR ${OPENSSL_SOURCE_DIR}
DOWNLOAD_NO_PROGRESS 1
UPDATE_COMMAND ""
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate UPDATE_COMMAND parameter

There's already an identical UPDATE_COMMAND "" parameter at line 46. Having two identical parameters in the same ExternalProject_Add call is redundant and may lead to confusion.

-        UPDATE_COMMAND ""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
UPDATE_COMMAND ""

USES_TERMINAL_DOWNLOAD TRUE
CONFIGURE_COMMAND
env CC=${CMAKE_C_COMPILER} CXX=${CMAKE_CXX_COMPILER}
Expand Down
1 change: 1 addition & 0 deletions cmake/protobuf.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ ExternalProject_Add(
DOWNLOAD_NAME "protobuf-3.18.0.tar.gz"
SOURCE_DIR ${PROTOBUF_SOURCES_DIR}
DOWNLOAD_NO_PROGRESS 1
UPDATE_COMMAND ""
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
${EXTERNAL_PROJECT_CXX}
Expand Down
1 change: 1 addition & 0 deletions cmake/rocksdb.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ExternalProject_Add(
GIT_REPOSITORY https://github.com/facebook/rocksdb.git
GIT_TAG v9.11.1
GIT_SHALLOW true
UPDATE_COMMAND ""
SOURCE_DIR ${ROCKSDB_SOURCES_DIR}
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
Expand Down
1 change: 1 addition & 0 deletions cmake/snappy.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ ExternalProject_Add(
GIT_REPOSITORY "https://github.com/google/snappy.git"
GIT_TAG "1.2.1"
GIT_SHALLOW true
UPDATE_COMMAND ""
SOURCE_DIR ${Snappy_SOURCES_DIR}
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
Expand Down
1 change: 1 addition & 0 deletions cmake/zlib.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ ExternalProject_Add(
GIT_REPOSITORY "https://github.com/madler/zlib.git"
GIT_TAG "v1.2.8"
GIT_SHALLOW true
UPDATE_COMMAND ""
Copy link

@coderabbitai coderabbitai bot Mar 29, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate UPDATE_COMMAND parameter

There's already an identical UPDATE_COMMAND "" parameter at line 29. Having two identical parameters in the same ExternalProject_Add call is redundant and may lead to confusion.

-        UPDATE_COMMAND ""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
UPDATE_COMMAND ""

Copy link
Collaborator

Choose a reason for hiding this comment

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

看看这个AI的comment,下面的跟这个重复了

Copy link

Choose a reason for hiding this comment

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

@marsevilspirit 是的,您说得对。代码中确实存在重复的 UPDATE_COMMAND "" 参数。一个在第16行(新添加的),另一个在第29行(已经存在的)。在 ExternalProject_Add 函数中只需要一个 UPDATE_COMMAND "" 参数即可达到避免重复编译的目的,保留其中一个就足够了。建议移除其中一处重复的参数。

SOURCE_DIR ${ZLIB_SOURCES_DIR}
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
Expand Down
1 change: 1 addition & 0 deletions cmake/zstd.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ExternalProject_Add(
DOWNLOAD_NAME "zstd-1.5.4.tar.gz"
SOURCE_DIR ${zstd_SOURCES_DIR}
DOWNLOAD_NO_PROGRESS 1
UPDATE_COMMAND ""
Copy link

@coderabbitai coderabbitai bot Mar 29, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Duplicate UPDATE_COMMAND parameter detected.

The UPDATE_COMMAND "" parameter has been added at line 19, but the same parameter already exists at line 34. Having duplicate parameters in the ExternalProject_Add function is redundant and could lead to confusion.

-        UPDATE_COMMAND ""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
UPDATE_COMMAND ""

Copy link
Collaborator

Choose a reason for hiding this comment

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

看看这个AI的comment,下面的跟这个重复了

Copy link

Choose a reason for hiding this comment

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

@marsevilspirit 您说得对,代码中确实存在重复的 UPDATE_COMMAND "" 参数。在 ExternalProject_Add 函数中,同一个参数出现了两次:

  1. 第19行(本次PR新增的)
  2. 第34行(已经存在的)

建议删除新增的那一行(第19行),保留原来的第34行即可。这样可以避免冗余并保持代码的清晰度,同时仍然能够实现PR目标:通过设置空的更新命令来优化构建过程,避免重复编译依赖库。

SOURCE_SUBDIR build/cmake
CMAKE_ARGS
${EXTERNAL_PROJECT_C}
Expand Down