ZOOKEEPER-3766: Clean up bash scripts

[ctubbsii]

For ZOOKEEPER-3766:

• Export CLASSPATH in scripts and remove -cp $CLASSPATH uses, so that
  java picks up the classpath from the environment, to avoid absurdly
  long command lines in process listings (this should also fix an issue
  with older versions of pgrep/pkill, such as that installed on RHEL8;
  see https://bugzilla.redhat.com/show_bug.cgi?id=1782309 for details)
• Add a new print-classpath command to print the CLASSPATH env
  variable in the form CLASSPATH=$CLASSPATH from the script, so that
  the script's view of the CLASSPATH environment can easily be inspected
  when troubleshooting java processes

Also fix other code quality issues in all bash scripts:

• Add CI checks for shfmt and shellcheck for all bash scripts
• Standardize "shebang" lines
• Standardize license header formatting
• Format using shfmt tool and address all shellcheck problems
• Remove unnecessary quoting and use of curly braces
• Standardize on double square braces to avoid unpredictable use of
  /usr/bin/[ instead of built-in bash braces
• Remove a useless check of $? in bin/zkServer-initialize.sh
• Remove an impossible to reach exit from bin/zkServer-initialize.sh
• Remove an impossible to fail check of $? after nohup is executed in
  bin/zkServer.sh (nohup with a trailing & always returns a zero exit
  code for success, even if the command fails; if it fails it fails
  undetectibly in the background)
• Avoid using sed for regex, and use built-in bash regex instead
• Delete some old scripts (some of which are broken anyway) that don't
  appear to be used anywhere, and don't seem to have any current need
• Bump rat plugin version for https in license and add license to
  .gitattributes to make rat plugin happy again

[ctubbsii]

This is draft because I'm still fixing the scripts outside the bin directory, and also adding CI checks for shfmt and shellcheck, to prevent regressions and further bugs.

I did find an issue that I am not yet sure how to resolve, at

zookeeper/zookeeper-server/src/test/resources/test-github-pr.sh

Line 100 in a39c8d8

defect=${${subject:$position:$length}#ZOOKEEPER-}

That line is broken... you can't have nested variables like that.

I'm wondering if that test-github-pr.sh script should just be deleted (and maybe some of the other files, too, like test-patch.sh) since GitHub Actions and JIRA exist. Those scripts don't really seem to add much value.

[ctubbsii]

I fixed all remaining scripts and updated the commit message to link it to the related JIRA. This is ready for review.

[ctubbsii]

If the scripts are the same in previous branches, this might be possible to be backported. However, it's a lot of work to try to resolve differences, so if the scripts are substantially different in earlier branches, I would not bother backporting. This cleans things up and ensures quality going forward, but it's not critical to backport.

[tisonkun]

May I ask how this change makes a difference?

[ctubbsii]

This makes it clear that the intent is to treat the $JVMFLAGS and $CLIENT_JVMFLAGS scalar variables as whitespace-delimited arrays. shellcheck warns about this because it does not know the author's intent. In order to suppress that shellcheck warning, I made the parsing of the scalar as an array an explicit separate action. shellcheck will still warn about that, but I suppress that with a very narrow exception (see lines 46 and 48).

It is possible to suppress the warning without changing the line.... however, if you do that, the suppression would be far too broad, affecting all the parameters on the java command, and it would hide future programming errors that shellcheck would normally catch. The way I wrote it here keeps a very narrow suppression, so we don't hide future programming errors, but without changing any behavior.

[tisonkun]

Why do we need an extra whitespace between ! and /?

[ctubbsii]

Short answer: it's not necessary, but it's helpful.

It's optional, but it improves readability, and in fact a space was used in the original example for how to use them in an email from Dennis Ritchie discussing the support for it added to the Unix kernel. Additionally, historically, there have been various rumors that a space was required on some versions of the kernel... but that appears to have just been rumors. Nevertheless, some people's beliefs about the space being important persists. Having the space there avoids unnecessary distractions and complaints from those people who are probably wrong, but might bring it up (see https://en.wikipedia.org/wiki/Shebang_(Unix)#History). If you have to standardize on something, you might as well standardize on the variant that is going to cause less conflict, if all else is equal.

So, I think the main reasons it is helpful to have the space are:

1. improved readability and
2. avoiding distracting complaints from people who still believe the historical rumors.

[tisonkun]

Thanks for your contribution! Generally I like the direction.

Add a new classpath command to print the CLASSPATH environment

I don't find this command though. Could you point out the related lines?

[ctubbsii]

@tisonkun Here is the command added to print the classpath. (I previously commented that I might have left it out of this PR, but I found it and deleted that earlier comment, which was incorrect.)

[tisonkun]

LGTM.

Thanks for spending time on cleaning up these scripts.

[ctubbsii]

I have no idea why the cpp tests are failing... I suspect it's completely unrelated to any changes in this PR, which pretty much only touches the bash scripts.