From 3f06ca4306a682e6ee631d8ea94b82baaafb14f0 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Wed, 16 Oct 2024 14:48:25 -0700 Subject: [PATCH] Breaking change: Remove CMake submodule support in favor of fetched or installed dependencies. This flips the default behavior to "fetch", downloading local copies of required dependencies. This can be disabled by setting `-Dprotobuf_FETCH_DEPENDENCIES=OFF`, in which case we will look for a local installation using find_package. Setting `-Dprotobuf_ABSL_PROVIDER=package` will continue to have the same behavior as before. See https://protobuf.dev/news/2024-10-02/#replace-cmake-submods for more details. #test-continuous PiperOrigin-RevId: 686649864 --- .github/workflows/test_cpp.yml | 54 +++++++++----------------------- CMakeLists.txt | 10 +++--- cmake/README.md | 11 ++----- cmake/abseil-cpp.cmake | 18 ++--------- cmake/conformance.cmake | 16 +++------- cmake/gtest.cmake | 56 +++++++--------------------------- 6 files changed, 38 insertions(+), 127 deletions(-) diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index e9ab589a10..6d4c946e13 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -131,7 +131,7 @@ jobs: sccache -z; cmake . -DWITH_PROTOC=/workspace/${{ steps.cross-compile.outputs.protoc }} -Dprotobuf_BUILD_LIBUPB=OFF -Dprotobuf_BUILD_CONFORMANCE=ON -DCMAKE_CXX_STANDARD=14 - -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_FETCH_DEPENDENCIES=ON ${{ env.SCCACHE_CMAKE_FLAGS }}; + -Dprotobuf_WITH_ZLIB=OFF ${{ env.SCCACHE_CMAKE_FLAGS }}; cmake --build . --parallel 20; ctest --parallel 20; sccache -s" @@ -141,19 +141,19 @@ jobs: fail-fast: false # Don't cancel all jobs if one fails. matrix: include: - - flags: -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_JSONCPP_PROVIDER=package + - flags: -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF - name: Ninja - flags: -G Ninja -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_JSONCPP_PROVIDER=package + flags: -G Ninja -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF continuous-only: true - name: Shared - flags: -Dprotobuf_BUILD_SHARED_LIBS=ON -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_JSONCPP_PROVIDER=package + flags: -Dprotobuf_BUILD_SHARED_LIBS=ON -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF continuous-only: true - name: C++17 - flags: -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_JSONCPP_PROVIDER=package + flags: -DCMAKE_CXX_STANDARD=17 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF - name: C++20 - flags: -DCMAKE_CXX_STANDARD=20 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_JSONCPP_PROVIDER=package + flags: -DCMAKE_CXX_STANDARD=20 -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF - name: Fetch - flags: -DCMAKE_CXX_STANDARD=17 -Dprotobuf_FETCH_DEPENDENCIES=ON + flags: -DCMAKE_CXX_STANDARD=17 name: ${{ matrix.continuous-only && inputs.continuous-prefix || '' }} Linux CMake ${{ matrix.name}} runs-on: ubuntu-latest @@ -190,12 +190,11 @@ jobs: # Set defaults - type: package name: Install - flags: -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_JSONCPP_PROVIDER=package + flags: -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF - type: fetch name: Install (Fetch) - flags: -Dprotobuf_FETCH_DEPENDENCIES=ON continuous-only: true - name: ${{ matrix.continuous-only && inputs.continuous-prefix || '' }}Linux CMake ${{ matrix.name }}) + name: ${{ matrix.continuous-only && inputs.continuous-prefix || '' }}Linux CMake ${{ matrix.name }} runs-on: ubuntu-latest steps: - name: Checkout pending changes @@ -255,7 +254,7 @@ jobs: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: >- /install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }} - -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package + -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_FETCH_DEPENDENCIES=OFF -Dprotobuf_BUILD_EXAMPLES=OFF \&\& mkdir examples/build \&\& cd examples/build \&\& @@ -302,36 +301,11 @@ jobs: -c 'set -ex; cd /workspace; sccache -z; - cmake . ${{ matrix.flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_FETCH_DEPENDENCIES=ON; + cmake . ${{ matrix.flags }} ${{ env.SCCACHE_CMAKE_FLAGS }}; cmake --build . --parallel 20; ctest --verbose --parallel 20; sccache -s' - linux-cmake-submodules: - name: Linux CMake Submodules - runs-on: ubuntu-latest - steps: - - name: Checkout pending changes - uses: protocolbuffers/protobuf-ci/checkout@v3 - with: - ref: ${{ inputs.safe-checkout }} - submodules: recursive - - - name: Setup sccache - uses: protocolbuffers/protobuf-ci/sccache@v3 - with: - cache-prefix: linux-cmake-submodules - credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - - - name: Run tests - uses: protocolbuffers/protobuf-ci/docker@v3 - with: - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.16.9-f39fc8b4e244fe5cd4c7138d0b6959a52b46ca48 - credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - command: >- - /test.sh ${{ env.SCCACHE_CMAKE_FLAGS }} - -Dprotobuf_BUILD_CONFORMANCE=ON -Dprotobuf_BUILD_EXAMPLES=ON -DCMAKE_CXX_STANDARD=14 - linux-cmake-32-bit: name: Linux CMake 32-bit runs-on: ubuntu-latest @@ -357,7 +331,7 @@ jobs: /bin/bash -cex ' cd /workspace; sccache -z; - cmake . -DCMAKE_CXX_STANDARD=14 -Dprotobuf_FETCH_DEPENDENCIES=ON ${{ env.SCCACHE_CMAKE_FLAGS }}; + cmake . -DCMAKE_CXX_STANDARD=14 ${{ env.SCCACHE_CMAKE_FLAGS }}; cmake --build . --parallel 20; ctest --verbose --parallel 20; sccache -s' @@ -511,7 +485,7 @@ jobs: uses: protocolbuffers/protobuf-ci/bash@v3 with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - command: cmake . ${{ matrix.install-flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON -Dprotobuf_FETCH_DEPENDENCIES=ON + command: cmake . ${{ matrix.install-flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON - name: Build for install if: ${{ matrix.install-flags && (!matrix.continuous-only || inputs.continuous-run) }} shell: bash @@ -534,7 +508,7 @@ jobs: uses: protocolbuffers/protobuf-ci/bash@v3 with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - command: cmake . ${{ matrix.flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON -Dprotobuf_FETCH_DEPENDENCIES=ON + command: cmake . ${{ matrix.flags }} ${{ env.SCCACHE_CMAKE_FLAGS }} -Dprotobuf_ALLOW_CCACHE=ON - name: Build if: ${{ !matrix.continuous-only || inputs.continuous-run }} diff --git a/CMakeLists.txt b/CMakeLists.txt index 1cebc038d9..e2478ba50c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -281,13 +281,13 @@ include_directories( ${protobuf_BINARY_DIR}/src ${protobuf_SOURCE_DIR}/src) -set(protobuf_FETCH_DEPENDENCIES OFF CACHE BOOL "Download dependencies from GitHub. If this option is not set, the dependency must be available locally, either as a sub-module or an installed package.") +set(protobuf_FETCH_DEPENDENCIES ON CACHE BOOL "Allow downloading dependencies from GitHub. If this option is not set, the dependency must be available locally as an installed package.") -set(protobuf_ABSL_PROVIDER "module" CACHE STRING "Provider of absl library. `module` uses sub-modules, `package` searches for a local installation, and `fetch` downloads from GitHub") -set_property(CACHE protobuf_ABSL_PROVIDER PROPERTY STRINGS "module" "package" "fetch") +set(protobuf_ABSL_PROVIDER "fetch" CACHE STRING "Provider of absl library. `fetch` downloads from GitHub and `package` searches for a local installation") +set_property(CACHE protobuf_ABSL_PROVIDER PROPERTY STRINGS "package" "fetch") -set(protobuf_JSONCPP_PROVIDER "module" CACHE STRING "Provider of jsoncpp library. `module` uses sub-modules, `package` searches for a local installation, and `fetch` downloads from GitHub") -set_property(CACHE protobuf_JSONCPP_PROVIDER PROPERTY STRINGS "module" "package" "fetch") +set(protobuf_JSONCPP_PROVIDER "fetch" CACHE STRING "Provider of jsoncpp library. `fetch` downloads from GitHub and `package` searches for a local installation") +set_property(CACHE protobuf_JSONCPP_PROVIDER PROPERTY STRINGS "package" "fetch") if (protobuf_BUILD_TESTS) include(${protobuf_SOURCE_DIR}/cmake/gtest.cmake) diff --git a/cmake/README.md b/cmake/README.md index 00532ec683..4c45190ebd 100644 --- a/cmake/README.md +++ b/cmake/README.md @@ -82,13 +82,6 @@ Go to the project folder: C:\Path\to\src> cd protobuf C:\Path\to\src\protobuf> -Remember to update any submodules if you are using git clone (you can skip this -step if you are using a release .tar.gz or .zip package): - -```console -C:\Path\to\src\protobuf> git submodule update --init --recursive -``` - Good. Now you are ready for *CMake* configuration. ## CMake Configuration @@ -156,8 +149,8 @@ It will generate *Visual Studio* solution file *protobuf.sln* in current directo Unit tests are being built along with the rest of protobuf. The unit tests require Google Mock (now a part of Google Test). -A copy of [Google Test](https://github.com/google/googletest) is included as a Git submodule in the `third-party/googletest` folder. -(You do need to initialize the Git submodules as explained above.) +By default, a local copy of [Google Test](https://github.com/google/googletest) +will be downloaded during CMake configuration. Alternately, you may want to use protobuf in a larger set-up, you may want to use that standard CMake approach where you build and install a shared copy of Google Test. diff --git a/cmake/abseil-cpp.cmake b/cmake/abseil-cpp.cmake index 688ca49fa2..4c8580cc40 100644 --- a/cmake/abseil-cpp.cmake +++ b/cmake/abseil-cpp.cmake @@ -13,7 +13,7 @@ endif() if(TARGET absl::strings) # If Abseil is included already, skip including it. # (https://github.com/protocolbuffers/protobuf/issues/10435) -elseif (protobuf_FETCH_DEPENDENCIES OR protobuf_ABSL_PROVIDER STREQUAL "fetch") +elseif (protobuf_FETCH_DEPENDENCIES AND protobuf_ABSL_PROVIDER STREQUAL "fetch") include(${protobuf_SOURCE_DIR}/cmake/dependencies.cmake) include(FetchContent) FetchContent_Declare( @@ -27,21 +27,7 @@ elseif (protobuf_FETCH_DEPENDENCIES OR protobuf_ABSL_PROVIDER STREQUAL "fetch") set(ABSL_ENABLE_INSTALL ON) endif() FetchContent_MakeAvailable(absl) -elseif(protobuf_ABSL_PROVIDER STREQUAL "module") - if(NOT ABSL_ROOT_DIR) - set(ABSL_ROOT_DIR ${CMAKE_CURRENT_SOURCE_DIR}/third_party/abseil-cpp) - endif() - if(EXISTS "${ABSL_ROOT_DIR}/CMakeLists.txt") - if(protobuf_INSTALL) - # When protobuf_INSTALL is enabled and Abseil will be built as a module, - # Abseil will be installed along with protobuf for convenience. - set(ABSL_ENABLE_INSTALL ON) - endif() - add_subdirectory(${ABSL_ROOT_DIR} third_party/abseil-cpp) - else() - message(WARNING "protobuf_ABSL_PROVIDER is \"module\" but ABSL_ROOT_DIR is wrong") - endif() -elseif(protobuf_ABSL_PROVIDER STREQUAL "package") +else () # Use "CONFIG" as there is no built-in cmake module for absl. find_package(absl REQUIRED CONFIG) endif() diff --git a/cmake/conformance.cmake b/cmake/conformance.cmake index 87961df7fa..a819d2cd80 100644 --- a/cmake/conformance.cmake +++ b/cmake/conformance.cmake @@ -1,7 +1,9 @@ # Don't run jsoncpp tests. set(JSONCPP_WITH_TESTS OFF) -if (protobuf_FETCH_DEPENDENCIES OR protobuf_JSONCPP_PROVIDER STREQUAL "fetch") +if (TARGET jsoncpp_lib) + # jsoncpp is already present. +elseif (protobuf_FETCH_DEPENDENCIES AND protobuf_JSONCPP_PROVIDER STREQUAL "fetch") include(${protobuf_SOURCE_DIR}/cmake/dependencies.cmake) include(FetchContent) FetchContent_Declare( @@ -11,17 +13,7 @@ if (protobuf_FETCH_DEPENDENCIES OR protobuf_JSONCPP_PROVIDER STREQUAL "fetch") GIT_TAG "1.9.4" ) FetchContent_MakeAvailable(jsoncpp) -elseif (protobuf_JSONCPP_PROVIDER STREQUAL "module") - if (NOT EXISTS "${protobuf_SOURCE_DIR}/third_party/jsoncpp/CMakeLists.txt") - message(FATAL_ERROR - "Cannot find third_party/jsoncpp directory that's needed to " - "build conformance tests. If you use git, make sure you have cloned " - "submodules:\n" - " git submodule update --init --recursive\n" - "If instead you want to skip them, run cmake with:\n" - " cmake -Dprotobuf_BUILD_CONFORMANCE=OFF\n") - endif() -elseif(protobuf_JSONCPP_PROVIDER STREQUAL "package") +else () find_package(jsoncpp REQUIRED) endif() diff --git a/cmake/gtest.cmake b/cmake/gtest.cmake index 680ae7f265..66e17cbf20 100644 --- a/cmake/gtest.cmake +++ b/cmake/gtest.cmake @@ -1,8 +1,17 @@ option(protobuf_USE_EXTERNAL_GTEST "Use external Google Test (i.e. not the one in third_party/googletest)" OFF) -if (protobuf_USE_EXTERNAL_GTEST) +if (TARGET GTest::gmock) + # GTest is already present. +elseif (protobuf_USE_EXTERNAL_GTEST) find_package(GTest REQUIRED CONFIG) -elseif (protobuf_FETCH_DEPENDENCIES) +else () + if (NOT protobuf_FETCH_DEPENDENCIES) + message(FATAL_ERROR + "Cannot find local googletest directory that's needed to " + "build tests.\n" + "If instead you want to skip tests, run cmake with:\n" + " cmake -Dprotobuf_BUILD_TESTS=OFF\n") + endif() include(${protobuf_SOURCE_DIR}/cmake/dependencies.cmake) include(FetchContent) FetchContent_Declare( @@ -13,47 +22,4 @@ elseif (protobuf_FETCH_DEPENDENCIES) # Due to https://github.com/google/googletest/issues/4384, we can't name this # GTest for use with find_package until 1.15.0. FetchContent_MakeAvailable(googletest) -else() - if (NOT EXISTS "${protobuf_SOURCE_DIR}/third_party/googletest/CMakeLists.txt") - message(FATAL_ERROR - "Cannot find third_party/googletest directory that's needed to " - "build tests. If you use git, make sure you have cloned submodules:\n" - " git submodule update --init --recursive\n" - "If instead you want to skip tests, run cmake with:\n" - " cmake -Dprotobuf_BUILD_TESTS=OFF\n") - endif() - - set(googlemock_source_dir "${protobuf_SOURCE_DIR}/third_party/googletest/googlemock") - set(googletest_source_dir "${protobuf_SOURCE_DIR}/third_party/googletest/googletest") - include_directories( - ${googlemock_source_dir} - ${googletest_source_dir} - ${googletest_source_dir}/include - ${googlemock_source_dir}/include - ) - - add_library(gmock ${protobuf_SHARED_OR_STATIC} - "${googlemock_source_dir}/src/gmock-all.cc" - "${googletest_source_dir}/src/gtest-all.cc" - ) - if (protobuf_BUILD_SHARED_LIBS) - set_target_properties(gmock - PROPERTIES - COMPILE_DEFINITIONS - "GTEST_CREATE_SHARED_LIBRARY=1" - ) - - endif() - if (protobuf_INSTALL) - set(protobuf_INSTALL_TESTS ON) - endif() - - target_link_libraries(gmock ${CMAKE_THREAD_LIBS_INIT}) - add_library(gmock_main STATIC "${googlemock_source_dir}/src/gmock_main.cc") - target_link_libraries(gmock_main gmock) - - add_library(GTest::gmock ALIAS gmock) - add_library(GTest::gmock_main ALIAS gmock_main) - add_library(GTest::gtest ALIAS gmock) - add_library(GTest::gtest_main ALIAS gmock_main) endif()