Init script presents challenges on some platforms

[EliahKagan]

The script for preparing the repository to be used to run tests, init-tests-after-clone.sh, can be hard to use, or to understand how to use, on some platforms.

Running it on Windows

It is unclear from the script and the current documentation how to run it on Windows. It should typically be invoked from within Git Bash (though another MinGW shell would be okay too). Opening it as a file, including by attempting to run it in PowerShell, may not work at all or may cause it to malfunction. Running it in Cygwin or WSL for the purpose of developing in those environments is fine, but that should be avoided if the goal is to prepare the repository for native Windows development. In particular, if the repository is cloned outside WSL, then it should not usually be operated on with git from within WSL; doing so can create a strange situation with line endings where every file appears to have been changed locally, and may otherwise use the wrong git configuration.

I think adding a note to README.md about how it can be run in Git Bash on Windows is sufficient to fix this. Other remedies are possible and might be of value, too. For example, a .cmd batch file could be created that either is a translation of the script (i.e., does the same things, including accounting for the effect of set -e by ending most commands with || exit /b) or invokes the shell script with Git Bash, and if the script's .sh suffix is dropped and the batch file is named the same but with .cmd, then separate invocation instructions would not be needed (for example, from PowerShell, ./init-tests-after-clone would run both). This may be better than writing a PowerShell script, which might be helpful but where an execution policy may be set that forbids it from running due to its remote origin. However, especially if the new script (batch file or otherwise) were a translation, this imposes the burden of updating both scripts anytime the logic needs to change. In addition, batch files have some subtleties related to what happens when they are deleted or modified while running (this is allowed, but it affects their behavior!), and since the script performs checkouts and resets, that would need to be taken into account.

Running it without bash

The script is not portable to platforms that don't have any version of bash installed anywhere.

Porting it to sh (i.e., using only POSIX shell features and changing the hashbang from #!/usr/bin/env bash to #!/bin/sh) would fix this. I think this is worthwhile, since its use of non-POSIX features is limited and easy to replace. This differs from the other scripts, which gain greater benefit from bash features like trap [edit: this was a misstatement, see this comment for clarification], and which are run far less often and by fewer people and therefore don't need to be as portable.

Manually performing the steps

If someone cannot or is not willing to run the script, it would be useful if they could easily inspect it to understand what each command achieves, so they can perform analogous actions themselves, modified if necessary according to their platform and requirements. Currently that is difficult because the script does not contain comments that explain the purpose of the operations it performs.

Checking it for correctness and portability

It is hard to remain confident that the script runs on the systems and with the interpreters it is believed to run on (currently bash but on any system, but this could change as suggested above), because we are not currently using static analysis to scan for incompatibilities with the language specified in the hashbang and to scan for common scripting problems (including nonportable assumptions about standard commands). Setting up shellcheck to run as a pre-commit hook and on CI would address this.

Although I think this issue is topically coherent, I think the benefit of adding shellcheck (to run automatically) makes it not not map one-to-one into a pull request. Adding shellcheck will benefit the other scripts as well. A suppression (or other code change) will be needed in check-version.sh, where $config_opts is intentionally expanded unquoted to trigger word splitting (and where globbing, which is not desired there, has already been disabled with set -f). Manual examination accompanying the introduction of shellcheck can identify other areas that might be improved. Most importantly, shellcheck is best added before making further shell script changes, so that the changes can be checked, and this applies to other changes to the scripts as well, including those for #1690 (which would also affect the instructions for, or around, running the init script, by simplifying them).

[EliahKagan]

By the way, I've noticed I misstated what is bash-specific about our use of trap. It's not that this command isn't POSIX--it is, and its presence can be relied on--but rather that POSIX does not require set to support -E/-o errtrace, so traps are not inherited by shell functions. Sorry about the confusion!