Skip to content

Commit f768d50

Browse files
committed
Rough fix for bash vs. pwsh splitting syntax bug
The `TEST_ARGS` environment variable needs to be split. It was introduced to avoid a `${{ matrix.test-args }}` interpolation directly into a shell script (where the intention for how many arguments to pass would be unclear, and where automated tooling has trouble identifying that it's not a template injection vulnerability). The problem is that the syntax used, unquoted `$TEST_ARGS`, works on `bash` but not `pwsh`. Likewise, the line-contination introduced along with it was written with a backslash, but PowerShell uses a backtick. This "fixes" the problem by writing shell-specific code, but in one step that means duplicating the step. This therefore needs to be refactored, by replacing this fix with a completely different one. The purpose of fixing it this way is really just to verify the bug. A better fix might entail going back to interpolating `${{ }}` into shell scripts in the few cases that this is by far the simplest way to do get the needed effect portably across shells. Usually the solution is to just force the same shell on all OSes, such as with `shell: bash`, and use the syntax of the chosen shell. But `bash` and `pwsh` can give subtly different environments, which is why we allow the default to be used in `nextest` runs, especially on Windows. Thus, that usual solution is not available.
1 parent b6f0ab3 commit f768d50

File tree

1 file changed

+12
-4
lines changed

1 file changed

+12
-4
lines changed

.github/workflows/ci.yml

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,16 @@ jobs:
249249
- uses: taiki-e/install-action@cc33365ec7e3350bc47bf935f247582cc6f68344 # v2.65.12
250250
with:
251251
tool: nextest
252+
# FIXME: This "Test (nextest)" duplication is temporary for testing and must be refactored.
252253
- name: Test (nextest)
254+
if: startsWith(matrix.os, 'windows')
255+
env:
256+
GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: '1'
257+
TEST_ARGS: ${{ matrix.test-args }}
258+
run: |
259+
cargo nextest run --workspace --no-fail-fast -- (-split $Env:TEST_ARGS)
260+
- name: Test (nextest)
261+
if: ${{ !startsWith(matrix.os, 'windows') }}
253262
env:
254263
GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: '1'
255264
TEST_ARGS: ${{ matrix.test-args }}
@@ -279,8 +288,7 @@ jobs:
279288
env:
280289
TEST_ARGS: ${{ matrix.test-args }}
281290
run: |
282-
cargo nextest run -p gix-path --no-fail-fast -- \
283-
$TEST_ARGS # Word splitting intended.
291+
cargo nextest run -p gix-path --no-fail-fast -- (-split $Env:TEST_ARGS)
284292
285293
test-fixtures-windows:
286294
strategy:
@@ -312,8 +320,8 @@ jobs:
312320
GIX_TEST_IGNORE_ARCHIVES: '1'
313321
TEST_ARGS: ${{ matrix.test-args }}
314322
run: |
315-
cargo nextest --profile=with-xml run --workspace --no-fail-fast -- \
316-
$TEST_ARGS # Word splitting intended.
323+
cargo nextest --profile=with-xml run --workspace --no-fail-fast -- `
324+
(-split $Env:TEST_ARGS)
317325
continue-on-error: true
318326
- name: Check for errors
319327
run: |

0 commit comments

Comments
 (0)