This also reorders the hooks from pre-commit/pre-commit-hooks sothat the overall order of all hooks from all repositories is: lintPython, lint non-Python, non-lint.

Its output is colorized normally, but on CI it is not colorizedwithout this (pre-commit's own output is, but not shellcheck's).

This suppresses ShellCheck SC2016, "Double quote to preventglobbing and word splitting," on the command in the version checkscript that expands $config_opts to build the "-c ..." arguments.It also moves the code repsonsible for getting the latest tag,which this is part of, into a function for that purpose, so it'sclear that building config_opts is specifically for that, and sothat the code is not made harder to read by adding the ShellChecksuppression comment.(The suppression applies only to the immediate next command.)

In the release building script, this changes $1 to "$1", because$1 without quotes means to perform word splitting and globbing(pathname expansion) on the parameter (unless otherwise disabled bythe value of $IFS or "set -f", respectively) and use the result ofdoing so, which isn't the intent of the code.This function is only used from within the script, where it isnot given values that would be changed by these additionalexpansions. So this is mainly about communicating intent.(If in the future it is intended that multiple arguments be usable,then they should be passed as separate arguments to release_with,which should be modified by replacing "$1" with "$@". I have notused "$@" at this time because it is not intuitively obvious thatthe arguments should be to the interpreter, rather than to thebuild module, so I don't think this should be supported unless oruntil a need for it determines that.)

I think this is easier to read, but this is admittedly subjective.This commit also makes the separate change of adjusting commentspacing for consistency within the script. (Two spaces before "#"is not widely regarded as better than one in shell scripting, sounlike Python where PEP-8 recommends that, it would be equally goodto have changed all the other places in the shell scrips to havejust one.)

The script is nonportable to other shells already because of itsuse of trap (and other features, such as implicit assumptions madeabout echo, and the function keyword), which its hashbang alreadyclearly conveys.This uses:- $(<X) in place of $(cat X), to have the shell read the file  directly rather than using cat.- printf -v in one place to set a variable rather than printing.  This eliminates a command substitution that was harder to read.

Because users may have an old version of git without "git switch",init-tests-after-clone.sh should continue to use "git checkout" toattempt to switch to master. But without "--", this suffers fromthe problem that it's ambiguous if master is a branch (so checkoutbehaves like switch) or a path (so checkout behaves like restore).There are two cases where this ambiguity can be a problem. The mostcommon is on a fork with no master branch but also, fortunately, nofile or directory named "master". Then the problem is just theerror message (printed just before the script proceeds to redothe checkout with -b):    error: pathspec 'master' did not match any file(s) known to gitThe real cause of the error is the branch being absent, as happenswhen a fork copies only the main branch and the upstream remote isnot also set up. Adding the "--" improves the error message:    fatal: invalid reference: masterHowever, it is possible, though unlikely, for a file or directorycalled "master" to exist. In that case, if there is also no masterbranch, git discards unstaged changes made to the file or (muchworse!) everywhere in that directory, potentially losing work.This commit adds "--" to the right of "master" so git neverregards it as a path. This is not needed with -b, which is alwaysfollowed by a symbolic name, so I have not added it there.(Note that the command is still imperfect because, for example, inrare cases there could be a master *tag*--and no master branch--inwhich case, as before, HEAD would be detached there and the scriptwould attempt to continue.)

This had been done everywhere except in init-tests-after-clone.sh.

This translates init-tests-after-clone.sh from bash to POSIX sh,changing the hashbang and replacing all bash-specific features, sothat it can be run on any POSIX-compatible ("Bourne style") shell,including but not limited to bash. This allows it to be used evenon systems that don't have any version of bash installed anywhere.Only that script is modified. The other shell scripts make greateruse of (and gain greater benefit from) bash features, and are alsoonly used for building and releasing. In contrast, this script ismeant to be used by most users who clone the repository.

This makes three related changes:- Removes "git fetch --tags" from the instructions in the readme,  because the goal of this command can be achieved in the  init-tests-after-clone.sh script, and because this fetch command,  as written, is probably only achieving that goal in narrow cases.  In clones and fetches, tags on branches are fetched by default,  and the tags needed by the tests are on branches. So the  situations where "git fetch --tags" was helping were (a) when the  remote recently gained the tags, and (b) when the original remote  was cloned in an unusual way, not fetching all tags. In both  cases, the "--tags" option is not what makes that fetch get the  needed tags.- Adds "git fetch --all --tags" to init-tests-after-clone.sh. The  "--all" option causes it to fetch from all remotes, and this is  more significant than "--tags", since the tags needed for testing  are on fetched branches. This achieves what "git fetch --tags"  was achieving, and it also has the benefit of getting tags from  remotes that have been added but not fetched from, as happens  with an upstream remote that was manually added with no further  action. (It also gets branches from those remotes, but if master  is on multiple remotes but at different commits then "git  checkout master" may not be able to pick one. So do this *after*  rather than before that.)- Skips this extra fetch, and also the submodule cloning/updating  step, when running on CI. CI jobs will already have taken care of  cloning the repo with submodules recursively, and fetching all  available tags. In forks without tags, the necessary tags for the  test are not fetched--but the upstream remote is not set up on  CI, so they wouldn't be obtained anyway, not even by refetching  with "--all".

This extracts duplicated code to functions in check-version.sh.One effect is unfortunately that the specific commands being runare less explicitly clear when reading the script. However, smallfuture changes, if accidentally made to one but not the other ineither pair of "git status" commands, would create a much moreconfusing situation. So I think this change is justified overall.

- Because the substitition string after the hyphen is empty,  "${VIRTUAL_ENV:-}" and "${VIRTUAL_ENV-}" have the same effect.  However, the latter, which this changes it to, expresses the  correct idea that the special case being handled is when the  variable is unset: in this case, we expand an empty field rather  than triggering an error due to set -u. When the variable is set  but empty, it already expands to the substitution value, and  including that in the special case with ":" is thus misleading.- Continuing in the vein ofd18d90a (and1e0b3f9), this removes  another explicit newline by adding another echo command to print  the leading blank line before the table.

This adds comments to init-tests-after-clone.sh to explain whateach of the steps is doing that is needed by some of the tests.It also refactors some recently added logic, in a way that lendsitself to greater clarity when combined with these comments.

This is already done in the other shell scripts. Althoughinit-tests-after-clone.sh does not have as many places where a bugcould slip through by an inadvertently nonexistent parameter, itdoes have $answer (and it may have more expansions in the future).

This is a minor improvement to the robustness of the command for"make all", in the face of plausible future target names.- Use [[:alpha:]] instead of [a-z] because, in POSIX BRE and ERE,  whether [a-z] includes capital letters depends on the current  collation order. (The goal here is greater consistency across  systems, and for this it would also be okay to use [[:lower:]].)- Pass -x to the command that filters out the all target itself,  so that the entire name must be "all", because a future target  may contain the substring "all" as part of a larger word.

This seems simpler to me, but I admit it is subjective.

- Add the psf/black-pre-commit-mirror pre-commit hook but have it  just check and report violations rather than fixing them  automatically (to avoid inconsistency with all the other hooks,  and also so that the linting instructions and CI lint workflow  can remain the same and automatically do the black check).- Remove the "black" environment from tox.ini, since it is now part  of the linting done in the "lint" environment. (But README.md  remains the same, as it documented actually auto-formatting with  black, which is still done the same way.)- Add missing "exclude" keys for the shellcheck and black  pre-commit hooks to explicitly avoid traversing into the  submodules.

This splits the pre-commit hook for black into two hooks, bothusing the same repo and id but separately aliased:- black-check, which checks but does not modify files. This only  runs when the manual stage is specified, and it is used by tox  and on CI, with tox.ini and lint.yml modified accordingly.- black-format, which autoformats code. This provides the behavior  most users will expect from a pre-commit hook for black. It runs  automatically along with other hooks. But tox.ini and lint.yml,  in addition to enabling the black-check hook, also explicitly  skip the black-format hook.The effect is that in ordinary development the pre-commit hook forblack does auto-formatting, but that pre-commit continues to beused effectively for running checks in which code should not bechanged.

In the lint workflow, passing extra-args replaced --all-files,rather than keeping it but adding the extra arguments after it.(But --show-diff-on-failure and --color=always were still passed.)

Now that the changes to lint.yml are fixed up, tested, and shownto be capable of revealing formatting errors, the formatting errorI was using for testing (which is from9fa1cee where I hadintroduced it inadvertently) can be fixed.

As on CI and with tox (but not having to build and create and use atox environment).This may not be the best way to do it, since currently theproject's makefiles are otherwise used only for things directlyrelated to building and publishing. However, this seemed like areadily available way to run the moderately complex command thatproduces the same behavior as on CI or with tox, but outside a toxenvironment. It may be possible to improve on this in the future.

Including tox.

This adds BUILDDIR as a variable in the documentation generationmakefile that, along SPHINXOPTS, SPHINXBUILD, and PAPER, defaultsto the usual best value but can be set when invoking make.The main use for choosing a different build output directory is totest building without overwriting or otherwise interfering with anyfiles from a build one may really want to use. tox.ini is thusmodified to pass a path pointing inside its temporary directory asBUILDDIR, so the "html" tox environment now makes no changesoutside the .tox directory. This is thus suitable for being runautomatically, so the "html" environment is now in the envlist.

As well as still checking for Travis, for backward compatibilityand because experience shows that this is safe.The check can be much broader, and would be more accurate, withfewer false negatives. But a false positive can result in localdata loss because the script does hard resets on CI withoutprompting for confirmation. So for now, this just checks $TRAVISand $GITHUB_ACTIONS.Now that GHA is included, the CI workflows no longer need to set$TRAVIS when running the script, so that is removed.

This extends the init script so it tries harder to get versiontags:- As before, locally, "git fetch --all --tags" is always run. (This  also fetches other objects, and the developer experience would be  undesirably inconsistent if that were only sometimes done.)- On CI, run it if version tags are absent. The criterion followed  here and in subsequent steps is that if any existing tag starts  with a digit, or with the letter v followed by a digit, we regard  version tags to be present. This is to balance the benefit of  getting the tags (to make the tests work) against the risk of  creating a very confusing situation in clones of forks that  publish packages or otherwise use their own separate versioning  scheme (especially if those tags later ended up being pushed).- Both locally and on CI, after that, if version tags are absent,  try to copy them from the original gitpython-developers/GitPython  repo, without copying other tags or adding that repo as a remote.  Copy only tags that start with a digit, since v+digit tags aren't  currently used in this project (though forks may use them). This  is a fallback option and it always displays a warning. If it  fails, another message is issued for that. Unexpected failure to  access the repo terminates the script with a failing exit status,  but the absence of version tags in the fallback remote does not,  so CI jobs can continue and reveal which tests fail as a result.On GitHub Actions CI specifically, the Actions syntax for creatinga warning annotation on the workflow is used, but the warning isstill also written to stderr (as otherwise). GHA workflowannotations don't support multi-line warnings, so the message isadjusted into a single line where needed.

In the step output, the warning that produces a workflow annotationis fully readable (and even made to stand out), so it doesn't needto be printed in the plain way as well, which can be reserved forwhen GitHub Actions is not detected.

The tox environments that are not duplicated per Python versionwere set to run on Python 3.9, for consistency with Cygwin, where3.9 is the latest available (through official Cygwin packages), sooutput can be compared between Cygwin and other platforms whiletroubleshooting problems.However, this prevented them from running altogether on systemswhere Python 3.9 was not installed. So I've added all the otherPython versions the project supports as fallback versions for thosetox environments to use, in one of the reasonable precedenceorders, while keeping 3.9 as the first choice for the same reasonsas before.

This changed a while ago but README.md still listed having a mainbranch as a condition for automatic CI linting and testing in afork.

This is probably the *only* way anyone should run that script onWindows, but I don't know of specific bad things that happen if itis run in some other way, such as with WSL bash, aside from messingup line endings, which users are likely to notice anyway.This commit also clarifies the instructions by breaking up anotherparagraph that really represented two separate steps.

Byron left a comment