From d1855d91176306a447d319df0202bae95ad0cbec Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 4 Jun 2026 21:13:01 +0000 Subject: [PATCH] codex: address PR review feedback (#26461) --- .github/actions/finish-cargo-ci/action.yml | 109 ++++++++++++++++++ .../rust-ci-full-nextest-platform.yml | 46 ++------ .github/workflows/rust-ci-full.yml | 65 +++-------- 3 files changed, 135 insertions(+), 85 deletions(-) create mode 100644 .github/actions/finish-cargo-ci/action.yml diff --git a/.github/actions/finish-cargo-ci/action.yml b/.github/actions/finish-cargo-ci/action.yml new file mode 100644 index 0000000000..4441448d5e --- /dev/null +++ b/.github/actions/finish-cargo-ci/action.yml @@ -0,0 +1,109 @@ +name: finish-cargo-ci +description: Save shared Cargo CI caches and report sccache statistics. +inputs: + target: + description: Rust target triple used for cache namespacing. + required: true + profile: + description: Cargo profile used for cache namespacing. + required: true + cache-runner: + description: Runner name used for cache namespacing. + required: true + cargo-home-cache-hit: + description: Whether the exact Cargo home cache key was restored. + required: true + lockfile-hash: + description: SHA-256 hash of codex-rs/Cargo.lock. + required: true + toolchain-hash: + description: SHA-256 hash of codex-rs/rust-toolchain.toml. + required: true + summary-label: + description: Label shown beside the target in the sccache summary. + required: true + use-hermetic-cargo-home: + description: Save the workspace-local Cargo home used by musl builds. + required: false + default: "false" + use-sccache: + description: Save and report sccache state. + required: false + default: "true" + supplemental-cache-path: + description: Optional workload-specific cache path to save. + required: false + default: "" + supplemental-cache-key: + description: Cache key for the optional workload-specific cache. + required: false + default: "" + supplemental-cache-hit: + description: Whether the optional workload-specific cache was restored exactly. + required: false + default: "false" + +runs: + using: composite + steps: + - name: Save cargo home cache + if: ${{ !cancelled() && inputs.cargo-home-cache-hit != 'true' && inputs.use-hermetic-cargo-home != 'true' }} + continue-on-error: true + uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + key: cargo-home-${{ inputs.cache-runner }}-${{ inputs.target }}-${{ inputs.profile }}-${{ inputs.lockfile-hash }}-${{ inputs.toolchain-hash }} + + - name: Save cargo home cache + if: ${{ !cancelled() && inputs.cargo-home-cache-hit != 'true' && inputs.use-hermetic-cargo-home == 'true' }} + continue-on-error: true + uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + ${{ github.workspace }}/.cargo-home/bin/ + ${{ github.workspace }}/.cargo-home/registry/index/ + ${{ github.workspace }}/.cargo-home/registry/cache/ + ${{ github.workspace }}/.cargo-home/git/db/ + key: cargo-home-${{ inputs.cache-runner }}-${{ inputs.target }}-${{ inputs.profile }}-${{ inputs.lockfile-hash }}-${{ inputs.toolchain-hash }} + + - name: Save sccache cache (fallback) + if: ${{ !cancelled() && inputs.use-sccache == 'true' && env.SCCACHE_GHA_ENABLED != 'true' }} + continue-on-error: true + uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + with: + path: ${{ env.SCCACHE_DIR }} + key: sccache-${{ inputs.cache-runner }}-${{ inputs.target }}-${{ inputs.profile }}-${{ inputs.lockfile-hash }}-${{ github.run_id }} + + - name: sccache stats + if: ${{ inputs.use-sccache == 'true' }} + continue-on-error: true + shell: bash + run: sccache --show-stats || true + + - name: sccache summary + if: ${{ inputs.use-sccache == 'true' }} + shell: bash + run: | + { + echo "### sccache stats — ${{ inputs.target }} (${{ inputs.summary-label }})"; + echo; + echo '```'; + sccache --show-stats || true; + echo '```'; + } >> "$GITHUB_STEP_SUMMARY" + + - name: Save supplemental cache + if: ${{ !cancelled() && inputs.supplemental-cache-path != '' && inputs.supplemental-cache-hit != 'true' }} + continue-on-error: true + uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + with: + path: ${{ inputs.supplemental-cache-path }} + key: ${{ inputs.supplemental-cache-key }} diff --git a/.github/workflows/rust-ci-full-nextest-platform.yml b/.github/workflows/rust-ci-full-nextest-platform.yml index 0238a90367..b3dfb3df96 100644 --- a/.github/workflows/rust-ci-full-nextest-platform.yml +++ b/.github/workflows/rust-ci-full-nextest-platform.yml @@ -185,42 +185,18 @@ jobs: if-no-files-found: error retention-days: 1 - - name: Save cargo home cache - if: always() && !cancelled() && steps.cargo_setup.outputs.cargo-home-cache-hit != 'true' - continue-on-error: true - uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + - name: Finish Cargo CI + if: always() + uses: ./.github/actions/finish-cargo-ci with: - path: | - ~/.cargo/bin/ - ~/.cargo/registry/index/ - ~/.cargo/registry/cache/ - ~/.cargo/git/db/ - key: cargo-home-${{ env.ARCHIVE_CACHE_RUNNER }}-${{ inputs.target }}-${{ inputs.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ steps.cargo_setup.outputs.toolchain-hash }} - - - name: Save sccache cache (fallback) - if: always() && !cancelled() && env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true' - continue-on-error: true - uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 - with: - path: ${{ env.SCCACHE_DIR }} - key: sccache-${{ env.ARCHIVE_CACHE_RUNNER }}-${{ inputs.target }}-${{ inputs.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ github.run_id }} - - - name: sccache stats - if: always() && env.USE_SCCACHE == 'true' - continue-on-error: true - run: sccache --show-stats || true - - - name: sccache summary - if: always() && env.USE_SCCACHE == 'true' - shell: bash - run: | - { - echo "### sccache stats — ${{ inputs.target }} (tests)"; - echo; - echo '```'; - sccache --show-stats || true; - echo '```'; - } >> "$GITHUB_STEP_SUMMARY" + target: ${{ inputs.target }} + profile: ${{ inputs.profile }} + cache-runner: ${{ env.ARCHIVE_CACHE_RUNNER }} + cargo-home-cache-hit: ${{ steps.cargo_setup.outputs.cargo-home-cache-hit }} + lockfile-hash: ${{ steps.cargo_setup.outputs.lockfile-hash }} + toolchain-hash: ${{ steps.cargo_setup.outputs.toolchain-hash }} + summary-label: tests + use-sccache: ${{ env.USE_SCCACHE }} shard: name: Tests shard ${{ matrix.shard }}/4 diff --git a/.github/workflows/rust-ci-full.yml b/.github/workflows/rust-ci-full.yml index 5031695066..196dea4233 100644 --- a/.github/workflows/rust-ci-full.yml +++ b/.github/workflows/rust-ci-full.yml @@ -337,57 +337,22 @@ jobs: path: codex-rs/target/**/cargo-timings/cargo-timing.html if-no-files-found: warn - # Save caches explicitly; make non-fatal so cache packaging - # never fails the overall job. Only save when key wasn't hit. - - name: Save cargo home cache - if: always() && !cancelled() && steps.cargo_setup.outputs.cargo-home-cache-hit != 'true' - continue-on-error: true - uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + - name: Finish Cargo CI + if: always() + uses: ./.github/actions/finish-cargo-ci with: - path: | - ~/.cargo/bin/ - ~/.cargo/registry/index/ - ~/.cargo/registry/cache/ - ~/.cargo/git/db/ - ${{ github.workspace }}/.cargo-home/bin/ - ${{ github.workspace }}/.cargo-home/registry/index/ - ${{ github.workspace }}/.cargo-home/registry/cache/ - ${{ github.workspace }}/.cargo-home/git/db/ - key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ steps.cargo_setup.outputs.toolchain-hash }} - - - name: Save sccache cache (fallback) - if: always() && !cancelled() && env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true' - continue-on-error: true - uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 - with: - path: ${{ env.SCCACHE_DIR }} - key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ github.run_id }} - - - name: sccache stats - if: always() && env.USE_SCCACHE == 'true' - continue-on-error: true - run: sccache --show-stats || true - - - name: sccache summary - if: always() && env.USE_SCCACHE == 'true' - shell: bash - run: | - { - echo "### sccache stats — ${{ matrix.target }} (${{ matrix.profile }})"; - echo; - echo '```'; - sccache --show-stats || true; - echo '```'; - } >> "$GITHUB_STEP_SUMMARY" - - - name: Save APT cache (musl) - if: always() && !cancelled() && (matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl') && steps.cache_apt_restore.outputs.cache-hit != 'true' - continue-on-error: true - uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 - with: - path: | - /var/cache/apt - key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 + target: ${{ matrix.target }} + profile: ${{ matrix.profile }} + cache-runner: ${{ matrix.runner }} + cargo-home-cache-hit: ${{ steps.cargo_setup.outputs.cargo-home-cache-hit }} + lockfile-hash: ${{ steps.cargo_setup.outputs.lockfile-hash }} + toolchain-hash: ${{ steps.cargo_setup.outputs.toolchain-hash }} + summary-label: ${{ matrix.profile }} + use-hermetic-cargo-home: ${{ contains(matrix.target, 'musl') && 'true' || 'false' }} + use-sccache: ${{ env.USE_SCCACHE }} + supplemental-cache-path: ${{ contains(matrix.target, 'musl') && '/var/cache/apt' || '' }} + supplemental-cache-key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 + supplemental-cache-hit: ${{ steps.cache_apt_restore.outputs.cache-hit }} tests_macos_aarch64: name: Tests — macos-15-xlarge - aarch64-apple-darwin