diff --git a/.github/bump-version.get.sh b/.github/bump-version.get.sh index 6e6ef07..d326e4b 100755 --- a/.github/bump-version.get.sh +++ b/.github/bump-version.get.sh @@ -28,6 +28,8 @@ if [ -z "${VERSFILES}" ] ; then fi # Read the versions. +CARGOERR=$(mktemp) +trap 'rm -f "${CARGOERR}"' EXIT CURRENTVERS="" for FILE in ${VERSFILES} ; do # Parse each version file according to its type. @@ -40,12 +42,20 @@ for FILE in ${VERSFILES} ; do # read-manifest reports only the package this manifest defines. `cargo metadata` resolves the # entire workspace no matter which member it's pointed at and orders packages alphabetically, # so it returned whichever member sorted first instead of the file we're reading. - if ! MANIFEST_JSON=$(cargo read-manifest --manifest-path "${FILE}" 2>&1) ; then - # A virtual manifest defines no package, so it has no version to contribute. - if [[ ${MANIFEST_JSON} == *"is a virtual manifest"* ]] ; then - continue - fi - echo "${MANIFEST_JSON}" 1>&2 + # Stderr goes to a file rather than into the captured stdout: when the repo pins a toolchain + # that isn't installed yet, the rustup shim writes "info: ..." progress lines there, and + # merging them with 2>&1 handed them to jq as if they were the manifest. + CARGOSTATUS=0 + MANIFEST_JSON=$(cargo read-manifest --manifest-path "${FILE}" 2>"${CARGOERR}") || CARGOSTATUS=$? + # A virtual manifest defines no package, so it has no version to contribute. We expect that, + # so swallow the complaint cargo makes about it. + if [ "${CARGOSTATUS}" -ne 0 ] && grep -q "is a virtual manifest" "${CARGOERR}" ; then + continue + fi + # The file is only a holding pen to keep stderr away from jq. Anything else cargo had to say + # still reaches the log, whether or not it succeeded. + cat "${CARGOERR}" 1>&2 + if [ "${CARGOSTATUS}" -ne 0 ] ; then exit 1 fi VERS=$(echo "${MANIFEST_JSON}" | jq -re '.version') diff --git a/.github/spec/get_spec.sh b/.github/spec/get_spec.sh index ae02e6f..95fc97b 100644 --- a/.github/spec/get_spec.sh +++ b/.github/spec/get_spec.sh @@ -6,9 +6,24 @@ Describe 'bump-version.get.sh' setup_repo() { REPO=$(mktemp -d) mkdir -p "${REPO}/.git" + ORIGPATH="${PATH}" } cleanup_repo() { rm -rf "${REPO}" + PATH="${ORIGPATH}" + } + + # Stands in for the rustup shim, which announces itself on stderr while it installs a pinned + # toolchain before handing off to the real cargo. + stub_noisy_cargo() { + mkdir -p "${REPO}/.stub-bin" + { + printf '#!/bin/sh\n' + printf "echo \"info: syncing channel updates for '1.93.1-x86_64-unknown-linux-gnu'\" 1>&2\n" + printf 'exec %s "$@"\n' "$(command -v cargo)" + } > "${REPO}/.stub-bin/cargo" + chmod +x "${REPO}/.stub-bin/cargo" + PATH="${REPO}/.stub-bin:${PATH}" } write_crate() { @@ -70,4 +85,24 @@ Describe 'bump-version.get.sh' The output should equal "4.5.6-pre" The status should be success End + + # Regression: cargo's stderr was folded into its stdout with 2>&1, so any chatter ahead of the + # JSON reached jq and it died with "Invalid numeric literal". Keeping the two apart must not cost + # us the chatter itself, which is worth having in the log. + It 'keeps cargo chatter out of the parsed json but still logs it' + write_crate . solo 9.8.7-pre + stub_noisy_cargo + When call run_get + The output should equal "9.8.7-pre" + The stderr should include "syncing channel updates" + The status should be success + End + + It 'still reports cargo errors that are not a virtual manifest' + printf 'this is not valid toml\n' > "${REPO}/Cargo.toml" + When call run_get + The output should equal "" + The stderr should not equal "" + The status should be failure + End End