Fix error messages in check-release.sh

[vvv]

• check_tag: Report file name correctly. Use named variables.
• Introduce read_version helper function. Simplify the implementation.
• Show meaningful error message if GITHUB_REF is not set or its format is incorrect.

[irevoire]

Hey @vvv.
There are a lot of good ideas here, but it doesn’t work anymore. Try to run the script before and after your PR, and you’ll see.

On my computer here are the results;

irevoire@macbook-pro-4 ~/clean/MeiliSearch 🌱 % bash .github/scripts/check-release.sh
Error: the current tag does not match the version in Cargo.toml: found  - expected 1.2.0
Error: the current tag does not match the version in Cargo.toml: found Cargo.lock - expected 1.2.0
1 irevoire@macbook-pro-4 ~/clean/MeiliSearch 🍐 % gh pr checkout 3799
Switched to branch 'fix.check-release-script'
irevoire@macbook-pro-4 ~/clean/MeiliSearch 🍴 % bash .github/scripts/check-release.sh
.github/scripts/check-release.sh: line 7: $3: unbound variable

[vvv]

Hey @irevoire,

Thanks for looking at the patch. (Also, nice shell prompt! 🧑🏼‍🌾 Pretty organic.)

irevoire@macbook-pro-4 ~/clean/MeiliSearch 🌱 % bash .github/scripts/check-release.sh
Error: the current tag does not match the version in Cargo.toml: found  - expected 1.2.0
Error: the current tag does not match the version in Cargo.toml: found Cargo.lock - expected 1.2.0
1 irevoire@macbook-pro-4 ~/clean/MeiliSearch 🍐 % gh pr checkout 3799
Switched to branch 'fix.check-release-script'
irevoire@macbook-pro-4 ~/clean/MeiliSearch 🍴 % bash .github/scripts/check-release.sh
.github/scripts/check-release.sh: line 7: $3: unbound variable

check-release.sh is supposed to run in CI environment, where GITHUB_REF variable is set.

When you ran the script on your laptop, GITHUB_REF variable wasn't set, so current_tag got evaluated to an empty string.

current_tag=${GITHUB_REF#'refs/tags/v'}

file_tag="$(cat Cargo.toml | read_version)"
check_tag $current_tag $file_tag Cargo.toml || ret=1

check_tag command doesn't recognize unquoted empty string as a separate argument due to the way parameter expansion works in bash. check_tag sees only 2 arguments instead of 3 and complains:

❯ bash .github/scripts/check-release.sh || echo FAIL $?
.github/scripts/check-release.sh: line 7: $3: unbound variable
FAIL 1

When CI runs the script, GITHUB_REF is set to a string that (supposedly) starts with refs/tags/ prefix. In this case the script performs its duties properly:

# Happy case
❯ GITHUB_REF=refs/tags/v1.2.0 bash .github/scripts/check-release.sh || echo FAIL $?
OK

# Wrong tag
❯ GITHUB_REF=refs/tags/v8.8.8 bash .github/scripts/check-release.sh
Error: the current tag does not match the version in Cargo.toml: found 1.2.0, expected 8.8.8
Error: the current tag does not match the version in Cargo.lock: found 1.2.0, expected 8.8.8

# Invalid tag
❯ GITHUB_REF=refs/tags/bad-tag bash .github/scripts/check-release.sh
Error: the current tag does not match the version in Cargo.toml: found 1.2.0, expected refs/tags/bad-tag
Error: the current tag does not match the version in Cargo.lock: found 1.2.0, expected refs/tags/bad-tag

# Not a tag
❯ GITHUB_REF=refs/heads/main bash .github/scripts/check-release.sh
Error: the current tag does not match the version in Cargo.toml: found 1.2.0, expected refs/heads/main
Error: the current tag does not match the version in Cargo.lock: found 1.2.0, expected refs/heads/main

Still, the UX is bad. Let me improve error reporting...

[curquiza]

Hello @vvv
Sorry for the delay, but @irevoire is sick and I rely on his review. So, merging PR can takes time but we don't forget you!

[vvv]

Sorry for the delay, but @irevoire is sick and I rely on his review. So, merging PR can takes time but we don't forget you!

@curquiza Sure, no worries Thanks for reaching out to me. 😊

@irevoire Get well soon!

[irevoire]

That looks good to me. @curquiza I don’t know how to test it on our CI though 😬

[curquiza]

Hello

Thanks for your PR @vvv

Th change you made for the current script does not work as expected.

Our tags start with v, and if I try to do the following commands, I have an error:

bash-3.2$ export GITHUB_REF=v1.2.0
bash-3.2$ bash .github/scripts/check-release.sh
Error: GITHUB_REF is not a valid tag: v1.2.0

[vvv]

@curquiza GitHub adds refs/tags/ prefix to the value of GITHUB_REF.

❯ GITHUB_REF=refs/tags/v1.2.0 bash .github/scripts/check-release.sh
OK

See https://docs.github.com/en/actions/learn-github-actions/variables :

The ref given is fully-formed, meaning that for branches the format is refs/heads/<branch_name>, for pull requests it is refs/pull/<pr_number>/merge, and for tags it is refs/tags/<tag_name>.

[vvv]

vvv dismissed irevoire’s stale review via 685422a

I didn't mean to dismiss the review. I guess the repository is configured to auto-dismiss the review once the PR's branch is updated.

If both required reviews and stale review dismissal are enabled and a code-modifying commit is pushed to the branch of an approved pull request, the approval is dismissed. The pull request must be reviewed and approved again before it can be merged.

[curquiza]

I didn't mean to dismiss the review. I guess the repository is configured to auto-dismiss the review once the PR's branch is updated.

Yes don't worry I know 😄

@curquiza GitHub adds refs/tags/ prefix to the value of GITHUB_REF.

Oh sorry @vvv, my bad, the worst part is I read what you wrote last week 🙈 I'm not a morning person definitely

@vvv
Can you remove your last commit? It's not useful for us to add this permission, I really want to avoid useless changes in the repository 😊 We will add it if we need it

[vvv]

@curquiza Sure. I've removed the chmod +x commit and rebased the branch onto the latest master.

[curquiza]

thank you @vvv 🙏 for your work, reactivity and involvement

bors merge

[meili-bors]

Build succeeded:

• Run Clippy
• Run Rustfmt
• Run tests in debug
• Tests on macos-12
• Tests on ubuntu-18.04
• Tests on windows-2022