From 7718daea845b06e68eb467cecde27223cb5cac95 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 6 May 2025 15:52:07 -0700 Subject: [PATCH] ci: remove binary size check Remove the "binary size check" workflow because it is often broken and now and then makes me sit through a discussion about its security needs. These discussions always make me feel very silly. I originally hoped this CI job would be a useful part of assuring that we would not accidentally bloat binaries when backtrace merges into std. However, it feels like that is not achieved by this workflow and at some point it has definitely outweighed the cost of maintaining it. We can readd something if we find a better way to implement things which is notably less jank and gets less complaints. --- .../actions/build-with-patched-std/action.yml | 50 ------ .../report-code-size-changes/action.yml | 111 ------------ .github/workflows/check-binary-size.yml | 159 ------------------ 3 files changed, 320 deletions(-) delete mode 100644 .github/actions/build-with-patched-std/action.yml delete mode 100644 .github/actions/report-code-size-changes/action.yml delete mode 100644 .github/workflows/check-binary-size.yml diff --git a/.github/actions/build-with-patched-std/action.yml b/.github/actions/build-with-patched-std/action.yml deleted file mode 100644 index 46edb7ee..00000000 --- a/.github/actions/build-with-patched-std/action.yml +++ /dev/null @@ -1,50 +0,0 @@ -# Github composite action to build a single-source-file test binary with an -# already-checked-out version of Rust's stdlib, that will be patched with a -# given revision of the backtrace crate. - -name: Build with patched std -description: > - Build a binary with a version of std that's had a specific revision of - backtrace patched in. -inputs: - backtrace-commit: - description: The git commit of backtrace to patch in to std - required: true - main-rs: - description: The (single) source code file to compile - required: true - rustc-dir: - description: The root directory of the rustc repo - required: true -outputs: - test-binary-size: - description: The size in bytes of the built test binary - value: ${{ steps.measure.outputs.test-binary-size }} -runs: - using: composite - steps: - - shell: bash - id: measure - env: - RUSTC_FLAGS: -Copt-level=3 -Cstrip=symbols - # This symlink is made by Build::new() in the bootstrap crate, using a - # symlink on Linux and a junction on Windows, so it will exist on both - # platforms. - RUSTC_BUILD_DIR: build/host - RUST_BACKTRACE: 1 - working-directory: ${{ inputs.rustc-dir }} - run: | - set -x - rm -rf "$RUSTC_BUILD_DIR/stage0-std" - - (cd library/backtrace && git checkout ${{ inputs.backtrace-commit }}) - git add library/backtrace - - python3 x.py build library --stage 0 - - TEMP_BUILD_OUTPUT=$(mktemp test-binary-XXXXXXXX) - "$RUSTC_BUILD_DIR/stage0-sysroot/bin/rustc" $RUSTC_FLAGS "${{ inputs.main-rs }}" -o "$TEMP_BUILD_OUTPUT" - BINARY_SIZE=$(stat -c '%s' "$TEMP_BUILD_OUTPUT") - rm "$TEMP_BUILD_OUTPUT" - - echo "test-binary-size=$BINARY_SIZE" >> "$GITHUB_OUTPUT" diff --git a/.github/actions/report-code-size-changes/action.yml b/.github/actions/report-code-size-changes/action.yml deleted file mode 100644 index ede26975..00000000 --- a/.github/actions/report-code-size-changes/action.yml +++ /dev/null @@ -1,111 +0,0 @@ -# Github composite action to report on code size changes across different -# platforms. - -name: Report binary size changes on PR -description: | - Report on code size changes across different platforms resulting from a PR. - The only input argument is the path to a directory containing a set of - "*.json" files (extension required), each file containing the keys: - - - platform: the platform that the code size change was measured on - - reference: the size in bytes of the reference binary (base of PR) - - updated: the size in bytes of the updated binary (head of PR) - - The size is reported as a comment on the PR (accessed via context). -inputs: - data-directory: - description: > - Path to directory containing size data as a set of "*.json" files. - required: true -runs: - using: composite - steps: - - name: Post a PR comment if the size has changed - uses: actions/github-script@v6 - env: - DATA_DIRECTORY: ${{ inputs.data-directory }} - with: - script: | - const fs = require("fs"); - - const size_dir = process.env.DATA_DIRECTORY; - - // Map the set of all the *.json files into an array of objects. - const globber = await glob.create(`${size_dir}/*.json`); - const files = await globber.glob(); - const sizes = files.map(path => { - const contents = fs.readFileSync(path); - return JSON.parse(contents); - }); - - // Map each object into some text, but only if it shows any difference - // to report. - const size_reports = sizes.flatMap(size_data => { - const platform = size_data["platform"]; - const reference = size_data["reference"]; - const updated = size_data["updated"]; - - if (!(reference > 0)) { - core.setFailed(`Reference size invalid: ${reference}`); - return; - } - - if (!(updated > 0)) { - core.setFailed(`Updated size invalid: ${updated}`); - return; - } - - const formatter = Intl.NumberFormat("en", { - useGrouping: "always" - }); - - const updated_str = formatter.format(updated); - const reference_str = formatter.format(reference); - - const diff = updated - reference; - const diff_pct = (updated / reference) - 1; - - const diff_str = Intl.NumberFormat("en", { - useGrouping: "always", - sign: "exceptZero" - }).format(diff); - - const diff_pct_str = Intl.NumberFormat("en", { - style: "percent", - useGrouping: "always", - sign: "exceptZero", - maximumFractionDigits: 2 - }).format(diff_pct); - - if (diff !== 0) { - // The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines, - // which is interpreted as a code block by Markdown. - const report = `On platform \`${platform}\`: - - - Original binary size: **${reference_str} B** - - Updated binary size: **${updated_str} B** - - Difference: **${diff_str} B** (${diff_pct_str}) - - `; - - return [report]; - } else { - return []; - } - }); - - // If there are any size changes to report, format a comment and post - // it. - if (size_reports.length > 0) { - const comment_sizes = size_reports.join(""); - const body = `Code size changes for a hello-world Rust program linked with libstd with backtrace: - - ${comment_sizes}`; - - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body - }); - } diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml deleted file mode 100644 index 5e8ba72f..00000000 --- a/.github/workflows/check-binary-size.yml +++ /dev/null @@ -1,159 +0,0 @@ -# This workflow checks if a PR commit has changed the size of a hello world Rust program. -# It downloads Rustc and compiles two versions of a stage0 compiler - one using the base commit -# of the PR, and one using the latest commit in the PR. -# If the size of the hello world program has changed, it posts a comment to the PR. -name: Check binary size - -on: - pull_request_target: - # HACK(jubilee): something broke the distributed LLVM libso and I don't know what. - branches: [] -# - master - -# Both the "measure" and "report" jobs need to know this. -env: - SIZE_DATA_DIR: sizes - RUST_BACKTRACE: 1 - -# Responsibility is divided between two jobs "measure" and "report", so that the -# job that builds (and potentnially runs) untrusted code does not have PR write -# permission, and vice-versa. -jobs: - measure: - name: Check binary size - strategy: - matrix: - # FIXME(jubilee): the immutable upload needs us to disambiguate things - # platform: [ubuntu-latest, windows-latest] - platform: [ubuntu-latest] - runs-on: ${{ matrix.platform }} - permissions: - contents: read - env: - # This cannot be used as a context variable in the 'uses' key later. If it - # changes, update those steps too. - BACKTRACE_DIR: backtrace - RUSTC_DIR: rustc - TEST_MAIN_RS: foo.rs - BASE_COMMIT: ${{ github.event.pull_request.base.sha }} - HEAD_COMMIT: ${{ github.event.pull_request.head.sha }} - SIZE_DATA_FILE: size-${{ strategy.job-index }}.json - steps: - - name: Print info - shell: bash - run: | - echo "Current SHA: $HEAD_COMMIT" - echo "Base SHA: $BASE_COMMIT" - # Note: the backtrace source that's cloned here is NOT the version to be - # patched in to std. It's cloned here to access the Github action for - # building and measuring the test binary. - - name: Clone backtrace to access Github action - uses: actions/checkout@v4 - with: - path: ${{ env.BACKTRACE_DIR }} - - name: Clone Rustc - uses: actions/checkout@v4 - with: - repository: rust-lang/rust - path: ${{ env.RUSTC_DIR }} - # Arbitrary version from 2024-04-28 - ref: cc74ed08d53fbb440b4ab70035a92d89d418d23c - - name: Set up std repository and backtrace submodule for size test - shell: bash - working-directory: ${{ env.RUSTC_DIR }} - env: - PR_SOURCE_REPO: ${{ github.event.pull_request.head.repo.full_name }} - run: | - set -x - # Bootstrap config - cat < bootstrap.toml - change-id = 9999999 - [llvm] - download-ci-llvm = "if-unchanged" - [rust] - incremental = false - EOF - - # Test program source - cat < $TEST_MAIN_RS - fn main() { - panic!(); - } - EOF - - git submodule update --init library/backtrace - - cd library/backtrace - git remote add head-pr "https://github.com/$PR_SOURCE_REPO" - git fetch --all - - name: Build binary with base version of backtrace - uses: ./backtrace/.github/actions/build-with-patched-std - with: - backtrace-commit: ${{ env.BASE_COMMIT }} - main-rs: ${{ env.TEST_MAIN_RS }} - rustc-dir: ${{ env.RUSTC_DIR }} - id: size-reference - - name: Build binary with PR version of backtrace - uses: ./backtrace/.github/actions/build-with-patched-std - with: - backtrace-commit: ${{ env.HEAD_COMMIT }} - main-rs: ${{ env.TEST_MAIN_RS }} - rustc-dir: ${{ env.RUSTC_DIR }} - id: size-updated - # There is no built-in way to "collect" all the outputs of a set of jobs - # run with a matrix strategy. Subsequent jobs that have a "needs" - # dependency on this one will be run once, when the last matrix job is - # run. Appending data to a single file within a matrix is subject to race - # conditions. So we write the size data to files with distinct names - # generated from the job index. - - name: Write sizes to file - uses: actions/github-script@v6 - env: - SIZE_REFERENCE: ${{ steps.size-reference.outputs.test-binary-size }} - SIZE_UPDATED: ${{ steps.size-updated.outputs.test-binary-size }} - PLATFORM: ${{ matrix.platform }} - with: - script: | - const fs = require("fs"); - const path = require("path"); - - fs.mkdirSync(process.env.SIZE_DATA_DIR, {recursive: true}); - - const output_data = JSON.stringify({ - platform: process.env.PLATFORM, - reference: process.env.SIZE_REFERENCE, - updated: process.env.SIZE_UPDATED, - }); - - // The "wx" flag makes this fail if the file exists, which we want, - // because there should be no collisions. - fs.writeFileSync( - path.join(process.env.SIZE_DATA_DIR, process.env.SIZE_DATA_FILE), - output_data, - { flag: "wx" }, - ); - - name: Upload size data - uses: actions/upload-artifact@v4 - with: - name: size-files - path: ${{ env.SIZE_DATA_DIR }}/${{ env.SIZE_DATA_FILE }} - retention-days: 1 - if-no-files-found: error - report: - name: Report binary size changes - runs-on: ubuntu-latest - needs: measure - permissions: - pull-requests: write - steps: - # Clone backtrace to access Github composite actions to report size. - - uses: actions/checkout@v4 - - name: Download size data - uses: actions/download-artifact@v4 - with: - name: size-files - path: ${{ env.SIZE_DATA_DIR }} - - name: Analyze and report size changes - uses: ./.github/actions/report-code-size-changes - with: - data-directory: ${{ env.SIZE_DATA_DIR }}