From 444f9242f54939729a6c46e6adbae2c5f36f7fde Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 10 Feb 2023 09:30:14 -0800 Subject: [PATCH 01/13] Add documentation for our new GHA infrastructure PiperOrigin-RevId: 508680514 --- .github/README.md | 196 ++++++++++++++++++++++++++++++++++++++++++++++ ci/README.md | 17 ++++ 2 files changed, 213 insertions(+) create mode 100644 .github/README.md create mode 100644 ci/README.md diff --git a/.github/README.md b/.github/README.md new file mode 100644 index 0000000000..b1c780323c --- /dev/null +++ b/.github/README.md @@ -0,0 +1,196 @@ +This directory contains all of our automatically triggered workflows. + +# Test runner + +Our top level `test_runner.yml` is responsible for kicking off all tests, which +are represented as reusable workflows. This is carefully constructed to satisfy +the design laid out in go/protobuf-gha-protected-resources (see below), and +duplicating it across every workflow file would be difficult to maintain. As an +added bonus, we can manually dispatch our full test suite with a single button +and monitor the progress of all of them simultaneously in GitHub's actions UI. + +There are five ways our test suite can be triggered: + +- **Post-submit tests** (`push`): These are run over newly submitted code +that we can assume has been thoroughly reviewed. There are no additional +security concerns here and these jobs can be given highly privileged access to +our internal resources and caches. + +- **Pre-submit tests from a branch** (`push_request`): These are run over +every PR as changes are made. Since they are coming from branches in our +repository, they have secret access by default and can also be given highly +privileged access. However, we expect *many* of these events per change, +and likely many from abandoned/exploratory changes. Given the much higher +frequency, we restrict the ability to *write* to our more expensive caches. + +- **Pre-submit tests from a fork** (`push_request_target`): These are run +over every PR from a forked repository as changes are made. These have much +more restricted access, since they could be coming from anywhere. To protect +our secret keys and our resources, tests will not run until a commit has been +labeled `safe to submit`. Further commits will require further approvals to +run our test suite. Once marked as safe, we will provide read-only access to +our caches and Docker images, but will generally disallow any writes to shared +resources. + +- **Continuous tests** (`schedule`): These are run on a fixed schedule. We +currently have them set up to run daily, and can help identify non-hermetic +issues in tests that don't get run often (such as due to test caching) or during +slow periods like weekends and holidays. Similar to post-submit tests, these +are run over submitted code and are highly privileged in the resources they +can use. + +- **Manual testing** (`workflow_dispatch`): Our test runner can be triggered +manually over any branch. This is treated similarly to pre-submit tests, +which should be highly privileged because they can only be triggered by the +protobuf team. + +# Staleness handling + +While Bazel handles code generation seamlessly, we do support build systems that +don't. There are a handful of cases where we need to check in generated files +that can become stale over time. In order to provide a good developer +experience, we've implemented a system to make this more manageable. + +- Stale files should have a corresponding `staleness_test` Bazel target. This +should be marked `manual` to avoid getting picked up in CI, but will fail if +files become stale. It also provides a `--fix` flag to update the stale files. + +- Bazel tests will never depend on the checked-in versions, and will generate +new ones on-the-fly during build. + +- Non-Bazel tests will always regenerate necessary files before starting. This +is done using our `bash` and `docker` actions, which should be used for any +non-Bazel tests. This way, no tests will fail due to stale files. + +- A post-submit job will immediately regenerate any stale files and commit them +if they've changed. + +- A scheduled job will run late at night every day to make sure the post-submit +is working as expected (that is, it will run all the staleness tests). + +The `regenerate_stale_files.sh` script is the central script responsible for all +the re-generation of stale files. + +# Caches + +We have a number of different caching strategies to help speed up tests. These +live either in GCP buckets or in our GitHub repository cache. The former has +a lot of resources available and we don't have to worry as much about bloat. +On the other hand, the GitHub repository cache is limited to 10GB, and will +start pruning old caches when it exceeds that threshold. Therefore, we need +to be very careful about the size and quantity of our caches in order to +maximize the gains. + +## Bazel remote cache + +As described in https://bazel.build/remote/caching, remote caching allows us to +offload a lot of our build steps to a remote server that holds a cache of +previous builds. We use our GCP project for this storage, and configure +*every* Bazel call to use it. This provides substantial performance +improvements at minimal cost. + +We do not allow forked PRs to upload updates to our Bazel caches, but they +do use them. Every other event is given read/write access to the caches. +Because Bazel behaves poorly under certain environment changes (such as +toolchain, operating system), we try to use finely-grained caches. Each job +should typically have its own cache to avoid cross-pollution. + +## Bazel repository cache + +When Bazel starts up, it downloads all the external dependencies for a given +build and stores them in the repository cache. This cache is *separate* from +the remote cache, and only exists locally. Because we have so many Bazel +dependencies, this can be a source of frequent flakes due to network issues. + +To avoid this, we keep a cached version of the repository cache in GitHub's +action cache. Our full set of repository dependencies ends up being ~300MB, +which is fairly expensive given our 10GB maximum. The most expensive ones seem +to come from Java, which has some very large downstream dependencies. + +Given the cost, we take a more conservative approach for this cache. Only push +events will ever write to this cache, but all events can read from them. +Additionally, we only store three caches for any given commit, one per platform. +This means that multiple jobs are trying to update the same cache, leading to a +race. GitHub rejects all but one of these updates, so we designed the system so +that caches are only updated if they've actually changed. That way, over time +(and multiple pushes) the repository caches will incrementally grow to encompass +all of our dependencies. A scheduled job will run monthly to clear these caches +to prevent unbounded growth as our dependencies evolve. + +## ccache + +In order to speed up non-Bazel builds to be on par with Bazel, we make use of +[ccache](https://ccache.dev/). This intercepts all calls to the compiler, and +caches the result. Subsequent calls with a cache-hit will very quickly +short-circuit and return the already computed result. This has minimal affect +on any *single* job, since we typically only run a single build. However, by +caching the ccache results in GitHub's action cache we can substantially +decrease the build time of subsequent runs. + +One useful feature of ccache is that you can set a maximum cache size, and it +will automatically prune older results to keep below that limit. On Linux and +Mac cmake builds, we generally get 30MB caches and set a 100MB cache limit. On +Windows, with debug symbol stripping we get ~70MB and set a 200MB cache limit. + +Because CMake build tend to be our slowest, bottlenecking the entire CI process, +we use a fairly expensive strategy with ccache. All events will cache their +ccache directory, keyed by the commit and the branch. This means that each +PR and each branch will write its own set of caches. When looking up which +cache to use initially, each job will first look for a recent cache in its +current branch. If it can't find one, it will accept a cache from the base +branch (for example, PRs will initially use the latest cache from their target +branch). + +While the ccache caches quickly over-run our GitHub action cache, they also +quickly become useless. Since GitHub prunes caches based on the time they were +last used, this just means that we'll see quicker turnover. + +## Bazelisk + +Bazelisk will automatically download a pinned version of Bazel on first use. +This can lead to flakes, and to avoid that we cache the result keyed on the +Bazel version. Only push events will write to this cache, but it's unlikely +to change very often. + +## Docker images + +Instead of downloading a fresh Docker image for every test run, we can save it +as a tar and cache it using `docker image save` and later restore using +`docker image load`. This can decrease download times and also reduce flakes. +Note, Docker's load can actually be significantly slower than a pull in certain +situations. Therefore, we should reserve this strategy for only Docker images +that are causing noticeable flakes. + +## Pip dependencies + +The actions/setup-python action we use for Python supports automated caching +of pip dependencies. We enable this to avoid having to download these +dependencies on every run, which can lead to flakes. + +# Custom actions + +We've defined a number of custom actions to abstract out shared pieces of our +workflows. + +- **Bazel** use this for running all Bazel tests. It can take either a single +Bazel command or a more general bash command. In the latter case, it provides +environment variables for running Bazel with all our standardized settings. + +- **Bazel-Docker** nearly identical to the **Bazel** action, this additionally +runs everything in a specified Docker image. + +- **Bash** use this for running non-Bazel tests. It takes a bash command and +runs it verbatim. It also handles the regeneration of stale files (which does +use Bazel), which non-Bazel tests might depend on. + +- **Docker** nearly identical to the **Bash** action, this additionally runs +everything in a specified Docker image. + +- **ccache** this sets up a ccache environment, and initializes some +environment variables for standardized usage of ccache. + +- **Cross-compile protoc** this abstracts out the compilation of protoc using +our cross-compilation infrastructure. It will set a `PROTOC` environment +variable that gets automatically picked up by a lot of our infrastructure. +This is most useful in conjunction with the **Bash** action with non-Bazel +tests. diff --git a/ci/README.md b/ci/README.md new file mode 100644 index 0000000000..01c8373145 --- /dev/null +++ b/ci/README.md @@ -0,0 +1,17 @@ +This directory contains CI-specific tooling. + +# Clang wrappers + +CMake allows for compiler wrappers to be injected such as ccache, which +intercepts compiler calls and short-circuits on cache-hits. This can be done +by specifying `CMAKE_C_COMPILER_LAUNCHER` and `CMAKE_CXX_COMPILER_LAUNCHER` +during CMake's configure step. Unfortunately, X-Code doesn't provide anything +like this, so we use basic wrapper scripts to invoke ccache + clang. + +# Bazelrc files + +In order to allow platform-specific `.bazelrc` flags during testing, we keep +3 different versions here along with a shared `common.bazelrc` that they all +include. Our GHA infrastructure will select the appropriate file for any test +and overwrite the default `.bazelrc` in our workspace, which is intended for +development only. From 70e14e515cd4209f44b8d58056ef6c1762e12f86 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 10 Feb 2023 10:49:56 -0800 Subject: [PATCH 02/13] Fix remove label action PiperOrigin-RevId: 508701071 --- .github/workflows/test_runner.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_runner.yml b/.github/workflows/test_runner.yml index 1ef929f191..6bf33e6a1f 100644 --- a/.github/workflows/test_runner.yml +++ b/.github/workflows/test_runner.yml @@ -91,7 +91,7 @@ jobs: steps: - uses: actions-ecosystem/action-remove-labels@2ce5d41b4b6aa8503e285553f75ed56e0a40bae0 # v1.3.0 with: - labels: safe for tests + labels: ':a: safe for tests' # Note: this pattern of passing the head sha is vulnerable to PWN requests for # pull_request_target events. We carefully limit those workflows to require a From 60e63637b5f77477ef172984dd4a4a28235a072b Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 10 Feb 2023 10:51:27 -0800 Subject: [PATCH 03/13] Explicitly ban changes to workflow files from forked PRs Changes to these files *can't* be tested in forked PRs, so we should explicitly block them with an error message to explain why. PiperOrigin-RevId: 508701535 --- .github/README.md | 8 ++++++ .../workflows/forked_pr_workflow_check.yml | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 .github/workflows/forked_pr_workflow_check.yml diff --git a/.github/README.md b/.github/README.md index b1c780323c..5c5a6a3a43 100644 --- a/.github/README.md +++ b/.github/README.md @@ -71,6 +71,14 @@ is working as expected (that is, it will run all the staleness tests). The `regenerate_stale_files.sh` script is the central script responsible for all the re-generation of stale files. +# Forked PRs + +Because we need secret access to run our tests, we use the `pull_request_target` +event for PRs coming from forked repositories. We do checkout the code from the +PR's head, but the workflow files themselves are always fetched from the *base* +branch (that is, the branch we're merging to). Therefore, any changes to these +files won't be tested, so we explicitly ban PRs that touch these files. + # Caches We have a number of different caching strategies to help speed up tests. These diff --git a/.github/workflows/forked_pr_workflow_check.yml b/.github/workflows/forked_pr_workflow_check.yml new file mode 100644 index 0000000000..28be6bbb73 --- /dev/null +++ b/.github/workflows/forked_pr_workflow_check.yml @@ -0,0 +1,27 @@ +name: Forked PR workflow check + +# This workflow prevents modifications to our workflow files in PRs from forked +# repositories. Since tests in these PRs always use the workflows in the +# *target* branch, modifications to these files can't be properly tested. + +on: + # safe presubmit + pull_request: + branches: + - main + - '[0-9]+.x' + # The 21.x branch still uses Kokoro + - '!21.x' + # For testing purposes so we can stage this on the `gha` branch. + - gha + paths: + - '.github/workflows/**' + +jobs: + check: + name: Check PR source + runs-on: ubuntu-latest + steps: + - run: > + ${{ github.event.pull_request.head.repo.full_name != 'protocolbuffers/protobuf' }} || + (echo "This pull request is from an unsafe fork and isn't allowed to modify workflow files!" && exit 1) From 34cd431f574edd086c04626f6dbebb10481cab89 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 10 Feb 2023 12:02:42 -0800 Subject: [PATCH 04/13] Fix forked PR check PiperOrigin-RevId: 508720169 --- .github/workflows/forked_pr_workflow_check.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/forked_pr_workflow_check.yml b/.github/workflows/forked_pr_workflow_check.yml index 28be6bbb73..f1dd9b5516 100644 --- a/.github/workflows/forked_pr_workflow_check.yml +++ b/.github/workflows/forked_pr_workflow_check.yml @@ -23,5 +23,5 @@ jobs: runs-on: ubuntu-latest steps: - run: > - ${{ github.event.pull_request.head.repo.full_name != 'protocolbuffers/protobuf' }} || - (echo "This pull request is from an unsafe fork and isn't allowed to modify workflow files!" && exit 1) + ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/protobuf' }} || + (echo "This pull request is from an unsafe fork (${{ github.event.pull_request.head.repo.full_name }}) and isn't allowed to modify workflow files!" && exit 1) From 746594a746e5e16e524f15706b627ef92c640007 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Fri, 10 Feb 2023 14:33:29 -0800 Subject: [PATCH 05/13] Have the CocoaPods CI update the WKTs before testing. PiperOrigin-RevId: 508756680 --- .github/workflows/test_objectivec.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test_objectivec.yml b/.github/workflows/test_objectivec.yml index f5720cd245..f572d6d19d 100644 --- a/.github/workflows/test_objectivec.yml +++ b/.github/workflows/test_objectivec.yml @@ -73,11 +73,14 @@ jobs: with: ref: ${{ inputs.safe-checkout }} - name: Pod lib lint - run: | - pod lib lint --verbose \ - --configuration=${{ matrix.CONFIGURATION }} \ - --platforms=${{ matrix.PLATFORM }} \ - Protobuf.podspec + uses: ./.github/actions/bash + with: + credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} + command: | + pod lib lint --verbose \ + --configuration=${{ matrix.CONFIGURATION }} \ + --platforms=${{ matrix.PLATFORM }} \ + Protobuf.podspec bazel: strategy: From c120cdc527119e99bae0919bc95e1ae376b299ba Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Fri, 10 Feb 2023 14:59:49 -0800 Subject: [PATCH 06/13] [ObjC] Move the Xcode CI off full_mac_build.sh Use the bash GHA to ensure WKTs are current, and then just directly invoke Xcode. This means the script full_mac_build.sh can go back to just being a simpler helper for folks doing local builds and we can remove all the extra support that was needed for CI. PiperOrigin-RevId: 508762464 --- .github/BUILD.bazel | 6 ++++- .github/workflows/test_objectivec.yml | 37 +++++++++++++++------------ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/.github/BUILD.bazel b/.github/BUILD.bazel index 98a95d1bb8..12daec4a37 100644 --- a/.github/BUILD.bazel +++ b/.github/BUILD.bazel @@ -1,5 +1,9 @@ # This information is extracted from the MacOS runner specs located at: -# https://github.com/actions/runner-images/blob/win19/20230129.2/images/macos/macos-12-Readme.md +# https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md +# +# When updating, also ensure the "xcode_destination" entries in +# `.github/workflows/test_objectivec.yml` are supported for the given versions +# of Xcode. xcode_version( name = "version14_2_14C18", version = "14.2.14C18", diff --git a/.github/workflows/test_objectivec.yml b/.github/workflows/test_objectivec.yml index f572d6d19d..465e022592 100644 --- a/.github/workflows/test_objectivec.yml +++ b/.github/workflows/test_objectivec.yml @@ -13,20 +13,19 @@ jobs: strategy: fail-fast: false # Don't cancel all jobs if one fails. matrix: + platform: ["macOS", "iOS"] + xc_config: ["Debug", "Release"] + # The "destination" entries need to match what is available for the selected Xcode. + # See `.github/BUILD.bazel` for the Xcode info. include: - - name: macOS - config: osx - flags: --skip-xcode-ios --skip-xcode-tvos --skip-objc-conformance - # The iOS simulator takes a while to start up, so Debug & Release are run in - # parallel to get the testing done faster. - - name: iOS Debug - config: ios_debug - flags: --skip-xcode-osx --skip-xcode-tvos --skip-objc-conformance --skip-xcode-release - - name: iOS Release - config: ios_release - flags: --skip-xcode-osx --skip-xcode-tvos --skip-objc-conformance --skip-xcode-debug + - platform: "macOS" + destination: "platform=macOS" + xc_project: "ProtocolBuffers_OSX.xcodeproj" + - platform: "iOS" + destination: "platform=iOS Simulator,name=iPhone 13,OS=latest" + xc_project: "ProtocolBuffers_iOS.xcodeproj" - name: Xcode ${{ matrix.name}} + name: Xcode ${{ matrix.platform}} ${{ matrix.xc_config }} runs-on: macos-12 env: DEVELOPER_DIR: /Applications/Xcode_14.1.app/Contents/Developer @@ -39,18 +38,24 @@ jobs: - name: Setup ccache uses: ./.github/actions/ccache with: - cache-prefix: objectivec_macos_${{ matrix.config }} + cache-prefix: objectivec_${{ matrix.platform }}_${{ matrix.xc_config }} support-modules: true - name: Run tests - uses: ./.github/actions/bazel + uses: ./.github/actions/bash env: CC: ${{ github.workspace }}/ci/clang_wrapper CXX: ${{ github.workspace }}/ci/clang_wrapper++ with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - bazel-cache: objectivec_macos/${{ matrix.config }} - bash: objectivec/DevTools/full_mac_build.sh ${{ matrix.flags }} + command: | + xcodebuild \ + -project "objectivec/${{ matrix.xc_project }}" \ + -scheme ProtocolBuffers \ + -configuration ${{ matrix.xc_config }} \ + -destination "${{ matrix.destination }}" \ + test \ + | xcpretty - name: Report ccache stats shell: bash From 5127af50f2f2406a2a70ff845b9483a6919502b1 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 10 Feb 2023 18:24:16 -0800 Subject: [PATCH 07/13] Switch to Ninja generator for windows cmake builds. These will still use MSVC as the compiler, but will no longer generate Visual Studio projects for the builds. Visual Studio is particularly bad at parallelizing builds, and is hostile to ccache. This change also tweaks the ccache setup to prevent unbounded the growth observed in our github caches. Windows builds have had debug symbols stripped to reduce ccache size by a factor of 2x, and ccache maximum have been tweaked so that we persist fewer older builds. Before this change, each CMake build took 12 minutes on every run (plus some constant overhead from staleness/gcloud), even with caching or on large multi-core runners. No amount of caching or parallelization made any noticeable difference above noise. With this change, we see the following improvements: - 12 minutes to build from scratch on normal runners (unchanged) - 4 minutes on 32-core runners from scratch - 1 minute with optimal caches available on normal runners. - 30 seconds on 32-core runners with optimal caches PiperOrigin-RevId: 508799909 --- .github/actions/ccache/action.yml | 36 ++-- .../internal/ccache-setup-windows/action.yml | 28 +-- .github/workflows/test_cpp.yml | 166 ++++++------------ CMakeLists.txt | 16 +- 4 files changed, 90 insertions(+), 156 deletions(-) diff --git a/.github/actions/ccache/action.yml b/.github/actions/ccache/action.yml index 2a100ccb7f..2d70550364 100644 --- a/.github/actions/ccache/action.yml +++ b/.github/actions/ccache/action.yml @@ -12,6 +12,18 @@ inputs: runs: using: 'composite' steps: + - name: Configure ccache environment variables + shell: bash + run: | + echo "CCACHE_BASEDIR=${{ github.workspace }}" >> $GITHUB_ENV + echo "CCACHE_DIR=${{ github.workspace }}/.ccache" >> $GITHUB_ENV + echo "CCACHE_COMPRESS=true" >> $GITHUB_ENV + echo "CCACHE_COMPRESSLEVEL=5" >> $GITHUB_ENV + echo "CCACHE_MAXSIZE=100M" >> $GITHUB_ENV + echo "CCACHE_SLOPPINESS=clang_index_store,include_file_ctime,include_file_mtime,file_macro,time_macros" >> $GITHUB_ENV + echo "CCACHE_DIRECT=true" >> $GITHUB_ENV + echo "CCACHE_CMAKE_FLAGS=-Dprotobuf_ALLOW_CCACHE=ON -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache" >> $GITHUB_ENV + - name: Setup ccache on Windows if: ${{ runner.os == 'Windows' }} uses: ./.github/actions/internal/ccache-setup-windows @@ -23,7 +35,10 @@ runs: - name: Setup fixed path ccache caching uses: actions/cache@627f0f41f6904a5b1efbaed9f96d9eb58e92e920 # v3.2.4 with: - path: .ccache + path: | + .ccache/** + !.ccache/lock + !.ccache/tmp # Always push to a cache key unique to this commit. key: ${{ format('ccache-{0}-{1}-{2}', inputs.cache-prefix, github.ref_name, github.sha) }} # Select a cache to restore from with the follow order of preference: @@ -35,18 +50,6 @@ runs: ${{ format('ccache-{0}-{1}', inputs.cache-prefix, github.ref_name) }} ${{ format('ccache-{0}-{1}', inputs.cache-prefix, github.base_ref) }} - - name: Configure ccache environment variables - shell: bash - run: | - echo "CCACHE_BASEDIR=${{ github.workspace }}" >> $GITHUB_ENV - echo "CCACHE_DIR=${{ github.workspace }}/.ccache" >> $GITHUB_ENV - echo "CCACHE_COMPRESS=true" >> $GITHUB_ENV - echo "CCACHE_COMPRESSLEVEL=6" >> $GITHUB_ENV - echo "CCACHE_MAXSIZE=600M" >> $GITHUB_ENV - echo "CCACHE_SLOPPINESS=clang_index_store,include_file_ctime,include_file_mtime,file_macro,time_macros" >> $GITHUB_ENV - echo "CCACHE_DIRECT=true" >> $GITHUB_ENV - echo "CCACHE_CMAKE_FLAGS=-Dprotobuf_ALLOW_CCACHE=ON -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache $CCACHE_CMAKE_FLAGS" >> $GITHUB_ENV - - name: Enable module support if: ${{ inputs.support-modules }} shell: bash @@ -55,11 +58,6 @@ runs: echo "CCACHE_DEPEND=true" >> $GITHUB_ENV - name: Zero out ccache - if: ${{ runner.os == 'macOS' }} + if: ${{ runner.os != 'Linux' }} shell: bash run: ccache -z - - - name: Zero out ccache - if: ${{ runner.os == 'Windows' }} - shell: pwsh - run: ${{ github.workspace }}\ccache.exe -z diff --git a/.github/actions/internal/ccache-setup-windows/action.yml b/.github/actions/internal/ccache-setup-windows/action.yml index 5479cd3280..a42e7bf8a8 100644 --- a/.github/actions/internal/ccache-setup-windows/action.yml +++ b/.github/actions/internal/ccache-setup-windows/action.yml @@ -10,6 +10,16 @@ inputs: runs: using: 'composite' steps: + - name: Setup MSVC + uses: ilammy/msvc-dev-cmd@cec98b9d092141f74527d0afa6feb2af698cfe89 # v1.12.1 + with: + arch: x64 + vsversion: '2019' + + - name: Install ccache + shell: bash + run: choco install ccache --version=4.7.4 + - name: Configure ccache environment variables shell: pwsh run: | @@ -18,19 +28,9 @@ runs: echo "CCACHE_COMPILER=$cllocation" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append echo "CCACHE_COMPILERTYPE=msvc" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append - - name: Download ccache + - name: Configure Windows-specific ccache environment variables shell: bash + # Windows caches are about 2x larger than other platforms. run: | - curl -kLSs "https://github.com/ccache/ccache/releases/download/v${{ inputs.ccache-version }}/ccache-${{ inputs.ccache-version }}-windows-x86_64.zip" -o ccache.zip - unzip ccache.zip - cp ccache-${{ inputs.ccache-version }}-windows-x86_64/ccache.exe ccache.exe - cp ccache.exe cl.exe - rm ccache.zip - - - name: Configure msbuild flags - shell: bash - run: echo "CCACHE_MSBUILD_FLAGS=/p:CLToolExe=cl.exe /p:CLToolPath=${{ github.workspace}}" >> $GITHUB_ENV - - - name: Configure cmake flags - shell: bash - run: echo "CCACHE_CMAKE_FLAGS=-Dprotobuf_ALLOW_CCACHE=ON" >> $GITHUB_ENV + echo "CCACHE_COMPRESSLEVEL=10" >> $GITHUB_ENV + echo "CCACHE_MAXSIZE=200M" >> $GITHUB_ENV diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml index 5d51bcae3c..f846d80a8d 100644 --- a/.github/workflows/test_cpp.yml +++ b/.github/workflows/test_cpp.yml @@ -179,49 +179,34 @@ jobs: bazel: test ${{ matrix.bazel }} bazel-cache: cpp_${{ matrix.os }} - macos-cmake: - name: MacOS CMake - runs-on: macos-12 - steps: - - name: Checkout pending changes - uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 - with: - submodules: recursive - ref: ${{ inputs.safe-checkout }} - - - name: Setup ccache - uses: ./.github/actions/ccache - with: - cache-prefix: macos-cmake - - - name: Configure CMake - uses: ./.github/actions/bash - with: - credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - command: cmake . -DCMAKE_CXX_STANDARD=14 ${{ env.CCACHE_CMAKE_FLAGS }} - - name: Build - run: cmake --build . --parallel 8 - - name: Test - run: ctest --verbose --parallel 20 -C Debug - - - name: Report ccache stats - shell: bash - run: ccache -s -v - - windows-cmake: + non-linux-cmake: strategy: fail-fast: false # Don't cancel all jobs if one fails. matrix: include: - - name: Visual Studio + - name: MacOS CMake + os: macos-12 + flags: -DCMAKE_CXX_STANDARD=14 + - name: Windows CMake + os: windows-2019 flags: >- + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF -Dprotobuf_BUILD_SHARED_LIBS=OFF -Dprotobuf_BUILD_EXAMPLES=ON - - name: Shared - flags: -Dprotobuf_BUILD_SHARED_LIBS=ON - - name: Windows CMake ${{ matrix.name}} - runs-on: windows-2019 + - name: Windows CMake Shared + os: windows-2019 + flags: >- + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -Dprotobuf_BUILD_SHARED_LIBS=ON + - name: Windows CMake Install + os: windows-2019 + install-flags: -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF -Dprotobuf_BUILD_TESTS=OFF + flags: >- + -G Ninja -Dprotobuf_WITH_ZLIB=OFF -Dprotobuf_BUILD_CONFORMANCE=OFF + -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON + -Dprotobuf_BUILD_PROTOBUF_BINARIES=OFF + name: ${{ matrix.name }} + runs-on: ${{ matrix.os }} steps: - name: Checkout pending changes uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 @@ -229,102 +214,49 @@ jobs: submodules: recursive ref: ${{ inputs.safe-checkout }} - - name: Setup MSVC - uses: ilammy/msvc-dev-cmd@cec98b9d092141f74527d0afa6feb2af698cfe89 # v1.12.1 - with: - arch: x64 - vsversion: '2019' - - name: Setup ccache uses: ./.github/actions/ccache with: - cache-prefix: windows-cmake-${{ matrix.name }} + cache-prefix: ${{ matrix.name }} - - name: Configure CMake + # Install phase. + - name: Configure CMake for install + if: matrix.install-flags uses: ./.github/actions/bash with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - command: | - cmake . -G "Visual Studio 16 2019" -A x64 \ - ${{ env.CCACHE_CMAKE_FLAGS }} \ - -Dprotobuf_BUILD_CONFORMANCE=OFF \ - -Dprotobuf_WITH_ZLIB=OFF \ - ${{ matrix.flags }} - - - name: Build for Windows 15 2017 - run: >- - msbuild.exe protobuf.sln /p:MultiProcessorCompilation=true /p:CL_MPCount=8 /maxcpucount:8 /p:BuildInParallel=true - /p:Configuration=Debug /p:Platform=x64 /p:VisualStudioVersion=15.0 - ${{ env.CCACHE_MSBUILD_FLAGS }} - - - name: Run Tests - run: ctest --verbose --parallel 20 -C Debug - - - name: Report ccache stats - run: ${{ github.workspace }}\ccache.exe -s -v - - windows-cmake-install: - name: Windows CMake Install - runs-on: windows-2019 - steps: - - name: Checkout pending changes - uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0 - with: - submodules: recursive - ref: ${{ inputs.safe-checkout }} - - - name: Setup MSVC - uses: ilammy/msvc-dev-cmd@cec98b9d092141f74527d0afa6feb2af698cfe89 # v1.12.1 - with: - arch: x64 - vsversion: '2019' - - - name: Setup ccache - uses: ./.github/actions/ccache - with: - cache-prefix: windows-cmake + command: cmake . ${{ matrix.install-flags }} ${{ env.CCACHE_CMAKE_FLAGS }} + - name: Build for install + if: matrix.install-flags + shell: bash + run: VERBOSE=1 cmake --build . --parallel 20 + - name: Install + if: matrix.install-flags + shell: bash + run: cmake --build . --target install + - name: Report and clear ccache stats + if: matrix.install-flags + shell: bash + run: ccache -s -v && ccache -z + - name: Clear CMake cache + if: matrix.install-flags + shell: bash + run: cmake --build . --target clean && rm CMakeCache.txt - - name: Configure CMake for Install + - name: Configure CMake uses: ./.github/actions/bash with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - command: | - mkdir build - pushd build - cmake .. -G "Visual Studio 16 2019" -A x64 \ - ${{ env.CCACHE_CMAKE_FLAGS }} \ - -Dprotobuf_BUILD_CONFORMANCE=OFF \ - -Dprotobuf_WITH_ZLIB=OFF - popd - - - name: Build and Install Protobuf for Windows 15 2017 - run: | - pushd build - msbuild.exe INSTALL.vcxproj /p:Platform=x64 /p:VisualStudioVersion=15.0 /p:MultiProcessorCompilation=true /p:CL_MPCount=8 /maxcpucount:8 /p:BuildInParallel=true ${{ env.CCACHE_MSBUILD_FLAGS }} - popd + command: cmake . ${{ matrix.flags }} ${{ env.CCACHE_CMAKE_FLAGS }} - - name: Clear CMake cache + - name: Build shell: bash - run: rm -rf build/* + run: VERBOSE=1 cmake --build . --parallel 20 - - name: Configure CMake + - name: Test shell: bash - run: | - cmake . -G "Visual Studio 16 2019" -A x64 \ - ${{ env.CCACHE_CMAKE_FLAGS }} \ - -Dprotobuf_REMOVE_INSTALLED_HEADERS=ON \ - -Dprotobuf_BUILD_PROTOBUF_BINARIES=OFF \ - -Dprotobuf_BUILD_CONFORMANCE=OFF \ - -Dprotobuf_WITH_ZLIB=OFF - - - name: Build for Windows 15 2017 - run: >- - msbuild.exe protobuf.sln /p:MultiProcessorCompilation=true /p:CL_MPCount=8 /maxcpucount:8 /p:BuildInParallel=true - /p:Configuration=Debug /p:Platform=x64 /p:VisualStudioVersion=15.0 - ${{ env.CCACHE_MSBUILD_FLAGS }} - - - name: Run Tests run: ctest --verbose --parallel 20 -C Debug - name: Report ccache stats - run: ${{ github.workspace }}\ccache.exe -s -v + shell: bash + run: ccache -s -v diff --git a/CMakeLists.txt b/CMakeLists.txt index e031785f0b..2ac9acbe47 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -292,13 +292,17 @@ if (MSVC) string(REPLACE "." "," protobuf_RC_FILEVERSION "${protobuf_VERSION}") if (protobuf_ALLOW_CCACHE) - # In order to support ccache, we replace the /Zi option with /Z7. This + # In order to support ccache, we need to remove the /Zi option because it + # puts debug symbols into separate pdb files (which in incompatible with + # ccache). This can be replaced with /Z7 to preserve debug symbols, which # embeds debug symbols into the object files instead of creating a separate - # pdb file, which isn't currently supported by ccache. - string(REPLACE "/Zi" "/Z7" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") - string(REPLACE "/Zi" "/Z7" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") - string(REPLACE "/Zi" "/Z7" CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO}") - string(REPLACE "/Zi" "/Z7" CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}") + # pdb file, which isn't currently supported by ccache. However, this bloats + # the ccache size by about a factor of 2x, making it very expensive in CI. + # Instead, we strip debug symbols to reduce this overhead. + string(REPLACE "/Zi" "/DEBUG:NONE" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") + string(REPLACE "/Zi" "/DEBUG:NONE" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") + string(REPLACE "/Zi" "/DEBUG:NONE" CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO}") + string(REPLACE "/Zi" "/DEBUG:NONE" CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}") endif() # Suppress linker warnings about files with no symbols defined. From d8cd3bd9c93a15ac36af9dc5e75468020eb05fed Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Sat, 11 Feb 2023 17:21:20 -0800 Subject: [PATCH 08/13] Tweak GHA names to make failures clearer in dashboard PiperOrigin-RevId: 508944353 --- .github/workflows/staleness_check.yml | 2 +- .github/workflows/staleness_refresh.yml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/staleness_check.yml b/.github/workflows/staleness_check.yml index 7ae33d22de..022d64eb39 100644 --- a/.github/workflows/staleness_check.yml +++ b/.github/workflows/staleness_check.yml @@ -14,7 +14,7 @@ jobs: branch: [main, 22.x] os: [{ name: Linux, value: ubuntu-latest}] - name: ${{ matrix.os.name }} ${{ matrix.branch}} + name: Test staleness ${{ matrix.os.name }} ${{ matrix.branch}} runs-on: ${{ matrix.os.value }} steps: - name: Checkout ${{ matrix.branch }} diff --git a/.github/workflows/staleness_refresh.yml b/.github/workflows/staleness_refresh.yml index 5264b14b80..8f083fdc6b 100644 --- a/.github/workflows/staleness_refresh.yml +++ b/.github/workflows/staleness_refresh.yml @@ -11,12 +11,13 @@ on: permissions: {} jobs: - cmake: + run: permissions: contents: write # for git push if: github.repository == 'protocolbuffers/protobuf' runs-on: ubuntu-latest + name: Refresh stale files strategy: fail-fast: false # Don't cancel all jobs if one fails. From 0da35b1e638760634cf6e39f83d30c5bd4c3d828 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Sun, 12 Feb 2023 19:35:55 -0800 Subject: [PATCH 09/13] Move GHA documentation into workflows directory. Placing it in the .github directory unexpectedly replaced our top-level documentation on the main page. PiperOrigin-RevId: 509104326 --- .github/{ => workflows}/README.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/{ => workflows}/README.md (100%) diff --git a/.github/README.md b/.github/workflows/README.md similarity index 100% rename from .github/README.md rename to .github/workflows/README.md From a3970ef1f261208a98545aebf9d5369d863cecf2 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Sun, 12 Feb 2023 21:49:12 -0800 Subject: [PATCH 10/13] Stop using 'push' event to determine privileged post-submit runs. Both 'schedule' and 'workflow_dispatch' are valid alternatives today. What we really want here is anything *except* pull request events. PiperOrigin-RevId: 509123777 --- .github/actions/bazel-docker/action.yml | 2 +- .github/actions/bazel/action.yml | 6 +++--- .github/workflows/test_runner.yml | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/actions/bazel-docker/action.yml b/.github/actions/bazel-docker/action.yml index 800d8eca24..af1104e23d 100644 --- a/.github/actions/bazel-docker/action.yml +++ b/.github/actions/bazel-docker/action.yml @@ -71,5 +71,5 @@ runs: - name: Save Bazel repository cache # Only allow repository cache updates during post-submits. - if: ${{ github.event_name == 'push' }} + if: ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' }} uses: ./.github/actions/internal/repository-cache-save diff --git a/.github/actions/bazel/action.yml b/.github/actions/bazel/action.yml index a848cfc394..141527db02 100644 --- a/.github/actions/bazel/action.yml +++ b/.github/actions/bazel/action.yml @@ -63,14 +63,14 @@ runs: run: echo "BAZELISK_PATH=$LOCALAPPDATA\bazelisk" >> $GITHUB_ENV - name: Cache Bazelisk - if: ${{ github.event_name == 'push' }} + if: ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target' }} uses: actions/cache@627f0f41f6904a5b1efbaed9f96d9eb58e92e920 # v3.2.4 with: path: ${{ env.BAZELISK_PATH }} key: bazel-${{ runner.os }}-${{ inputs.version }} - name: Restore Bazelisk - if: ${{ github.event_name != 'push' }} + if: ${{ github.event_name == 'pull_request' || github.event_name == 'pull_request_target' }} uses: actions/cache/restore@627f0f41f6904a5b1efbaed9f96d9eb58e92e920 # v3.2.4 with: path: ${{ env.BAZELISK_PATH }} @@ -107,5 +107,5 @@ runs: - name: Save Bazel repository cache # Only allow repository cache updates during post-submits. - if: ${{ github.event_name == 'push' }} + if: ${{ github.event_name != 'pull_request' && github.event_name != 'pull_request_target'}} uses: ./.github/actions/internal/repository-cache-save diff --git a/.github/workflows/test_runner.yml b/.github/workflows/test_runner.yml index 6bf33e6a1f..f414605aa1 100644 --- a/.github/workflows/test_runner.yml +++ b/.github/workflows/test_runner.yml @@ -57,7 +57,8 @@ jobs: # repository, it's safe and we can use `pull_request`. Otherwise, we should # use `pull_request_target`. if: | - (github.event_name == 'push' && + (github.event_name != 'pull_request' && + github.event_name != 'pull_request_target' && github.event.repository.full_name == 'protocolbuffers/protobuf') || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == 'protocolbuffers/protobuf') || From de8bb714e252a6bc91e2b056260f9ccc495d4b4a Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 13 Feb 2023 10:07:13 -0800 Subject: [PATCH 11/13] [ObjC] CI dbg & opt macOS bazel builds. PiperOrigin-RevId: 509260795 --- .github/workflows/test_objectivec.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test_objectivec.yml b/.github/workflows/test_objectivec.yml index 465e022592..4093b180b7 100644 --- a/.github/workflows/test_objectivec.yml +++ b/.github/workflows/test_objectivec.yml @@ -91,11 +91,15 @@ jobs: strategy: fail-fast: false # Don't cancel all jobs if one fails. matrix: + config: + - { name: Optimized, flags: --config=opt } + - { name: Debug, flags: --config=dbg } + # TODO: Could add iOS to atleast build the objc_library targets for that. + platform: ["macOS"] include: - # TODO: Could add iOS to atleast build the objc_library targets for that. - - name: macOS - bazel: //objectivec/... - name: Bazel ${{ matrix.name }} + - platform: "macOS" + bazel_targets: //objectivec/... + name: Bazel ${{ matrix.platform }} ${{ matrix.config.name }} runs-on: macos-12 steps: - name: Checkout pending changes @@ -106,5 +110,5 @@ jobs: uses: ./.github/actions/bazel with: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} - bazel: test ${{ matrix.bazel }} - bazel-cache: objc_${{ matrix.name }} + bazel: test ${{ matrix.config.flags }} ${{ matrix.bazel_targets }} + bazel-cache: objc_${{ matrix.platform }}_${{ matrix.config.name }} From 9a5e86d4527c31bb957b5df82ef8e4a0104a3ef4 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Mon, 13 Feb 2023 20:22:21 -0800 Subject: [PATCH 12/13] Run continuous tests hourly to boost statistics for analysis PiperOrigin-RevId: 509409873 --- .github/workflows/test_runner.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_runner.yml b/.github/workflows/test_runner.yml index f414605aa1..a99d090cc3 100644 --- a/.github/workflows/test_runner.yml +++ b/.github/workflows/test_runner.yml @@ -12,8 +12,9 @@ name: Tests on: # continuous schedule: - # Run daily at 10 AM UTC (2 AM PDT) - - cron: 0 10 * * * + # TODO(mkruskal) Run daily at 10 AM UTC (2 AM PDT) + # Run every hour for now to gather statistics + - cron: 0 * * * * # postsubmit push: From d39aeac2c1113b4207774d560414cc780e64cc16 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 14 Feb 2023 09:05:33 -0800 Subject: [PATCH 13/13] Fixing broken rust package PiperOrigin-RevId: 509546035 --- rust/BUILD.bazel | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/BUILD.bazel b/rust/BUILD.bazel index c8dbca76ec..5efdab5493 100644 --- a/rust/BUILD.bazel +++ b/rust/BUILD.bazel @@ -1,8 +1,8 @@ # Protobuf Rust runtime packages. -load("@rules_rust//:defs.bzl", "rust_library") +load("@rules_rust//rust:defs.bzl", "rust_library") -package(default_visibility = "//src/google/protobuf:__subpackages__") +package(default_visibility = ["//src/google/protobuf:__subpackages__"]) rust_library( name = "protobuf",