Copilot commented on code in PR #811: URL: https://github.com/apache/iceberg-cpp/pull/811#discussion_r3627480321
########## .github/actions/setup-sccache/action.yml: ########## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Set up sccache +description: Configure sccache, restore its cache, and start the sccache action. + +inputs: + key-prefix: + description: Cache key prefix without the run id or Windows compiler suffix. + required: true + cache-size: + description: Maximum sccache cache size. + required: false + default: 2G + +runs: + using: composite + steps: + - name: Configure sccache environment + shell: bash + env: + CACHE_SIZE: ${{ inputs.cache-size }} + run: | + echo "SCCACHE_DIR=${{ github.workspace }}/.sccache" >> "$GITHUB_ENV" + echo "SCCACHE_CACHE_SIZE=${CACHE_SIZE}" >> "$GITHUB_ENV" + echo "SCCACHE_KEY_SUFFIX=" >> "$GITHUB_ENV" Review Comment: In bash, `echo` has implementation-defined handling of backslash escapes and can behave unexpectedly with Windows-style paths (e.g., `D:\a\...`). Using `printf` avoids any risk of writing a corrupted value into `$GITHUB_ENV` on Windows runners. ########## .github/workflows/test.yml: ########## @@ -59,30 +56,20 @@ jobs: - name: Install dependencies shell: bash run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev - - name: Restore sccache cache - uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Set up sccache + uses: ./.github/actions/setup-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-test-ubuntu-${{ matrix.cmake_build_type }}-${{ github.run_id }} - restore-keys: | - sccache-test-ubuntu-${{ matrix.cmake_build_type }}- - - name: Setup sccache - uses: mozilla-actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 + key-prefix: sccache-test-ubuntu-${{ matrix.cmake_build_type }} - name: Build Iceberg shell: bash env: CC: gcc-14 CXX: g++-14 run: ci/scripts/build_iceberg.sh $(pwd) ON ON OFF OFF ON ${{ matrix.cmake_build_type }} - - name: Show sccache stats - shell: bash - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-test-ubuntu-${{ matrix.cmake_build_type }}-${{ github.run_id }} + key-prefix: sccache-test-ubuntu-${{ matrix.cmake_build_type }} Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/actions/save-sccache/action.yml: ########## @@ -0,0 +1,39 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Save sccache +description: Show sccache stats and save the cache on main branch runs after setup-sccache. + +inputs: + key-prefix: + description: Cache key prefix without the run id or Windows compiler suffix. + required: true + +runs: + using: composite + steps: + - name: Show sccache stats + if: always() + shell: bash + run: sccache --show-stats + + - name: Save sccache cache + if: ${{ github.ref == 'refs/heads/main' }} + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 Review Comment: If workflow call sites change to `if: always()` to ensure stats run on failed builds, this step could also save a partial/invalid cache on main. Consider gating the save on `success()` as well, so callers can safely run the action unconditionally. ########## .github/workflows/test.yml: ########## @@ -96,35 +83,22 @@ jobs: timeout-minutes: 30 strategy: fail-fast: false - env: - SCCACHE_DIR: ${{ github.workspace }}/.sccache - SCCACHE_CACHE_SIZE: "2G" steps: - name: Checkout iceberg-cpp uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false - - name: Restore sccache cache - uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Set up sccache + uses: ./.github/actions/setup-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-test-macos-${{ github.run_id }} - restore-keys: | - sccache-test-macos- - - name: Setup sccache - uses: mozilla-actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 + key-prefix: sccache-test-macos - name: Build Iceberg shell: bash run: ci/scripts/build_iceberg.sh $(pwd) OFF ON - - name: Show sccache stats - shell: bash - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-test-macos-${{ github.run_id }} + key-prefix: sccache-test-macos Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/workflows/test.yml: ########## @@ -246,27 +188,18 @@ jobs: run: | echo "CC=sccache ${{ matrix.CC }}" >> $GITHUB_ENV echo "CXX=sccache ${{ matrix.CXX }}" >> $GITHUB_ENV - - name: Restore sccache cache - uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Set up sccache + uses: ./.github/actions/setup-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-meson-${{ matrix.runs-on }}-${{ github.run_id }} - restore-keys: | - sccache-meson-${{ matrix.runs-on }}- - - name: Setup sccache - uses: mozilla-actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 + key-prefix: sccache-meson-${{ matrix.runs-on }} - name: Build Iceberg run: | meson setup builddir ${{ matrix.meson-setup-args || '' }} meson compile -C builddir - - name: Show sccache stats - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-meson-${{ matrix.runs-on }}-${{ github.run_id }} + key-prefix: sccache-meson-${{ matrix.runs-on }} Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/workflows/sql_catalog_test.yml: ########## @@ -140,15 +112,10 @@ jobs: - name: Build SQL catalog tests shell: bash run: cmake --build build --target sql_catalog_test - - name: Show sccache stats - shell: bash - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-sqlcatalog-${{ matrix.runs-on }}${{ env.SCCACHE_KEY_SUFFIX }}-${{ github.run_id }} + key-prefix: sccache-sqlcatalog-${{ matrix.runs-on }} Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if an earlier step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/workflows/sanitizer_test.yml: ########## @@ -67,15 +59,10 @@ jobs: cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Debug -DICEBERG_ENABLE_ASAN=ON -DICEBERG_ENABLE_UBSAN=ON \ -DCMAKE_C_COMPILER_LAUNCHER=sccache -DCMAKE_CXX_COMPILER_LAUNCHER=sccache cmake --build . --verbose - - name: Show sccache stats - shell: bash - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-sanitizer-ubuntu-${{ github.run_id }} + key-prefix: sccache-sanitizer-ubuntu Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/workflows/aws_test.yml: ########## @@ -114,26 +112,16 @@ jobs: if: ${{ matrix.s3 == 'ON' }} shell: bash run: bash ci/scripts/start_minio.sh - - name: Restore sccache cache - uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Set up sccache + uses: ./.github/actions/setup-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-aws-${{ matrix.runs-on }}-bundle${{ matrix.bundle_awssdk }}-s3${{ matrix.s3 }}-sigv4${{ matrix.sigv4 }}-${{ github.run_id }} - restore-keys: | - sccache-aws-${{ matrix.runs-on }}-bundle${{ matrix.bundle_awssdk }}-s3${{ matrix.s3 }}-sigv4${{ matrix.sigv4 }}- - - name: Setup sccache - uses: mozilla-actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 + key-prefix: sccache-aws-${{ matrix.runs-on }}-bundle${{ matrix.bundle_awssdk }}-s3${{ matrix.s3 }}-sigv4${{ matrix.sigv4 }} - name: Build and test Iceberg shell: bash env: CMAKE_TOOLCHAIN_FILE: ${{ startsWith(matrix.runs-on, 'ubuntu') && matrix.bundle_awssdk == 'OFF' && '/usr/local/share/vcpkg/scripts/buildsystems/vcpkg.cmake' || '' }} run: ci/scripts/build_iceberg.sh "$(pwd)" OFF ON ${{ matrix.s3 }} ${{ matrix.sigv4 }} ${{ matrix.bundle_awssdk }} - - name: Show sccache stats - shell: bash - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-aws-${{ matrix.runs-on }}-bundle${{ matrix.bundle_awssdk }}-s3${{ matrix.s3 }}-sigv4${{ matrix.sigv4 }}-${{ github.run_id }} + key-prefix: sccache-aws-${{ matrix.runs-on }}-bundle${{ matrix.bundle_awssdk }}-s3${{ matrix.s3 }}-sigv4${{ matrix.sigv4 }} Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build/test step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/workflows/test.yml: ########## @@ -176,28 +129,20 @@ jobs: shell: pwsh run: | vcpkg install zlib:x64-windows nlohmann-json:x64-windows nanoarrow:x64-windows roaring:x64-windows cpr:x64-windows - - name: Restore sccache cache - uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Set up sccache + uses: ./.github/actions/setup-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-test-windows-${{ env.MSVC_VER }}-${{ github.run_id }} - restore-keys: | - sccache-test-windows-${{ env.MSVC_VER }}- - - name: Setup sccache - uses: mozilla-actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 + key-prefix: sccache-test-windows - name: Build Iceberg shell: pwsh run: | $ErrorActionPreference = "Stop" bash -lc 'ci/scripts/build_iceberg.sh $(pwd) OFF ON' if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-test-windows-${{ env.MSVC_VER }}-${{ github.run_id }} + key-prefix: sccache-test-windows Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). ########## .github/workflows/cpp-linter.yml: ########## @@ -82,15 +74,10 @@ jobs: -DICEBERG_SQL_POSTGRESQL=ON \ -DICEBERG_SQL_MYSQL=ON cmake --build . - - name: Show sccache stats - shell: bash - run: sccache --show-stats - - name: Save sccache cache - if: github.ref == 'refs/heads/main' - uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + - name: Save sccache + uses: ./.github/actions/save-sccache with: - path: ${{ github.workspace }}/.sccache - key: sccache-cpp-linter-ubuntu-${{ github.run_id }} + key-prefix: sccache-cpp-linter-ubuntu Review Comment: The PR description says sccache stats should be shown for every run, but this step won’t execute if the preceding build step fails (default `if: success()`). Add `if: always()` here so the composite action’s `Show sccache stats` step can still run on failures (and rely on the composite action to gate saving). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
