Breaking change: Remove CMake submodule support in favor of fetched o…

…r 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
protocolbuffers · Oct 16, 2024 · 3f06ca4 · 3f06ca4
1 parent 66f80bb
commit 3f06ca4
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 127 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()