From 54caf40312b3e7fd7794e267ef17e3be202de83d Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 5 May 2023 16:24:31 -0700 Subject: [PATCH 1/2] Avoid using string(JOIN..., which requires cmake 3.12 This also downgrades our tests to use cmake 3.10, compatible with our support window. Closes #12672 PiperOrigin-RevId: 529840763 --- .github/workflows/test_cpp.yml | 16 ++++++++-------- cmake/install.cmake | 8 ++++++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index 70f3a75a91..aae5df2c73 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -116,7 +116,7 @@ jobs: - name: Run tests uses: protocolbuffers/protobuf-ci/docker@v1 with: - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake@sha256:7058879cac0224e443043eae2de82c13a63a54dd31855dac020c5522aa573cd2 + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: >- /test.sh ${{ matrix.flags}} ${{ env.CCACHE_CMAKE_FLAGS }} @@ -142,7 +142,7 @@ jobs: - name: Run tests uses: protocolbuffers/protobuf-ci/docker@v1 with: - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake@sha256:7058879cac0224e443043eae2de82c13a63a54dd31855dac020c5522aa573cd2 + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: >- /install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.CCACHE_CMAKE_FLAGS }} -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package \&\& @@ -171,16 +171,16 @@ jobs: - name: Run tests uses: protocolbuffers/protobuf-ci/docker@v1 with: - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake@sha256:7058879cac0224e443043eae2de82c13a63a54dd31855dac020c5522aa573cd2 + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: >- /install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.CCACHE_CMAKE_FLAGS }} -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -Dprotobuf_BUILD_EXAMPLES=OFF \&\& - cd examples \&\& - mkdir build \&\& - cmake -S . -B build -DCMAKE_CXX_STANDARD=14 \&\& - cmake --build build + mkdir examples/build \&\& + cd examples/build \&\& + cmake .. -DCMAKE_CXX_STANDARD=14 \&\& + cmake --build . linux-cmake-submodules: name: Linux CMake Submodules @@ -200,7 +200,7 @@ jobs: - name: Run tests uses: protocolbuffers/protobuf-ci/docker@v1 with: - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake@sha256:e0eb6c69b7551d89f0dbdbe11906077a1d501229c28db39623b945e0c5d7029a + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.13.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} command: >- /test.sh ${{ env.CCACHE_CMAKE_FLAGS }} diff --git a/cmake/install.cmake b/cmake/install.cmake index af891ca0c3..9d837ac536 100644 --- a/cmake/install.cmake +++ b/cmake/install.cmake @@ -6,8 +6,12 @@ foreach(_target IN LISTS protobuf_ABSL_USED_TARGETS) endforeach() list(APPEND _pc_targets "utf8_range") -string(JOIN " " _pc_target_list ${_pc_targets}) -set(_protobuf_PC_REQUIRES ${_pc_target_list}) +set(_protobuf_PC_REQUIRES "") +set(_sep "") +foreach (_target IN LISTS _pc_target_list) + string(CONCAT _protobuf_PC_REQUIRES "${_protobuf_PC_REQUIRES}" "${_sep}" "${_target}") + set(_sep " ") +endforeach () configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/protobuf.pc.cmake ${CMAKE_CURRENT_BINARY_DIR}/protobuf.pc @ONLY) From bc9a1d219e9b0043036c6af612ae0e0de3369e0c Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 5 May 2023 21:25:58 -0700 Subject: [PATCH 2/2] Add tests for GCC support window. Note: gcc only supports docker images down to 9.5, and the 7.3 image is very old and problematic. A follow-up change might enable testing for GCC 7.3, which is our minimal supported version PiperOrigin-RevId: 529885733 --- .github/workflows/test_cpp.yml | 51 +++++++++++++++++++- src/google/protobuf/io/printer_death_test.cc | 1 + src/google/protobuf/port_def.inc | 6 +++ src/google/protobuf/repeated_field.h | 2 +- 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index aae5df2c73..f4d1436ae1 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -51,6 +51,26 @@ jobs: bazel-cache: cpp_linux/${{ matrix.config.name }} bazel: test ${{ matrix.targets }} ${{ matrix.config.flags }} + linux-gcc: + strategy: + fail-fast: false # Don't cancel all jobs if one fails. + matrix: + version: ['9.5', '13.1'] + name: Linux GCC ${{ matrix.version }} + runs-on: ubuntu-latest + steps: + - name: Checkout pending changes + uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 + with: + ref: ${{ inputs.safe-checkout }} + - name: Run tests + uses: protocolbuffers/protobuf-ci/bazel-docker@v1 + with: + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:${{ matrix.version }}-5.4.0-2d15d9e888c9e7f90961dbd3afc8ea209717fb4b + credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} + bazel-cache: cpp_linux/gcc-${{ matrix.version }} + bazel: test //pkg/... //src/... @com_google_protobuf_examples//... + linux-release: strategy: fail-fast: false # Don't cancel all jobs if one fails. @@ -122,7 +142,6 @@ jobs: /test.sh ${{ matrix.flags}} ${{ env.CCACHE_CMAKE_FLAGS }} -DCMAKE_CXX_STANDARD=14 -Dprotobuf_BUILD_TESTS=ON -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package - linux-cmake-install: name: Linux CMake Install @@ -182,6 +201,36 @@ jobs: cmake .. -DCMAKE_CXX_STANDARD=14 \&\& cmake --build . + linux-cmake-gcc: + name: Linux CMake GCC + runs-on: ubuntu-latest + steps: + - name: Checkout pending changes + uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 + with: + ref: ${{ inputs.safe-checkout }} + submodules: recursive + + - name: Setup ccache + uses: protocolbuffers/protobuf-ci/ccache@v1 + with: + cache-prefix: linux-cmake-gcc + + - name: Run tests + uses: protocolbuffers/protobuf-ci/docker@v1 + with: + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/gcc:13.1-5.4.0-307caa02808127e49720f3e77d6a9f3b3ef5a915 + credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} + entrypoint: bash + command: >- + -c 'set -ex; + cd /workspace; + ccache -z; + cmake . -DCMAKE_CXX_STANDARD=14 ${{ env.CCACHE_CMAKE_FLAGS }}; + cmake --build . --parallel 20; + ctest --verbose --parallel 20; + ccache -s' + linux-cmake-submodules: name: Linux CMake Submodules runs-on: ubuntu-latest diff --git a/src/google/protobuf/io/printer_death_test.cc b/src/google/protobuf/io/printer_death_test.cc index 7365418273..992e3ca7ef 100644 --- a/src/google/protobuf/io/printer_death_test.cc +++ b/src/google/protobuf/io/printer_death_test.cc @@ -90,6 +90,7 @@ class FakeAnnotationCollector : public AnnotationCollector { public: ~FakeAnnotationCollector() override = default; + using AnnotationCollector::AddAnnotation; void AddAnnotation(size_t begin_offset, size_t end_offset, const std::string& file_path, const std::vector& path) override { diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 204e60140a..a18fcc4912 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -1013,6 +1013,12 @@ static_assert(PROTOBUF_CPLUSPLUS_MIN(201402L), "Protobuf only supports C++14 and // This error has been generally flaky, but we need to disable it specifically // to fix https://github.com/protocolbuffers/protobuf/issues/12313 #pragma GCC diagnostic ignored "-Wunused-parameter" +#ifndef __clang__ +// This causes spurious warnings in GCC 13. +#pragma GCC diagnostic ignored "-Wstringop-overflow" +// This causes spurious warnings in GCC 13. +#pragma GCC diagnostic ignored "-Wself-move" +#endif #if __GNUC__ == 12 && __GNUC_MINOR__ < 4 // Wrong warning emitted when assigning a single char c-string to a std::string // in c++20 mode and optimization on. diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index d5e531a197..bfcb93833d 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -977,7 +977,7 @@ PROTOBUF_NOINLINE void RepeatedField::GrowNoAnnotate(int current_size, Element* pold = elements(); // TODO(b/263791665): add absl::is_trivially_relocatable if (std::is_trivial::value) { - memcpy(pnew, pold, current_size * sizeof(Element)); + memcpy(static_cast(pnew), pold, current_size * sizeof(Element)); } else { for (Element* end = pnew + current_size; pnew != end; ++pnew, ++pold) { ::new (static_cast(pnew)) Element(std::move(*pold));