Make sure shellcheck exist

[HeavenTonight]

Signed-off-by: guiyong.ou [email protected]

Fixed #14869
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

[Bhargav-InfraCloud]

I don't think staticcheck will cover for shellcheck. Need to check for shellcheck installation or an alternative.

[ahrtr]

Thanks @Bhargav-InfraCloud for pointing this out. I mis-read the shellcheck as staticcheck. It seems that the shellcheck has never worked in the github workflow? @serathius @ptabor

[ahrtr]

Based on the guide https://github.com/koalaman/shellcheck#installing, it seems that we should install the tool using command sudo apt install shellcheck, but we should install a specified version so as to make the workflow stable. WDYT? @serathius @ptabor

[HeavenTonight]

.PHONY: install-shellcheck install-shellcheck: if ! command -v shellcheck > /dev/null 2>&1; then sudo apt install shellcheck fi

like this,check first,is that ok? @ahrtr

[ahrtr]

We need to install a specific version of shellcheck. sudo apt install shellcheck somehow can only specify version 0.3.7-5. Probably we can download the tool directly something like below,

VERSION=v0.8.0

wget https://github.com/koalaman/shellcheck/releases/download/${VERSION}/shellcheck-${VERSION}.linux.x86_64.tar.xz

[ahrtr]

Please squash the commits into one commit.

[lowryxiao]

If you don't mind, I'd like to share my 2 cents here:
If it's better to add check and download shellcheck logic here if it doesn't exist? that we needn't check twice.
https://github.com/etcd-io/etcd/blob/main/scripts/test.sh#L393
Since the original error comes from the function tool_exists:
https://github.com/etcd-io/etcd/blob/main/scripts/test_lib.sh#L351

As what I can image is like this:

function shellcheck_pass {
  # make sure shellcheck is there, if not, install it first
  if ! tool_exists "shellcheck" "https://github.com/koalaman/shellcheck#installing"; then
    # if shellcheck is not there, we need to install it first
  fi
  # run shellcheck
  generic_checker run shellcheck -fgcc scripts/*.sh
  # return $? of above command
  return $?
}

It's better to return corresponding error code of shellcheck to the caller then the caller will take care if there error happens.

[ahrtr]

I agree that we shouldn't change anything of system-wide or user-wide settings. The proposal #14872 (comment) seems like a valid solution to me.

If the command shellcheck already exists, then use it directly, and set SHELLCHECK="shellcheck". Otherwise, download and install it in $(pwd)/bin, and set SHELLCHECK="$(pwd)/bin/shellcheck". Eventually execute command ${SHELLCHECK} something like below,

generic_checker run ${SHELLCHECK}  -fgcc scripts/*.sh

[ahrtr]

Please also add an item rm -rf ./bin/shellcheck* into the target clean in Makefile.

[ahrtr]

LGTM

Thank you @HeavenTonight

[serathius]

Looks reasonable. Personally I would prefer we simplify dev scripting enough to migrate most commands to Makefile.