coreos-assembler
coreos-assembler copied to clipboard
shell: Drop most ignores for SC2031
Followup to https://github.com/coreos/coreos-assembler/pull/3068
The original logic bug here was a classic shell script error, and I was really surprised why shellcheck didn't catch it - but instead we've had these ignores for SC2031 in other places.
I didn't fully debug this, but it looks like there's something going wrong in the analysis pass that's specific to the variable name "arch". Changing the variable name lets us omit the shellcheck ignore.
I didn't fully debug this, but it looks like there's something going wrong in the analysis pass that's specific to the variable name "arch".
I'd be really interested in the root cause there since $arch is a pretty common shorthand for $architecture. Maybe a bug in shellcheck?
I think the reason this happens in the case of create_disk.sh at least is that DEFAULT_TERMINAL=$(. "$(dirname "$0")"/cmdlib.sh; echo "$DEFAULT_TERMINAL") line. ShellCheck sees the arch= assignment in cmdlib.sh and the sourcing happens in a subshell so it's correct that "arch was modified in a subshell". In that case, a simpler approach is to move the DEFAULT_TERMINAL declaration higher up to before we declare our own arch variable.
ShellCheck sees the arch= assignment in cmdlib.sh and the sourcing happens in a subshell so it's correct that "arch was modified in a subshell". In that case, a simpler approach is to move the DEFAULT_TERMINAL declaration higher up to before we declare our own arch variable.
Oh wow, yes. Good analysis. But I think the right fix here is to stop reading cmdlib.sh inside create_disk.sh at all, no? We should be passing this as input via the image.json I think.
Although actually AIUI, this whole thing is basically dead code now because we can rely on --platforms-json?
Although actually AIUI, this whole thing is basically dead code now because we can rely on
--platforms-json?
Not until the console changes reach FCOS stable next month.
@cgwalters: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Closing, cause create_disk.sh would go away soon