From cb16d947260fc4b6fd088ae87bcd32d126a3622d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Nov 2024 14:32:08 -0500 Subject: [PATCH 1/3] A few more CI workflow comment and style improvements These didn't make it into #1668. Besides comments, the changes are for consistency with the prevailing style, usually by omitting redundant YAML quoting. Removal of the outer double quotes in the `if` in `tests-pass` is a case of this, and produces an equivalent node in parsing (i.e. its equivalence does not depend on anything about GHA itself). But just to be sure, I did run yq '.jobs.tests-pass.steps[0].if' .github/workflows/ci.yml before and after the change, to ensure the output was the same. The other change here that deserves comment is the removal of `--` as an argument to a `diff` command. When any path argument is formed from paramter expansion or from a glob with a leading `*` or other globbing character, `--` helps express that the following arguments are not options. For `git diff`, a `--` expresses that the following arguments are neither options nor refs, but paths, so all `git diff` commands with paths in the CI workflows use `--` even if no shell expansions are involved. (In practice this means `--` is often useful for `diff` with paths and, based on this habit, I had inadvertently written a `--` where neither of the above scenarios applied. But that had actually decreased stylistic consistency because we are not using `--` elsewhere that the meaning of all arguments after it is unambiguous even without examining any surrounding context.) --- .github/workflows/ci.yml | 15 +++++++-------- .github/workflows/release.yml | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd0a82649f0..44578bd1da4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,15 +84,14 @@ jobs: - uses: Swatinem/rust-cache@v2 - name: Setup dependencies (macos) if: startsWith(matrix.os, 'macos') - run: - brew install tree openssl gnu-sed + run: brew install tree openssl gnu-sed - name: "cargo check default features" if: startsWith(matrix.os, 'windows') run: cargo check --workspace --bins --examples - uses: taiki-e/install-action@v2 with: tool: nextest - - name: "Test (nextest)" + - name: Test (nextest) env: GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: '1' run: cargo nextest run --workspace --no-fail-fast @@ -157,9 +156,9 @@ jobs: - uses: taiki-e/install-action@v2 with: tool: cross - - name: "check" + - name: check run: cross check -p gix --target ${{ matrix.target }} - - name: "Test (unit)" + - name: Test (unit) # run high-level unit tests that exercise a lot of code while being pure Rust to ease building test binaries. # TODO: figure out why `git` doesn't pick up environment configuration so build scripts fail when using `-p gix`. run: cross test -p gix-hashtable --target ${{ matrix.target }} @@ -275,7 +274,7 @@ jobs: defaults: run: - shell: bash + shell: bash # Use bash even on Windows, if we ever reenable windows-latest for testing. steps: - uses: actions/checkout@v4 @@ -327,7 +326,7 @@ jobs: - name: Each job must block PRs or be declared not to run: | sort -m blocking-jobs.txt expected-nonblocking-jobs.txt | - diff --color=always -U1000 -- - all-jobs.txt + diff --color=always -U1000 - all-jobs.txt # Dummy job to have a stable name for the "all tests pass" requirement tests-pass: @@ -349,7 +348,7 @@ jobs: steps: - name: Fail if ANY dependency has failed or cancelled - if: "contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')" + if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') run: exit 1 - name: OK run: exit 0 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 040022fb8d7..7323d36986e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ on: workflow_dispatch: permissions: - contents: read # Set more permissively in jobs that need `write`. + contents: read # This is set more permissively in jobs that need `write`. defaults: run: From 54c778568e91ed0620eb549f61dc8fd3e3811f3c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Nov 2024 14:51:05 -0500 Subject: [PATCH 2/3] Clarify why pure-rust-build needs gcc And try omitting libc-dev. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44578bd1da4..56416b2ab14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,7 +31,7 @@ jobs: - name: Prerequisites run: | apt-get update - apt-get install --no-install-recommends -y ca-certificates curl gcc libc-dev # gcc is required as OS abstraction + apt-get install --no-install-recommends -y ca-certificates curl gcc # rustc invokes gcc for linking - name: Verify environment is sufficiently minimal for the test run: | set -x From 8c71bd15d6dd9890f658e4bac2868baceafb180b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 12 Nov 2024 15:06:36 -0500 Subject: [PATCH 3/3] Add back libc-dev; reorganize to explain both it and gcc --- .github/workflows/ci.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56416b2ab14..73ca728ccdc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,8 +30,15 @@ jobs: - uses: actions/checkout@v4 - name: Prerequisites run: | + prerequisites=( + ca-certificates + curl + gcc # rustc calls gcc to invoke the linker. + libc-dev # rustc, in the toolchain we are using, dynamically links to the system libc. + ) apt-get update - apt-get install --no-install-recommends -y ca-certificates curl gcc # rustc invokes gcc for linking + apt-get install --no-install-recommends -y -- "${prerequisites[@]}" + shell: bash - name: Verify environment is sufficiently minimal for the test run: | set -x