Audit and remove copied || true failure-masking #6

Closed
opened 2026-07-04 19:08:31 +03:00 by oleks · 2 comments
Owner

~18 s390x files carry kill $ATTIC_PID && wait $ATTIC_PID || true plus 4 occurrences in servers/mermaid-gpu pipelines. Per standing rule, || true is banned — replace with explicit exit-code handling. The attic-cleanup instances are benign but must still be converted; the pattern is copied uncritically.

~18 s390x files carry `kill $ATTIC_PID && wait $ATTIC_PID || true` plus 4 occurrences in servers/mermaid-gpu pipelines. Per standing rule, `|| true` is banned — replace with explicit exit-code handling. The attic-cleanup instances are benign but must still be converted; the pattern is copied uncritically.
oleks added this to the CI Scripts Optimization milestone 2026-07-04 19:08:31 +03:00
Collaborator

Removed every || true in scope (building/s390x + building/arm64), replacing with explicit exit-code handling that still tolerates the one benign case (the backgrounded attic watch-store exiting non-zero after being killed):

- if ! kill $ATTIC_PID 2>/dev/null; then echo "attic watch-store already exited"; fi
- if ! wait $ATTIC_PID 2>/dev/null; then echo "attic watch-store exited non-zero after kill (expected)"; fi

10 via the template (same commits as #1). 5 fixed directly since they're bespoke pipelines outside the template's scope: scipy-s390x ce76a0e, scikit-learn-s390x 38db510, geesefs-s390x cf25d44, attic-client-s390x 78f77d4, onnxruntime-s390x 6d27d73.

grep -rl '|| true' building/s390x/*/.woodpecker.yaml building/arm64/*/.woodpecker.yaml building/arm64/mempalace/.woodpecker/*.yaml now returns nothing. All touched files pass woodpecker-cli lint.

Note: the issue body also mentions "4 occurrences in servers/mermaid-gpu pipelines" — that's outside my assigned scope (building/s390x, building/arm64, oleks/ci-scripts only), left untouched. Flagging to team-lead as a separate follow-up if wanted.

Closing.

Removed every `|| true` in scope (building/s390x + building/arm64), replacing with explicit exit-code handling that still tolerates the one benign case (the backgrounded `attic watch-store` exiting non-zero after being killed): ``` - if ! kill $ATTIC_PID 2>/dev/null; then echo "attic watch-store already exited"; fi - if ! wait $ATTIC_PID 2>/dev/null; then echo "attic watch-store exited non-zero after kill (expected)"; fi ``` 10 via the template (same commits as #1). 5 fixed directly since they're bespoke pipelines outside the template's scope: scipy-s390x ce76a0e, scikit-learn-s390x 38db510, geesefs-s390x cf25d44, attic-client-s390x 78f77d4, onnxruntime-s390x 6d27d73. `grep -rl '|| true' building/s390x/*/.woodpecker.yaml building/arm64/*/.woodpecker.yaml building/arm64/mempalace/.woodpecker/*.yaml` now returns nothing. All touched files pass `woodpecker-cli lint`. Note: the issue body also mentions "4 occurrences in servers/mermaid-gpu pipelines" — that's outside my assigned scope (building/s390x, building/arm64, oleks/ci-scripts only), left untouched. Flagging to team-lead as a separate follow-up if wanted. Closing.
Collaborator

Follow-up: fixed the last || true residue flagged as out-of-scope above — oleks/mermaid-gpu .woodpecker/test-mermaid-gpu-worker.yaml lines 37 and 42 (cat /etc/os-release || true, ls -la /dev/kfd /dev/dri/ || true). Since these are diagnostic probes whose whole point is "don't fail the pipeline if the file/device is missing," replaced with explicit branching that preserves that intent without the blanket mask:

- if ! cat /etc/os-release; then echo "/etc/os-release unavailable"; fi
- if ! ls -la /dev/kfd /dev/dri/; then echo "/dev/kfd or /dev/dri unavailable"; fi

Commit: oleks/mermaid-gpu 8897228 (main). droplet-image.yaml was already clean (another worker had extracted its shell into ci/*.sh). Passed woodpecker-cli lint and the repo's strict shellcheck/yamllint pre-receive hook on push.

Follow-up: fixed the last `|| true` residue flagged as out-of-scope above — oleks/mermaid-gpu `.woodpecker/test-mermaid-gpu-worker.yaml` lines 37 and 42 (`cat /etc/os-release || true`, `ls -la /dev/kfd /dev/dri/ || true`). Since these are diagnostic probes whose whole point is "don't fail the pipeline if the file/device is missing," replaced with explicit branching that preserves that intent without the blanket mask: ``` - if ! cat /etc/os-release; then echo "/etc/os-release unavailable"; fi - if ! ls -la /dev/kfd /dev/dri/; then echo "/dev/kfd or /dev/dri unavailable"; fi ``` Commit: oleks/mermaid-gpu 8897228 (main). `droplet-image.yaml` was already clean (another worker had extracted its shell into `ci/*.sh`). Passed `woodpecker-cli lint` and the repo's strict shellcheck/yamllint pre-receive hook on push.
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: oleks/ci-scripts#6