Use double quotes consistently for shell scripts#14938
Conversation
| "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ | ||
| "$(printf "${RUFF_BASE_IMG}@sha256:%s " *)" |
There was a problem hiding this comment.
This is where the release failure happened.
| "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ | ||
| "$(printf "${RUFF_BASE_IMG}@sha256:%s " *)" |
| run: | | ||
| # Write and read notes from a file to avoid quoting breaking things | ||
| echo "$ANNOUNCEMENT_BODY" > $RUNNER_TEMP/notes.txt | ||
| echo "$ANNOUNCEMENT_BODY" > "$RUNNER_TEMP/notes.txt" |
There was a problem hiding this comment.
Isn't this file auto generated?
There was a problem hiding this comment.
Yeah, right. Not sure why one is quoted while the other isn't. I'll revert.
| docker buildx imagetools create \ | ||
| $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ | ||
| $(printf '${RUFF_BASE_IMG}@sha256:%s ' *) | ||
| "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ |
There was a problem hiding this comment.
Are the nested double quotes here not an issue because they're inside of a $( ) construct? I would have naively assumed you'd need to backslash-escape them.
There was a problem hiding this comment.
I think no because $( ) should start a new context but I think in this specific case I should revert because of the glob.
|
Is it worth adding shellcheck to our pre-commit config so we can get these issues flagged in CI in the future? https://github.com/koalaman/shellcheck?tab=readme-ov-file#pre-commit |
|
Thanks for fixing -- sorry for the breakage :( |
I don't know if shellcheck can parse workflow files but |
Summary
The release failed (https://github.com/astral-sh/ruff/actions/runs/12298190472/job/34321509636) because the shell script in the Docker release workflow was using single quotes instead of double quotes.
This is related to https://www.shellcheck.net/wiki/SC2016. I found it via
actionlint. Related #14893.I also went ahead and fixed https://www.shellcheck.net/wiki/SC2086 which were raised in a couple of places.