Fix up checks in Makefile and make them portable

[EliahKagan]

Fixes #1660

In .gitattributes:

• Fix checking out init-tests-after-clone.sh with LF line endings on all systems (this was broken).
• Add Makefile to so it is also checked out with LF line endings on all systems.
  (Windows ports of make will usually tolerate CRLF line endings, but \r or other representations of CR may, and often do, appear in output, giving an appearance of a problem that is hard to dismiss. So it's worth keeping it LF.)

For the force_release target:

• Don't try to upload if building failed ("make [force_]release" attempts upload even if build fails #1660).
• Only suggest using a virtual environment to solve a failure that occurred outside of one.
• In a virtual environment, use python, not python3, for compatibility with Windows.
  (Windows may have a python3 command installed outside the virtual environment, which is the most confusing case, since it gets used even from inside, so it feels like installing packages in the venv had no effect. In contrast, some Debian and derived systems, and some macOS systems, have no python command outside a venv, or it's Python 2. So keep using python3 when no venv is detected.)

For the release target:

• Reference the current commit as HEAD rather than head. HEAD is more portable, because git does not treat names case-insensitively by default on all systems. head works on Windows and I think macOS, but not GNU/Linux or most other Unix-like systems.
• Use git tag --sort=-v:refname with an appropriate versionsort.suffix to order the version tags in the specific SemVer-inspired way this project uses, and a glob to omit the non-version tags. This avoids the correctness problem with the | sort -nr approach, is portable, and is more precise for this project's versions than even nonportable sort -V tweaked for SemVer, because this project has SemVer suffixes like -pre where N-pre sorts before N, but also non-SemVer -patched where N-patched sorts after N.
  (Thanks to @hugovk for investigating what sort -nr was doing on macOS!)
• Decrease duplication of complex git commands and pipelines.
• Write the checks to fail if a command in $( ) fails (by extracting them to assignments), so that if a git command whose output is needed fails, this doesn't cause a check to wrongly succeed.
• Check for VERSION and changes.rst, against each other and also the latest tag name.
• Check for uncommitted changes.
• Do the version/tag/reference agreement check after printing out a table showing all the relevant values, so the specific problems can always be efficiently identified.

The above includes checks for indications that each of the actions listed in the README.md release instructions was done, and is intended to make it so make release is correct, fully usable, and preferable to make force_release in most situations.

I have tested this on Ubuntu and Windows. When testing, I pressed Ctrl+C when twine prompted for credentials. I tested both the release and force_release targets. Some of the manual tests of the release target on Ubuntu can be seen in this original gist from before that review, and this newer gist testing build-release.sh. Although not shown in those gists, I have also tested that the Makefile targets use the scripts correctly since extracting commands to them.

Here's an example of output from make release showing various checks and the new version/tag/ref table and failing with an error due to an inconsistency that should block the release:

(.venv) ek@Glub:~/repos-wsl/GitPython (portable-makefile=)$ make release
rm -rf build/ dist/ .eggs/ .tox/
./check-version.sh
Checking current directory.
Checking that VERSION and doc/source/changes.rst exist and have no uncommitted changes.
Checking that ALL changes are committed.
Gathering current version, latest tag, and current HEAD commit info.
VERSION file   = 3.1.36
changes.rst    = 3.1.36
Latest tag     = 3.1.36
HEAD SHA       = 4b7d7028193d3a03cc20867fa534e21528886f51
Latest tag SHA = 5297f6210f185c9df9c97392ffef4da8562b58ff
./check-version.sh: Check failed. Stopping.
make: *** [Makefile:10: release] Error 1

Here's an example of a failure where it doesn't get anywhere near that far:

(.venv) ek@Glub:~/repos-wsl/GitPython (portable-makefile +=)$ make release
rm -rf build/ dist/ .eggs/ .tox/
./check-version.sh
Checking current directory.
Checking that VERSION and doc/source/changes.rst exist and have no uncommitted changes.
M  VERSION
./check-version.sh: Check failed. Stopping.
make: *** [Makefile:10: release] Error 1

[Byron]

Thanks a lot! I didn't even read all explanation yet as I have a single change request before doing so: Could you place the script section of the release section into an actual shell script? If you see fit, it could take an argument that allows to force the release.

With that, it will be way easier to produce a script that is free of unnecessary noise. It's my recommendation to use set -eu -o pipefail to be able to get rid of && as well.

Thank you.

[hugovk]

As a follow up, I recommend setting up automated deploys to PyPI from GitHub Actions using the new "Trusted Publishers", to increase security and make releasing easier:

• https://github.com/pypa/gh-action-pypi-publish
• https://blog.pypi.org/posts/2023-04-20-introducing-trusted-publishers/

[EliahKagan]

@Byron Good idea. That has a number of advantages, including that shell scripts in proper files can be checked for bugs using static analyzers. I have now made such a change, or something like it--this may not be quite what you want and it can definitely be changed further.

I extracted everything in the release target except make force_release to a shell script check-version.sh. In addition, although you did not specifically request this, I extracted the code in the force_release target from before the twine command to a separate script build-release.sh. For now, this maintains the original separation between the release and force_release targets, so I have not yet at this point done anything like having either script accept a "force" option, which I think is something that would make more sense to do if the Makefile as a whole is replaced by a shell script.

In both scripts, I have gotten rid of most && and \ in preference for set with -e and some other options.

Because make is no longer echoing many commands, I added messages to explain what is going on. I considered using set -x but I believe that would be excessively verbose, and while it could be turned on and off at different points in the script, that would make things more complicated if one later wishes to use it for debugging (or just to get super-verbose output for some other reason).

Because not all failures--especially with test commands--produce output that makes it clear there was an error, I used trap to append a message when check-version.sh fails for any reason. While I was at it, I used trap to append the virtual-env recommendation in build-release.sh when the build fails outside one; this is one way to preserve the failing command's original exit code, which the old || false way did not do.

Maybe the scripts should be moved and/or renamed. I put them in the root of the repository, but it may be best to reduce clutter there. I named them with a .sh suffix because init-tests-after-clone.sh has one, but it may be better to omit the suffix (which traditionally denotes files that are meant to be sourced rather than executed, whereas these scripts should not be sourced).

[Byron]

A huge thanks for this massive improvement! I learned a lot when studying the shell script - absolutely stunning what tricks printf has up its sleeve - mapping of arguments, spacing, multi-line output depending on arguments, very powerful - previously I would have had to do that in a loop.

As always, the attention to detail of this PR and the description are nothing a mere mortal could have produced, and your PRs inspire me to do better myself in the 'explain your reasoning' department, if not for the world, then for future me. I still have the problem that most of that information that I would leave in commit messages is lost the next time the file moves, as git blame stops there, but I hope one day there will be a way to extract all commits that touch a certain section of a file. But I digress.

I have made a few adjustments in case you want to take a look. Something you might find interesting is the $'\n works here' syntax in bash that helps with inserting newlines into echo at will.

Maybe the scripts should be moved and/or renamed. I put them in the root of the repository, but it may be best to reduce clutter there. I named them with a .sh suffix because init-tests-after-clone.sh has one, but it may be better to omit the suffix (which traditionally denotes files that are meant to be sourced rather than executed, whereas these scripts should not be sourced).

I also thought that maybe there would be a better place for them in some directory, but then I could also appreciate that some automation is so easily discoverable. It's probably OK to leave them as is for now. As for the .sh suffix, I am among the people who take the executable bit as indication of whether they are to be sourced or executed, so (at least to me) the intent is communicated clear enough.

Thanks for this awesome PR, it makes making releases so much safer. And of course I will use make release from now on :).

[Byron]

As a follow up, I recommend setting up automated deploys to PyPI from GitHub Actions using the new "Trusted Publishers", to increase security and make releasing easier:

• https://github.com/pypa/gh-action-pypi-publish
• https://blog.pypi.org/posts/2023-04-20-introducing-trusted-publishers/

@hugovk Thanks for the hint! I imagine that this could be a viable next step. The workflow could be that the release is prepared to the point where check-version.sh passes. This implies a correctly named tag which I would sign as usual. Then, on push, said tag, CI could make a release automatically.

When thinking about it, at this point most of the work was already done and running make release locally isn't a problem, with the only 'inconvenience' that I have to type __token__ and then paste a passcode. These few seconds could be saved at the expense of complexity, abstraction, and giving some secrets to the cloud. And I think this is a price too high overall for the little time saved.

However, I truly think this is the way forward once multiple people should be able to make releases without entrusting them with write access to the python packages on pypi.

[EliahKagan]

Something you might find interesting is the $'\n works here' syntax in bash that helps with inserting newlines into echo at will.

In #1693 I took this further in what I would regard to be that direction, using two echo commands since the extra newline is at the edge:

-echo $'\nThe VERSION must be the same in all locations, and so must the HEAD and tag SHA'
+echo
+echo 'The VERSION must be the same in all locations, and so must the HEAD and tag SHA'

However, you might prefer the way with $' ' and \n for compactness. So I understand you may change it back (or request that I do so).

Another way to write this, which personally I do not think is as nice as either $' ' or using two separate echo commands, is:

echo -e '\nThe VERSION must...'

The reason I think the other approaches (including $' ' as you used here) are nicer is that the versatility of echo -e is more powerful than often intended. For example, this expands escape sequences in the value of var, which if untrusted, could cause problems, including giving strange instructions to some terminals that obscure output the user may rely on:

echo -e "2 loaves of bread\n3 eggs\n$var juice\na sponge"

In contrast, $" " exists, but it is unrelated to expanding escape sequences, so if one accidentally uses it, one finds out when the code is exercised.

POSIX permits echo to always, never, or sometimes, behave as though echo -e were used, and some echos have -e as the default behavior, sometimes allowing -E to turn it off. Actually, what POSIX actually permits is either behavior, or an arbitrary idiosyncratic combination of them. POSIX actually forbids -e and -E from being treated in these ways--they are supposed to be printed literally--but implementations widely treat them as options anyway. -n likewise may or may not be supported, though POSIX permits it, while also permitting it to be treated literally (or to have any other meaning).

For these and other reasons, echo is nonportable, but since echo in bash is a builtin, it is okay to use it in bash (so long as one does not write things like ... | xargs echo or find ... -exec echo ..., which use an external echo executable that may behave differently). It is for this reason that, in #1693, I've used printf in the init script, which as part of my proposed changes there is converted to be a POSIX shell script to support more systems. But this is inapplicable to the other scripts, which use bash's echo builtin without problems.

I also thought that maybe there would be a better place for them in some directory, but then I could also appreciate that some automation is so easily discoverable. It's probably OK to leave them as is for now.

I think we could document the Makefile in the readme and this would give sufficient discoverability, if in the future it is found useful to move them. But I have not included any such proposed change in #1693.

As for the .sh suffix, I am among the people who take the executable bit as indication of whether they are to be sourced or executed, so (at least to me) the intent is communicated clear enough.

This actually works remarkably better even than one might expect. The permissions system in Windows is very different from Unix-like systems, and nothing that really corresponds to Unix-style executable bits exists. But when one uses a Unix-style shell on Windows, such as the Git Bash shell, the available Unix-style utilities like ls handle this by guessing the executable bit from the hashbang. When one is not using a Unix-style shell, one typically cannot easily run a shell script, and one definitely cannot source one since it's the wrong language for the shell being used. So things tend to work out.

(This is separate from WSL: a system in WSL fully supports Unix-style permissions, though like any such operating system, it will not support them when using a Windows filesystem. Windows filesystems being used in a Unix-like system are typically--both when accessing files outside WSL from inside it, and when mounting an NTFS drive on an independent Unix-like operating system--mounted in such a way as to treat every file as having executable bits set. Then, the output of commands like ls -l is not illuminating, but also, for other reasons such as line endings, one should typically not work in a Git repository that way. For this reason I have included in #1693 an explicit mention in the readme that Windows users can invoke the init script from Git Bash. This suggestion is independent of actual changes made to that file, except inasmuch as both are with the aim of improving the experience on less well supported platforms.)

When thinking about it, at this point most of the work was already done and running make release locally isn't a problem, with the only 'inconvenience' that I have to type __token__ and then paste a passcode.

Publishing with GitHub Actions could potentially replace some of the logic in these scripts. For example, a workflow could be set up that builds and published to PyPI when a release is made on GitHub, or when a push creates a tag, comes from a PR with a particular label, etc. (If one of those latter triggers is used, the workflow could even create the GitHub release as well, populating it with the default description or with a custom-generated one.)

Here's an example of building and publishing to PyPI when a release is made on GitHub (though that project uses the dependency manager poetry to build the package). This is technically separate from the use of trusted publishing, in that publishing could be done in the workflow by running a twine command supplied with access credentials sourced from a repository-level GitHub Actions secret. However, these days, trusted publishing is increasingly used, which avoids the need to store PyPI access credentials in a CI secret store.

These few seconds could be saved at the expense of complexity, abstraction, and giving some secrets to the cloud.

I think this is strictly speaking true: any user who has the power to make arbitrary changes to the repository can use that power to cause a release to be published to PyPI, when trusted publishing is used. However, no PyPI secret has to be stored in the repository--changes are validated as having originated from the GitHub repository (and more specifically as from a CI job produced from a particular workflow whose file name has been put in via the PyPI web interface while setting it up), without involving separate stored credentials.

[Byron]

Thanks for illuminating what happens to executable bits on Windows under various conditions - I see that it's definitely not portable. I think I am slowly starting to be less ignorant towards Windows, once again.

Regarding publishing through GitHub actions, I see as main benefit that it would allow other maintainers to create valid publishes, and this is what I would want to use it for as well.

[EliahKagan]

When using Windows, I usually use PowerShell rather than a Unix-style shell (as my interactive shell), so ls doesn't show me Unix-style permissions (inferred from hashbangs or otherwise). But git itself can tell me what permissions are recorded in its index, and also allows me to change them. git ls-files -s gives output in a form I find cumbersome, but I use a custom git command that omits the hash column and less interesting mode bits, and shows executable files and symlinks with colorization and indicator sigils (like ls shows filenames when --color=auto and -F/--classify are passed).

Regarding publishing through GitHub actions, I see as main benefit that it would allow other maintainers to create valid publishes, and this is what I would want to use it for as well.

Yes, I agree. However, it may be that the process of building and publishing releases could be simplified in other ways, if and when this is done.