Skip to content

Commit

Permalink
Further improve GitHub Actions workflow code style
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Nov 11, 2024
1 parent f41a58c commit a7a3526
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 39 deletions.
75 changes: 48 additions & 27 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ permissions:

env:
CARGO_TERM_COLOR: always
CLICOLOR: 1
CLICOLOR: '1'

jobs:
pure-rust-build:
Expand All @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down
24 changes: 12 additions & 12 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,15 @@ 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'
run: |
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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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/<arch>-apple-darwin/' -- "$1.txt"; }
mask() { sed -E 's/\w+-apple-darwin/<arch>-apple-darwin/' -- "$1.txt"; }
diff -- <(mask aarch64) <(mask universal)
diff -- <(mask x86_64) <(mask universal)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}

0 comments on commit a7a3526

Please sign in to comment.