From a7a352639e1552113834541cfb100258cad3cc5f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 Nov 2024 23:09:27 -0500 Subject: [PATCH] Further improve GitHub Actions workflow code style This makes more stylistic adjustments to YAML workflows files. (Previous recent adjustments were, for the most part, only those needed to ensure clarity would be preserved while adding `permissions:`. This commit does some others that weren't needed for that.) The changes made here are: - Don't explicitly set the `CI` environment variable in the `test` job. This variable is guaranteed to be set automatically with the value of `true` by the GitHub Actions runner. Furthermore, no environment variables to indicate CI are explicitly set for other jobs, even though others rely on their presence as well due to the use of `is_ci` through `gix-testtools`. - Specify all environment variables' values as strings. Most values in YAML are strings without explicit quoting. This does not add quotation marks in those cases. But some values are parsed as some other type, such as integer, unless quoted. That makes it less obvious what the value will actually be in the environment, since it will be implicitly converted to a string, which does not always consist of the same sequence of characters as the original expression. This effect is most prominent for booleans (e.g. unquoted `yes` and `true` in YAML have the same meaning) but not entirely limited to them. In addition, such expressions may also lead to confusion if it is misread to mean that they will retain any kind of type information. So this quotes such values (except when other changes here remove them). - Minor quoting style adjustments, for YAML strings are quoted. Omit quotes when clearly not needed, and use single-quotes by default when quotes are needed. - Use 2-space indent inside scripts in script steps for consistency with other scripts. Most existing multi-line script steps, as well as all shell scripts, use 2-space indents already. - Remove `\` in a script step where it was not needed for line continuation because it followed a `|`. - In the `wasm` job's script steps, put `name:` and, when present, `if:`, ahead of `run:`, so it's clear what each step is for. (This was already done where applicable in other jobs.) - In the `wasm` job's script steps, split single-line `run:` values with `&&` into separate commands, one per line, where doing so has the same effect due to the `-e` option the CI runner automatically passes to `bash` shells unless `shell:` is overridden with a value that contains `{0}`. - In the `wasm` job's script steps, split single-line `run:` values with `;` into separate lines, including running loops over multiple lines, for readability. - In the `wasm` job, extract `${{ matrix.target }}` to an environment variable. This seems to make the steps slightly more readable, since `wasm` steps make heavy use of it. - Extremely minor adjustment to array style, for consistency. - In commands, use `--` where guaranteed to be supported if any non-option argument begins with parameter expansion or a glob. This was already almost, but not quite, already done. Since the possible values can be inferred from nearby code and do not begin with `-`, the reason is to clearly communicate that they are non-option arguments, rather than to fix any actual bug. (This continues to be avoided where is not guaranteed correct.) - Use the `-e` option before a pattern argument to `grep` formed through parameter expansion (same rationale as `--` above). - Use `-E` rather than `-r` with `sed`, since `-E` is standardized. --- .github/workflows/ci.yml | 75 ++++++++++++++++++++++------------- .github/workflows/release.yml | 24 +++++------ 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af538f3623b..eba953308b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ permissions: env: CARGO_TERM_COLOR: always - CLICOLOR: 1 + CLICOLOR: '1' jobs: pure-rust-build: @@ -36,14 +36,14 @@ jobs: run: | set -x for pattern in cmake g++ libssl-dev make pkgconf pkg-config; do - if dpkg-query --status -- "$pattern"; then - exit 1 - fi + if dpkg-query --status -- "$pattern"; then + exit 1 + fi done for cmd in cmake g++ make pkgconf pkg-config; do - if command -v -- "$cmd"; then - exit 1 - fi + if command -v -- "$cmd"; then + exit 1 + fi done - name: Install Rust via Rustup run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal @@ -65,8 +65,7 @@ jobs: tool: nextest - name: test env: - CI: true - GIX_TEST_IGNORE_ARCHIVES: 1 + GIX_TEST_IGNORE_ARCHIVES: '1' run: just ci-test test-fast: @@ -95,7 +94,7 @@ jobs: tool: nextest - name: "Test (nextest)" env: - GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: 1 + GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: '1' run: cargo nextest run --workspace --no-fail-fast - name: Doctest run: cargo test --workspace --doc --no-fail-fast @@ -127,8 +126,8 @@ jobs: [xml]$junit_xml = Get-Content -Path 'target/nextest/with-xml/junit.xml' $actual_failures = $junit_xml.SelectNodes("//testcase[failure]") | - ForEach-Object { "$($_.classname) $($_.name)" } | - Sort-Object + ForEach-Object { "$($_.classname) $($_.name)" } | + Sort-Object Write-Output $actual_failures Set-Content -Path 'actual-failures.txt' -Value $actual_failures @@ -137,7 +136,7 @@ jobs: # Fail on any differences, even unexpectedly passing tests, so they can be investigated. # (If the job is made blocking for PRs, it may make sense to make this less stringent.) git --no-pager diff --no-index --exit-code --unified=1000000 --color=always -- ` - etc/test-fixtures-windows-expected-failures-see-issue-1358.txt actual-failures.txt + etc/test-fixtures-windows-expected-failures-see-issue-1358.txt actual-failures.txt test-32bit: runs-on: ubuntu-latest @@ -183,8 +182,8 @@ jobs: run: cargo fmt --all -- --check - name: Run cargo diet run: | - curl -LSfs https://raw.githubusercontent.com/the-lean-crate/cargo-diet/master/ci/install.sh | \ - sh -s -- --git the-lean-crate/cargo-diet --target x86_64-unknown-linux-musl --tag v1.2.4 + curl -LSfs https://raw.githubusercontent.com/the-lean-crate/cargo-diet/master/ci/install.sh | + sh -s -- --git the-lean-crate/cargo-diet --target x86_64-unknown-linux-musl --tag v1.2.4 # Let's not fail CI for this, it will fail locally often enough, and a crate a little bigger # than allows is no problem either if it comes to that. @@ -219,22 +218,44 @@ jobs: matrix: target: [ wasm32-unknown-unknown, wasm32-wasi ] + env: + TARGET: ${{ matrix.target }} + steps: - - uses: actions/checkout@master + - uses: actions/checkout@v4 - name: Install Rust - run: rustup update stable && rustup default stable && rustup target add ${{ matrix.target }} + run: | + rustup update stable + rustup default stable + rustup target add "$TARGET" - uses: Swatinem/rust-cache@v2 - - run: set +x; for name in gix-sec; do (cd $name && cargo build --target ${{ matrix.target }}); done - name: "WASI only: crates without feature toggle" + - name: 'WASI only: crates without feature toggle' if: endsWith(matrix.target, '-wasi') - - run: set +x; for name in gix-actor gix-attributes gix-bitmap gix-chunk gix-command gix-commitgraph gix-config-value gix-date gix-glob gix-hash gix-hashtable gix-mailmap gix-object gix-packetline gix-path gix-pathspec gix-prompt gix-quote gix-refspec gix-revision gix-traverse gix-url gix-validate; do (cd $name && cargo build --target ${{ matrix.target }}); done - name: crates without feature toggles - - run: set +x; for feature in progress fs-walkdir-parallel parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do (cd gix-features && cargo build --features $feature --target ${{ matrix.target }}); done - name: features of gix-features - - run: set +x; for name in gix-pack; do (cd $name && cargo build --features wasm --target ${{ matrix.target }}); done - name: crates with 'wasm' feature - - run: cd gix-pack && cargo build --all-features --target ${{ matrix.target }} - name: gix-pack with all features (including wasm) + run: | + set +x + for name in gix-sec; do + (cd -- "$name" && cargo build --target "$TARGET") + done + - name: crates without feature toggles + run: | + set +x + for name in gix-actor gix-attributes gix-bitmap gix-chunk gix-command gix-commitgraph gix-config-value gix-date gix-glob gix-hash gix-hashtable gix-mailmap gix-object gix-packetline gix-path gix-pathspec gix-prompt gix-quote gix-refspec gix-revision gix-traverse gix-url gix-validate; do + (cd -- "$name" && cargo build --target "$TARGET") + done + - name: features of gix-features + run: | + set +x + for feature in progress fs-walkdir-parallel parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do + (cd gix-features && cargo build --features "$feature" --target "$TARGET") + done + - name: crates with 'wasm' feature + run: | + set +x + for name in gix-pack; do + (cd -- "$name" && cargo build --features wasm --target "$TARGET") + done + - name: gix-pack with all features (including wasm) + run: cd gix-pack && cargo build --all-features --target "$TARGET" check-packetline: strategy: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ade9a7dd56a..040022fb8d7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -266,7 +266,7 @@ jobs: - name: Pre-populate directory for archive run: | mkdir -- "$ARCHIVE" - cp {README.md,LICENSE-*,CHANGELOG.md} "$ARCHIVE/" + cp -- {README.md,LICENSE-*,CHANGELOG.md} "$ARCHIVE/" - name: Build archive (Windows) if: matrix.os == 'windows-latest' @@ -274,7 +274,7 @@ jobs: file -- "$TARGET_DIR/$PROFILE"/{ein,gix}.exe cp -- "$TARGET_DIR/$PROFILE"/{ein,gix}.exe "$ARCHIVE/" 7z a "$ARCHIVE.zip" "$ARCHIVE" - /usr/bin/core_perl/shasum --algorithm=256 --binary "$ARCHIVE.zip" > "$ARCHIVE.zip.sha256" + /usr/bin/core_perl/shasum --algorithm=256 --binary -- "$ARCHIVE.zip" > "$ARCHIVE.zip.sha256" echo "ASSET=$ARCHIVE.zip" >> "$GITHUB_ENV" echo "ASSET_SUM=$ARCHIVE.zip.sha256" >> "$GITHUB_ENV" @@ -283,8 +283,8 @@ jobs: run: | file -- "$TARGET_DIR/$PROFILE"/{ein,gix} cp -- "$TARGET_DIR/$PROFILE"/{ein,gix} "$ARCHIVE/" - tar czf "$ARCHIVE.tar.gz" "$ARCHIVE" - shasum --algorithm=256 --binary "$ARCHIVE.tar.gz" > "$ARCHIVE.tar.gz.sha256" + tar czf "$ARCHIVE.tar.gz" -- "$ARCHIVE" + shasum --algorithm=256 --binary -- "$ARCHIVE.tar.gz" > "$ARCHIVE.tar.gz.sha256" echo "ASSET=$ARCHIVE.tar.gz" >> "$GITHUB_ENV" echo "ASSET_SUM=$ARCHIVE.tar.gz.sha256" >> "$GITHUB_ENV" @@ -329,7 +329,7 @@ jobs: - name: Unpack single-architecture releases run: | - shasum --check "$(name aarch64).tar.gz.sha256" "$(name x86_64).tar.gz.sha256" + shasum --check -- "$(name aarch64).tar.gz.sha256" "$(name x86_64).tar.gz.sha256" tar xf "$(name aarch64).tar.gz" tar xf "$(name x86_64).tar.gz" @@ -350,8 +350,8 @@ jobs: - name: Build archive run: | - tar czf "$ARCHIVE.tar.gz" "$ARCHIVE" - shasum --algorithm=256 --binary "$ARCHIVE.tar.gz" > "$ARCHIVE.tar.gz.sha256" + tar czf "$ARCHIVE.tar.gz" -- "$ARCHIVE" + shasum --algorithm=256 --binary -- "$ARCHIVE.tar.gz" > "$ARCHIVE.tar.gz.sha256" echo "ASSET=$ARCHIVE.tar.gz" >> "$GITHUB_ENV" echo "ASSET_SUM=$ARCHIVE.tar.gz.sha256" >> "$GITHUB_ENV" @@ -389,12 +389,12 @@ jobs: - name: Extract macOS asset names by architecture run: | for arch in aarch64 x86_64 universal; do - grep -Fw "$arch-apple-darwin" assets.txt | sort | tee -- "$arch.txt" + grep -Fwe "$arch-apple-darwin" assets.txt | sort | tee -- "$arch.txt" done - name: Check macOS archive features run: | - mask() { sed -r 's/\w+-apple-darwin/-apple-darwin/' -- "$1.txt"; } + mask() { sed -E 's/\w+-apple-darwin/-apple-darwin/' -- "$1.txt"; } diff -- <(mask aarch64) <(mask universal) diff -- <(mask x86_64) <(mask universal) @@ -432,7 +432,7 @@ jobs: installation: strategy: matrix: - build: [win-msvc, win-gnu, win32-msvc, win32-gnu] + build: [ win-msvc, win-gnu, win32-msvc, win32-gnu ] include: - build: win-msvc os: windows-latest @@ -465,8 +465,8 @@ jobs: msystem: MINGW${{ startsWith(matrix.target, 'i686-') && '32' || '64' }} pacboy: cc:p path-type: inherit - - name: "Install prerequisites" + - name: Install prerequisites run: vcpkg install zlib:x64-windows-static-md - - name: "Installation from crates.io: gitoxide" + - name: 'Installation from crates.io: gitoxide' run: cargo +${{ matrix.rust }} install --target ${{ matrix.target }} --no-default-features --features max-pure --target-dir install-artifacts --debug --force gitoxide shell: msys2 {0}