ci: set -u in cirrus.sh to treat unset variables as an error

[jonasnick]

This PR is supposed to prevent accidental misuse of cirrus.sh. Maybe there is a way to check if CC, AR and NM are set within the loop that deals with the other variables, but so far I did not come up with one (that's POSIX shell compliant).

[sipa]

utACK 1c820cd

[hebasto]

ACK 28fa827, I have reviewed the code and it looks OK.

I've compared some CI logs as well. For example, https://api.cirrus-ci.com/v1/task/5602148176625664/logs/test.log vs https://api.cirrus-ci.com/v1/task/4915160440111104/logs/test.log.

[real-or-random]

ACK mod suggestion

[real-or-random]

for loop and without echo -n which is not POSIX (I don't think we really care in CI but so far we tried to stick to POSIX)

Suggested change

	if [ -n "${CC+x}" ]; then
	echo -n "CC=\"${CC}\" "
	fi
	if [ -n "${CFLAGS+x}" ]; then
	echo -n "CFLAGS=\"${CFLAGS}\" "
	fi
	if [ -n "${AR+x}" ]; then
	echo -n "AR=\"${AR}\" "
	fi
	if [ -n "${NM+x}" ]; then
	echo -n "NM=\"${NM}\" "
	fi
	for var in CC CFLAGS AR NM; do
	eval "isset=\${$var+x}"
	if [ -n "$isset" ]; then
	eval "val=\${$var}"
	# shellcheck disable=SC2154
	printf '%s="%s" ' "$var" "$val"
	fi
	done

[hebasto]

Can't find the point where shellcheck is called... Did I miss a script in CI?

[hebasto]

With the suggested loop, why do separate CC, CFLAGS, AR and NM variables from others? A tiny overhead for a check seems an acceptable tradeoff for the succinct code, no?

[real-or-random]

With the suggested loop, why do separate CC, CFLAGS, AR and NM variables from others? A tiny overhead for a check seems an acceptable tradeoff for the succinct code, no?

Oh indeed. If we have a working loop that handles arbitrary variables, we should use it for all variables. Actually this makes me more convinced of my suggestion than I was before (the eval is somehwat ugly but having one piece of code for all env variables is probably a good idea.) @jonasnick What do you think?

Can't find the point where shellcheck is called... Did I miss a script in CI?

Sorry, I called it locally on my machine. :D I've seen it being very useful a few times in Core, and so I thought it doesn't hurt to add the shellcheck comment here (even if maintainers use locally). Perhaps we want to add it in the future. If @jonasnick picks up this suggestion at all, I'll let him decide if the comment is appropriate / consistent without running shellcheck in CI. ;)

[hebasto]

I've seen it being very useful a few times in Core,

Indeed.

[jonasnick]

Thanks @real-or-random, with your snippet I was able to merge these variables with the loop above. I'd keep the shellcheck directive, since I'm using shellcheck locally.

[real-or-random]

ACK 7a74688

[hebasto]

re-ACK 7a74688