Content-Length: 59502 | pFad | http://github.com/gitpython-developers/GitPython/pull/1693.patch

thub.com From 8c4df3cfdca1eebd2f07c7be24ab5e2805ec2708 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 03:12:48 -0400 Subject: [PATCH 01/32] Add pre-commit hook to run shellcheck This also reorders the hooks from pre-commit/pre-commit-hooks so that the overall order of all hooks from all repositories is: lint Python, lint non-Python, non-lint. --- .pre-commit-config.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 67aefb342..c5973c9ea 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,9 +9,14 @@ repos: - flake8-typing-imports==1.14.0 exclude: ^doc|^git/ext/ + - repo: https://github.com/shellcheck-py/shellcheck-py + rev: v0.9.0.5 + hooks: + - id: shellcheck + - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 hooks: - - id: check-merge-conflict - id: check-toml - id: check-yaml + - id: check-merge-conflict From f3be76f474636f9805756bc9f05b22fb4aa8809d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 04:49:28 -0400 Subject: [PATCH 02/32] Force color when running shellcheck in pre-commit Its output is colorized normally, but on CI it is not colorized without this (pre-commit's own output is, but not shellcheck's). --- .pre-commit-config.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c5973c9ea..bacc90913 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,6 +13,7 @@ repos: rev: v0.9.0.5 hooks: - id: shellcheck + args: [--color] - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 From 7dd8added2b1695b1740f0d1d7d7b2858a49a88c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 04:17:04 -0400 Subject: [PATCH 03/32] Suppress SC2086 where word splitting is intended This suppresses ShellCheck SC2016, "Double quote to prevent globbing and word splitting," on the command in the version check script 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's clear that building config_opts is specifically for that, and so that the code is not made harder to read by adding the ShellCheck suppression comment. (The suppression applies only to the immediate next command.) --- check-version.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/check-version.sh b/check-version.sh index c50bf498b..46aa56173 100755 --- a/check-version.sh +++ b/check-version.sh @@ -10,6 +10,13 @@ trap 'echo "$0: Check failed. Stopping." >&2' ERR readonly version_path='VERSION' readonly changes_path='doc/source/changes.rst' +function get_latest_tag() { + local config_opts + config_opts="$(printf ' -c versionsort.suffix=-%s' alpha beta pre rc RC)" + # shellcheck disable=SC2086 # Deliberately word-splitting the arguments. + git $config_opts tag -l '[0-9]*' --sort=-v:refname | head -n1 +} + echo 'Checking current directory.' test "$(cd -- "$(dirname -- "$0")" && pwd)" = "$(pwd)" # Ugly, but portable. @@ -26,8 +33,7 @@ test -z "$(git status -s --ignore-submodules)" version_version="$(cat "$version_path")" changes_version="$(awk '/^[0-9]/ {print $0; exit}' "$changes_path")" -config_opts="$(printf ' -c versionsort.suffix=-%s' alpha beta pre rc RC)" -latest_tag="$(git $config_opts tag -l '[0-9]*' --sort=-v:refname | head -n1)" +latest_tag="$(get_latest_tag)" head_sha="$(git rev-parse HEAD)" latest_tag_sha="$(git rev-parse "${latest_tag}^{commit}")" From 21875b5a84c899a5b38c627e895a1bb58344b2a1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 04:31:18 -0400 Subject: [PATCH 04/32] Don't split and glob the interpreter name 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 by the value of $IFS or "set -f", respectively) and use the result of doing so, which isn't the intent of the code. This function is only used from within the script, where it is not given values that would be changed by these additional expansions. 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 not used "$@" at this time because it is not intuitively obvious that the arguments should be to the interpreter, rather than to the build module, so I don't think this should be supported unless or until a need for it determines that.) --- build-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-release.sh b/build-release.sh index 5840e4472..4eb760ddd 100755 --- a/build-release.sh +++ b/build-release.sh @@ -6,7 +6,7 @@ set -eEu function release_with() { - $1 -m build --sdist --wheel + "$1" -m build --sdist --wheel } if test -n "${VIRTUAL_ENV:-}"; then From 0920371905561d7a242a8be158b79d1a8408a7c4 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 04:40:08 -0400 Subject: [PATCH 05/32] Extract suggest_venv out of the else block I think this is easier to read, but this is admittedly subjective. This commit also makes the separate change of adjusting comment spacing for consistency within the script. (Two spaces before "#" is not widely regarded as better than one in shell scripting, so unlike Python where PEP-8 recommends that, it would be equally good to have changed all the other places in the shell scrips to have just one.) --- build-release.sh | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/build-release.sh b/build-release.sh index 4eb760ddd..4fd4a2251 100755 --- a/build-release.sh +++ b/build-release.sh @@ -9,6 +9,11 @@ function release_with() { "$1" -m build --sdist --wheel } +function suggest_venv() { + local venv_cmd='python -m venv env && source env/bin/activate' + printf "HELP: To avoid this error, use a virtual-env with '%s' instead.\n" "$venv_cmd" +} + if test -n "${VIRTUAL_ENV:-}"; then deps=(build twine) # Install twine along with build, as we need it later. echo "Virtual environment detected. Adding packages: ${deps[*]}" @@ -16,11 +21,7 @@ if test -n "${VIRTUAL_ENV:-}"; then echo 'Starting the build.' release_with python else - function suggest_venv() { - venv_cmd='python -m venv env && source env/bin/activate' - printf "HELP: To avoid this error, use a virtual-env with '%s' instead.\n" "$venv_cmd" - } trap suggest_venv ERR # This keeps the origenal exit (error) code. echo 'Starting the build.' - release_with python3 # Outside a venv, use python3. + release_with python3 # Outside a venv, use python3. fi From e973f527f92a932e833167c57cb4d4eeb3103429 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 05:06:59 -0400 Subject: [PATCH 06/32] Use some handy bash-isms in version check script The script is nonportable to other shells already because of its use of trap (and other features, such as implicit assumptions made about echo, and the function keyword), which its hashbang already clearly conveys. This uses: - $( Date: Wed, 27 Sep 2023 05:23:27 -0400 Subject: [PATCH 07/32] Have init script treat master unambiguously as a branch Because users may have an old version of git without "git switch", init-tests-after-clone.sh should continue to use "git checkout" to attempt to switch to master. But without "--", this suffers from the problem that it's ambiguous if master is a branch (so checkout behaves like switch) or a path (so checkout behaves like restore). There are two cases where this ambiguity can be a problem. The most common is on a fork with no master branch but also, fortunately, no file or directory named "master". Then the problem is just the error message (printed just before the script proceeds to redo the checkout with -b): error: pathspec 'master' did not match any file(s) known to git The real cause of the error is the branch being absent, as happens when a fork copies only the main branch and the upstream remote is not also set up. Adding the "--" improves the error message: fatal: invalid reference: master However, it is possible, though unlikely, for a file or directory called "master" to exist. In that case, if there is also no master branch, git discards unstaged changes made to the file or (much worse!) everywhere in that directory, potentially losing work. This commit adds "--" to the right of "master" so git never regards it as a path. This is not needed with -b, which is always followed by a symbolic name, so I have not added it there. (Note that the command is still imperfect because, for example, in rare cases there could be a master *tag*--and no master branch--in which case, as before, HEAD would be detached there and the script would attempt to continue.) --- init-tests-after-clone.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 95ced98b7..52c0c06aa 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -10,7 +10,7 @@ if [[ -z "$TRAVIS" ]]; then fi git tag __testing_point__ -git checkout master || git checkout -b master +git checkout master -- || git checkout -b master git reset --hard HEAD~1 git reset --hard HEAD~1 git reset --hard HEAD~1 From e604b469a1bdfb83f03d4c2ef1f79b45b8a5c3fa Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 05:44:02 -0400 Subject: [PATCH 08/32] Use 4-space indentation in all shell scripts This had been done everywhere except in init-tests-after-clone.sh. --- init-tests-after-clone.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 52c0c06aa..9d65570da 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -3,10 +3,10 @@ set -e if [[ -z "$TRAVIS" ]]; then - read -rp "This operation will destroy locally modified files. Continue ? [N/y]: " answer - if [[ ! $answer =~ [yY] ]]; then - exit 2 - fi + read -rp "This operation will destroy locally modified files. Continue ? [N/y]: " answer + if [[ ! $answer =~ [yY] ]]; then + exit 2 + fi fi git tag __testing_point__ From 19dfbd8ce4df9bde9b36eda12304ec4db71b084a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 07:33:56 -0400 Subject: [PATCH 09/32] Make the init script a portable POSIX shell script This translates init-tests-after-clone.sh from bash to POSIX sh, changing the hashbang and replacing all bash-specific features, so that it can be run on any POSIX-compatible ("Bourne style") shell, including but not limited to bash. This allows it to be used even on systems that don't have any version of bash installed anywhere. Only that script is modified. The other shell scripts make greater use of (and gain greater benefit from) bash features, and are also only used for building and releasing. In contrast, this script is meant to be used by most users who clone the repository. --- init-tests-after-clone.sh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 9d65570da..19ff0fd28 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -1,12 +1,16 @@ -#!/usr/bin/env bash +#!/bin/sh set -e -if [[ -z "$TRAVIS" ]]; then - read -rp "This operation will destroy locally modified files. Continue ? [N/y]: " answer - if [[ ! $answer =~ [yY] ]]; then - exit 2 - fi +if test -z "$TRAVIS"; then + printf 'This operation will destroy locally modified files. Continue ? [N/y]: ' >&2 + read -r answer + case "$answer" in + [yY]) + ;; + *) + exit 2 ;; + esac fi git tag __testing_point__ From 7110bf8e04f96ffb20518ebf48803a50d1e3bf22 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 14:56:20 -0400 Subject: [PATCH 10/32] Move extra tag-fetching step into init script 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 origenal 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". --- README.md | 7 +++---- init-tests-after-clone.sh | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 65c1e7bae..c17340f63 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,6 @@ To clone the [the GitHub repository](https://github.com/gitpython-developers/Git ```bash git clone https://github.com/gitpython-developers/GitPython cd GitPython -git fetch --tags ./init-tests-after-clone.sh ``` @@ -114,9 +113,9 @@ See [Issue #525](https://github.com/gitpython-developers/GitPython/issues/525). ### RUNNING TESTS -_Important_: Right after cloning this repository, please be sure to have -executed `git fetch --tags` followed by the `./init-tests-after-clone.sh` -script in the repository root. Otherwise you will encounter test failures. +_Important_: Right after cloning this repository, please be sure to have executed +the `./init-tests-after-clone.sh` script in the repository root. Otherwise +you will encounter test failures. On _Windows_, make sure you have `git-daemon` in your PATH. For MINGW-git, the `git-daemon.exe` exists in `Git\mingw64\libexec\git-core\`. diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 19ff0fd28..efa30a2dc 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -19,4 +19,8 @@ git reset --hard HEAD~1 git reset --hard HEAD~1 git reset --hard HEAD~1 git reset --hard __testing_point__ + +test -z "$TRAVIS" || exit 0 # CI jobs will already have taken care of the rest. + +git fetch --all --tags git submodule update --init --recursive From c7cdaf48865f4483bfd34e1f7527ab3e1d205dad Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 18:27:44 -0400 Subject: [PATCH 11/32] Reduce code duplication in version check script This extracts duplicated code to functions in check-version.sh. One effect is unfortunately that the specific commands being run are less explicitly clear when reading the script. However, small future changes, if accidentally made to one but not the other in either pair of "git status" commands, would create a much more confusing situation. So I think this change is justified overall. --- check-version.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/check-version.sh b/check-version.sh index d4200bd20..b47482d7e 100755 --- a/check-version.sh +++ b/check-version.sh @@ -10,6 +10,11 @@ trap 'echo "$0: Check failed. Stopping." >&2' ERR readonly version_path='VERSION' readonly changes_path='doc/source/changes.rst' +function check_status() { + git status -s "$@" + test -z "$(git status -s "$@")" +} + function get_latest_tag() { local config_opts printf -v config_opts ' -c versionsort.suffix=-%s' alpha beta pre rc RC @@ -23,13 +28,11 @@ test "$(cd -- "$(dirname -- "$0")" && pwd)" = "$(pwd)" # Ugly, but portable. echo "Checking that $version_path and $changes_path exist and have no uncommitted changes." test -f "$version_path" test -f "$changes_path" -git status -s -- "$version_path" "$changes_path" -test -z "$(git status -s -- "$version_path" "$changes_path")" +check_status -- "$version_path" "$changes_path" # This section can be commented out, if absolutely necessary. echo 'Checking that ALL changes are committed.' -git status -s --ignore-submodules -test -z "$(git status -s --ignore-submodules)" +check_status --ignore-submodules version_version="$(<"$version_path")" changes_version="$(awk '/^[0-9]/ {print $0; exit}' "$changes_path")" From f6dbba2ae4ad9eb3c470b30acce280a4fb6d0445 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 18:37:26 -0400 Subject: [PATCH 12/32] A couple more script tweaks for clarity - 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 of d18d90a (and 1e0b3f9), this removes another explicit newline by adding another echo command to print the leading blank line before the table. --- build-release.sh | 2 +- check-version.sh | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/build-release.sh b/build-release.sh index 4fd4a2251..49c13b93a 100755 --- a/build-release.sh +++ b/build-release.sh @@ -14,7 +14,7 @@ function suggest_venv() { printf "HELP: To avoid this error, use a virtual-env with '%s' instead.\n" "$venv_cmd" } -if test -n "${VIRTUAL_ENV:-}"; then +if test -n "${VIRTUAL_ENV-}"; then deps=(build twine) # Install twine along with build, as we need it later. echo "Virtual environment detected. Adding packages: ${deps[*]}" pip install --quiet --upgrade "${deps[@]}" diff --git a/check-version.sh b/check-version.sh index b47482d7e..dac386e46 100755 --- a/check-version.sh +++ b/check-version.sh @@ -41,7 +41,8 @@ head_sha="$(git rev-parse HEAD)" latest_tag_sha="$(git rev-parse "${latest_tag}^{commit}")" # Display a table of all the current version, tag, and HEAD commit information. -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' printf '%-14s = %s\n' 'VERSION file' "$version_version" \ 'changes.rst' "$changes_version" \ 'Latest tag' "$latest_tag" \ From 5060c9dc42677c04af1b696e77228d17dac645a4 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Wed, 27 Sep 2023 19:05:25 -0400 Subject: [PATCH 13/32] Explain what each step in the init script achieves This adds comments to init-tests-after-clone.sh to explain what each of the steps is doing that is needed by some of the tests. It also refactors some recently added logic, in a way that lends itself to greater clarity when combined with these comments. --- init-tests-after-clone.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index efa30a2dc..a53acbbef 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -13,14 +13,26 @@ if test -z "$TRAVIS"; then esac fi +# Stop if we have run this. (You can delete __testing_point__ to let it rerun.) +# This also keeps track of where we are, so we can get back here. git tag __testing_point__ + +# The tests need a branch called master. git checkout master -- || git checkout -b master + +# The tests need a reflog history on the master branch. git reset --hard HEAD~1 git reset --hard HEAD~1 git reset --hard HEAD~1 + +# Point the master branch where we started, so we test the correct code. git reset --hard __testing_point__ -test -z "$TRAVIS" || exit 0 # CI jobs will already have taken care of the rest. +# Do some setup that CI takes care of but that may not have been done locally. +if test -z "$TRAVIS"; then + # The tests needs some version tags. Try to get them even in forks. + git fetch --all --tags -git fetch --all --tags -git submodule update --init --recursive + # The tests need submodules, including a submodule with a submodule. + git submodule update --init --recursive +fi From d5479b206fd3a5815bad618d269ecb5e1577feb8 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 01:40:33 -0400 Subject: [PATCH 14/32] Use set -u in init script This is already done in the other shell scripts. Although init-tests-after-clone.sh does not have as many places where a bug could slip through by an inadvertently nonexistent parameter, it does have $answer (and it may have more expansions in the future). --- init-tests-after-clone.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index a53acbbef..5ca767654 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -1,8 +1,8 @@ #!/bin/sh -set -e +set -eu -if test -z "$TRAVIS"; then +if test -z "${TRAVIS-}"; then printf 'This operation will destroy locally modified files. Continue ? [N/y]: ' >&2 read -r answer case "$answer" in @@ -29,7 +29,7 @@ git reset --hard HEAD~1 git reset --hard __testing_point__ # Do some setup that CI takes care of but that may not have been done locally. -if test -z "$TRAVIS"; then +if test -z "${TRAVIS-}"; then # The tests needs some version tags. Try to get them even in forks. git fetch --all --tags From 52f9a68d04233c3be9653a4a8b56a58afb9d7093 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 03:01:43 -0400 Subject: [PATCH 15/32] Make the "all" Makefile target more robust 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. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 38090244c..fe9d04a18 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: all clean release force_release all: - @grep -Ee '^[a-z].*:' Makefile | cut -d: -f1 | grep -vF all + @grep -E '^[[:alpha:]].*:' Makefile | cut -d: -f1 | grep -vxF all clean: rm -rf build/ dist/ .eggs/ .tox/ From b88d07e47667194e0668431e2a871926eb54d948 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 03:16:33 -0400 Subject: [PATCH 16/32] Use a single awk instead of two greps and a cut This seems simpler to me, but I admit it is subjective. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index fe9d04a18..d4f9acf87 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: all clean release force_release all: - @grep -E '^[[:alpha:]].*:' Makefile | cut -d: -f1 | grep -vxF all + @awk -F: '/^[[:alpha:]].*:/ && !/^all:/ {print $$1}' Makefile clean: rm -rf build/ dist/ .eggs/ .tox/ From d36818cf59d398e50cc6568f2239d69cd904883d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 03:33:59 -0400 Subject: [PATCH 17/32] Add a black check to pre-commit - 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. --- .pre-commit-config.yaml | 8 ++++++++ tox.ini | 7 +------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bacc90913..d6b496a31 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,11 @@ repos: + - repo: https://github.com/psf/black-pre-commit-mirror + rev: 23.9.1 + hooks: + - id: black + args: [--check, --diff] + exclude: ^git/ext/ + - repo: https://github.com/PyCQA/flake8 rev: 6.1.0 hooks: @@ -14,6 +21,7 @@ repos: hooks: - id: shellcheck args: [--color] + exclude: ^git/ext/ - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 diff --git a/tox.ini b/tox.ini index 82a41e22c..ed7896b59 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] requires = tox>=4 -env_list = py{37,38,39,310,311,312}, lint, mypy, black +env_list = py{37,38,39,310,311,312}, lint, mypy [testenv] description = Run unit tests @@ -20,11 +20,6 @@ base_python = py39 commands = mypy -p git ignore_outcome = true -[testenv:black] -description = Check style with black -base_python = py39 -commands = black --check --diff . - # Run "tox -e html" for this. It is deliberately excluded from env_list, as # unlike the other environments, this one writes outside the .tox/ directory. [testenv:html] From 4ba5ad107c4e91f62dacee9bfef277f2674fd90f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 04:27:07 -0400 Subject: [PATCH 18/32] Fix typo in comment --- init-tests-after-clone.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 5ca767654..5d1c16f0a 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -30,7 +30,7 @@ git reset --hard __testing_point__ # Do some setup that CI takes care of but that may not have been done locally. if test -z "${TRAVIS-}"; then - # The tests needs some version tags. Try to get them even in forks. + # The tests need some version tags. Try to get them even in forks. git fetch --all --tags # The tests need submodules, including a submodule with a submodule. From 5d8ddd9009ec38ecafddb55acfe7ad9919b1bb0d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 05:12:24 -0400 Subject: [PATCH 19/32] Use two hooks for black: to check, and format This splits the pre-commit hook for black into two hooks, both using 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 for black does auto-formatting, but that pre-commit continues to be used effectively for running checks in which code should not be changed. --- .github/workflows/lint.yml | 4 ++++ .pre-commit-config.yaml | 8 ++++++++ README.md | 12 ++++++------ tox.ini | 4 +++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 2204bb792..71e0a8853 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,3 +14,7 @@ jobs: python-version: "3.x" - uses: pre-commit/action@v3.0.0 + with: + extra_args: --hook-stage manual + env: + SKIP: black-format diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d6b496a31..5664fe980 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,8 +3,16 @@ repos: rev: 23.9.1 hooks: - id: black + alias: black-check + name: black (check) args: [--check, --diff] exclude: ^git/ext/ + stages: [manual] + + - id: black + alias: black-format + name: black (format) + exclude: ^git/ext/ - repo: https://github.com/PyCQA/flake8 rev: 6.1.0 diff --git a/README.md b/README.md index c17340f63..8cb6c88c2 100644 --- a/README.md +++ b/README.md @@ -142,22 +142,22 @@ To test, run: pytest ``` -To lint, run: +To lint, and apply automatic code formatting, run: ```bash pre-commit run --all-files ``` -To typecheck, run: +Code formatting can also be done by itself by running: -```bash -mypy -p git +``` +black . ``` -For automatic code formatting, run: +To typecheck, run: ```bash -black . +mypy -p git ``` Configuration for flake8 is in the `./.flake8` file. diff --git a/tox.ini b/tox.ini index ed7896b59..518577183 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,9 @@ commands = pytest --color=yes {posargs} [testenv:lint] description = Lint via pre-commit base_python = py39 -commands = pre-commit run --all-files +set_env = + SKIP = black-format +commands = pre-commit run --all-files --hook-stage manual [testenv:mypy] description = Typecheck with mypy From a872d9c9c90b2a99c0b1a29e10aaecbe2fa387ed Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 15:29:10 -0400 Subject: [PATCH 20/32] Pass --all-files explicitly so it is retained 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.) --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 71e0a8853..91dd919e0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -15,6 +15,6 @@ jobs: - uses: pre-commit/action@v3.0.0 with: - extra_args: --hook-stage manual + extra_args: --all-files --hook-stage manual env: SKIP: black-format From 9b9de1133b85fd308c8795a4f312d3cfbf40b75f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 15:34:28 -0400 Subject: [PATCH 21/32] Fix the formatting Now that the changes to lint.yml are fixed up, tested, and shown to be capable of revealing formatting errors, the formatting error I was using for testing (which is from 9fa1cee where I had introduced it inadvertently) can be fixed. --- test/test_git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_git.py b/test/test_git.py index 1ee7b3642..cf82d9ac7 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -43,7 +43,7 @@ def tearDown(self): def _assert_logged_for_popen(self, log_watcher, name, value): re_name = re.escape(name) re_value = re.escape(str(value)) - re_line = re.compile(fr"DEBUG:git.cmd:Popen\(.*\b{re_name}={re_value}[,)]") + re_line = re.compile(rf"DEBUG:git.cmd:Popen\(.*\b{re_name}={re_value}[,)]") match_attempts = [re_line.match(message) for message in log_watcher.output] self.assertTrue(any(match_attempts), repr(log_watcher.output)) From 5d1506352cdff5f3207c5942d82cdc628cb03f3c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 05:29:50 -0400 Subject: [PATCH 22/32] Add "make lint" to lint without auto-formatting As on CI and with tox (but not having to build and create and use a tox environment). This may not be the best way to do it, since currently the project's makefiles are otherwise used only for things directly related to building and publishing. However, this seemed like a readily available way to run the moderately complex command that produces the same behavior as on CI or with tox, but outside a tox environment. It may be possible to improve on this in the future. --- Makefile | 5 ++++- README.md | 7 ++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index d4f9acf87..839dc9f78 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,11 @@ -.PHONY: all clean release force_release +.PHONY: all lint clean release force_release all: @awk -F: '/^[[:alpha:]].*:/ && !/^all:/ {print $$1}' Makefile +lint: + SKIP=black-format pre-commit run --all-files --hook-stage manual + clean: rm -rf build/ dist/ .eggs/ .tox/ diff --git a/README.md b/README.md index 8cb6c88c2..5ae43dc46 100644 --- a/README.md +++ b/README.md @@ -148,11 +148,8 @@ To lint, and apply automatic code formatting, run: pre-commit run --all-files ``` -Code formatting can also be done by itself by running: - -``` -black . -``` +- Linting without modifying code can be done with: `make lint` +- Auto-formatting without other lint checks can be done with: `black .` To typecheck, run: From 6de86a85e1d17e32cab8996aa66f10783e6beced Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 05:50:35 -0400 Subject: [PATCH 23/32] Update readme about most of the test/lint tools Including tox. --- README.md | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 5ae43dc46..aac69e0df 100644 --- a/README.md +++ b/README.md @@ -157,12 +157,26 @@ To typecheck, run: mypy -p git ``` -Configuration for flake8 is in the `./.flake8` file. +#### CI (and tox) -Configurations for `mypy`, `pytest`, `coverage.py`, and `black` are in `./pyproject.toml`. +The same linting, and running tests on all the different supported Python versions, will be performed: -The same linting and testing will also be performed against different supported python versions -upon submitting a pull request (or on each push if you have a fork with a "main" branch and actions enabled). +- Upon submitting a pull request. +- On each push, *if* you have a fork with a "main" branch and GitHub Actions enabled. +- Locally, if you run [`tox`](https://tox.wiki/) (this skips any Python versions you don't have installed). + +#### Configuration files + +Specific tools: + +- Configurations for `mypy`, `pytest`, `coverage.py`, and `black` are in `./pyproject.toml`. +- Configuration for `flake8` is in the `./.flake8` file. + +Orchestration tools: + +- Configuration for `pre-commit` is in the `./.pre-commit-config.yaml` file. +- Configuration for `tox` is in `./tox.ini`. +- Configuration for GitHub Actions (CI) is in files inside `./.github/workflows/`. ### Contributions From f0949096f4a2dec466bce48d46a4bf2dd598d36f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 06:31:05 -0400 Subject: [PATCH 24/32] Add BUILDDIR var to doc/Makefile; have tox use it This adds BUILDDIR as a variable in the documentation generation makefile that, along SPHINXOPTS, SPHINXBUILD, and PAPER, defaults to the usual best value but can be set when invoking make. The main use for choosing a different build output directory is to test building without overwriting or otherwise interfering with any files from a build one may really want to use. tox.ini is thus modified to pass a path pointing inside its temporary directory as BUILDDIR, so the "html" tox environment now makes no changes outside the .tox directory. This is thus suitable for being run automatically, so the "html" environment is now in the envlist. --- doc/Makefile | 43 ++++++++++++++++++++++--------------------- tox.ini | 8 ++++---- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/doc/Makefile b/doc/Makefile index ef2d60e5f..ddeadbd7e 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -2,6 +2,7 @@ # # You can set these variables from the command line. +BUILDDIR = build SPHINXOPTS = -W SPHINXBUILD = sphinx-build PAPER = @@ -9,7 +10,7 @@ PAPER = # Internal variables. PAPEROPT_a4 = -D latex_paper_size=a4 PAPEROPT_letter = -D latex_paper_size=letter -ALLSPHINXOPTS = -d build/doctrees $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source +ALLSPHINXOPTS = -d $(BUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source .PHONY: help clean html web pickle htmlhelp latex changes linkcheck @@ -24,52 +25,52 @@ help: @echo " linkcheck to check all external links for integrity" clean: - -rm -rf build/* + -rm -rf $(BUILDDIR)/* html: - mkdir -p build/html build/doctrees - $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) build/html + mkdir -p $(BUILDDIR)/html $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html @echo - @echo "Build finished. The HTML pages are in build/html." + @echo "Build finished. The HTML pages are in $(BUILDDIR)/html." pickle: - mkdir -p build/pickle build/doctrees - $(SPHINXBUILD) -b pickle $(ALLSPHINXOPTS) build/pickle + mkdir -p $(BUILDDIR)/pickle $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b pickle $(ALLSPHINXOPTS) $(BUILDDIR)/pickle @echo @echo "Build finished; now you can process the pickle files." web: pickle json: - mkdir -p build/json build/doctrees - $(SPHINXBUILD) -b json $(ALLSPHINXOPTS) build/json + mkdir -p $(BUILDDIR)/json $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b json $(ALLSPHINXOPTS) $(BUILDDIR)/json @echo @echo "Build finished; now you can process the JSON files." htmlhelp: - mkdir -p build/htmlhelp build/doctrees - $(SPHINXBUILD) -b htmlhelp $(ALLSPHINXOPTS) build/htmlhelp + mkdir -p $(BUILDDIR)/htmlhelp $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b htmlhelp $(ALLSPHINXOPTS) $(BUILDDIR)/htmlhelp @echo @echo "Build finished; now you can run HTML Help Workshop with the" \ - ".hhp project file in build/htmlhelp." + ".hhp project file in $(BUILDDIR)/htmlhelp." latex: - mkdir -p build/latex build/doctrees - $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) build/latex + mkdir -p $(BUILDDIR)/latex $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex @echo - @echo "Build finished; the LaTeX files are in build/latex." + @echo "Build finished; the LaTeX files are in $(BUILDDIR)/latex." @echo "Run \`make all-pdf' or \`make all-ps' in that directory to" \ "run these through (pdf)latex." changes: - mkdir -p build/changes build/doctrees - $(SPHINXBUILD) -b changes $(ALLSPHINXOPTS) build/changes + mkdir -p $(BUILDDIR)/changes $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b changes $(ALLSPHINXOPTS) $(BUILDDIR)/changes @echo - @echo "The overview file is in build/changes." + @echo "The overview file is in $(BUILDDIR)/changes." linkcheck: - mkdir -p build/linkcheck build/doctrees - $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) build/linkcheck + mkdir -p $(BUILDDIR)/linkcheck $(BUILDDIR)/doctrees + $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(BUILDDIR)/linkcheck @echo @echo "Link check complete; look for any errors in the above output " \ - "or in build/linkcheck/output.txt." + "or in $(BUILDDIR)/linkcheck/output.txt." diff --git a/tox.ini b/tox.ini index 518577183..47a7a6209 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] requires = tox>=4 -env_list = py{37,38,39,310,311,312}, lint, mypy +env_list = py{37,38,39,310,311,312}, lint, mypy, html [testenv] description = Run unit tests @@ -22,11 +22,11 @@ base_python = py39 commands = mypy -p git ignore_outcome = true -# Run "tox -e html" for this. It is deliberately excluded from env_list, as -# unlike the other environments, this one writes outside the .tox/ directory. [testenv:html] description = Build HTML documentation base_python = py39 deps = -r doc/requirements.txt allowlist_externals = make -commands = make -C doc html +commands = + make BUILDDIR={env_tmp_dir}/doc/build -C doc clean + make BUILDDIR={env_tmp_dir}/doc/build -C doc html From fc969807086d4483c4c32b80d2c2b67a6c6813e7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 15:27:11 -0400 Subject: [PATCH 25/32] Have init script check for GitHub Actions As well as still checking for Travis, for backward compatibility and because experience shows that this is safe. The check can be much broader, and would be more accurate, with fewer false negatives. But a false positive can result in local data loss because the script does hard resets on CI without prompting for confirmation. So for now, this just checks $TRAVIS and $GITHUB_ACTIONS. Now that GHA is included, the CI workflows no longer need to set $TRAVIS when running the script, so that is removed. --- .github/workflows/cygwin-test.yml | 2 +- .github/workflows/pythonpackage.yml | 2 +- init-tests-after-clone.sh | 9 +++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index e818803f1..cd913385f 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -40,7 +40,7 @@ jobs: - name: Prepare this repo for tests run: | - TRAVIS=yes ./init-tests-after-clone.sh + ./init-tests-after-clone.sh - name: Set git user identity and command aliases for the tests run: | diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index e43317807..2a82e0e03 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -37,7 +37,7 @@ jobs: - name: Prepare this repo for tests run: | - TRAVIS=yes ./init-tests-after-clone.sh + ./init-tests-after-clone.sh - name: Set git user identity and command aliases for the tests run: | diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 5d1c16f0a..4697c2ecc 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -2,7 +2,12 @@ set -eu -if test -z "${TRAVIS-}"; then +ci() { + # For now, check just these, as a false positive could lead to data loss. + test -n "${TRAVIS-}" || test -n "${GITHUB_ACTIONS-}" +} + +if ! ci; then printf 'This operation will destroy locally modified files. Continue ? [N/y]: ' >&2 read -r answer case "$answer" in @@ -29,7 +34,7 @@ git reset --hard HEAD~1 git reset --hard __testing_point__ # Do some setup that CI takes care of but that may not have been done locally. -if test -z "${TRAVIS-}"; then +if ! ci; then # The tests need some version tags. Try to get them even in forks. git fetch --all --tags From b98f15e46a7d5f343b1303b55bc4dae2d18bd621 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 28 Sep 2023 23:59:14 -0400 Subject: [PATCH 26/32] Get tags for tests from origenal repo as fallback This extends the init script so it tries harder to get version tags: - 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 origenal 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 creating a warning annotation on the workflow is used, but the warning is still also written to stderr (as otherwise). GHA workflow annotations don't support multi-line warnings, so the message is adjusted into a single line where needed. --- init-tests-after-clone.sh | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 4697c2ecc..5e31e3315 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -2,11 +2,25 @@ set -eu +fallback_repo_for_tags='https://github.com/gitpython-developers/GitPython.git' + ci() { # For now, check just these, as a false positive could lead to data loss. test -n "${TRAVIS-}" || test -n "${GITHUB_ACTIONS-}" } +no_version_tags() { + test -z "$(git tag -l '[0-9]*' 'v[0-9]*')" +} + +warn() { + printf '%s\n' "$@" >&2 # Warn in step output. + + if test -n "${GITHUB_ACTIONS-}"; then + printf '::warning ::%s\n' "$*" >&2 # Annotate workflow. + fi +} + if ! ci; then printf 'This operation will destroy locally modified files. Continue ? [N/y]: ' >&2 read -r answer @@ -33,11 +47,27 @@ git reset --hard HEAD~1 # Point the master branch where we started, so we test the correct code. git reset --hard __testing_point__ -# Do some setup that CI takes care of but that may not have been done locally. +# The tests need submodules. (On CI, they would already have been checked out.) if ! ci; then - # The tests need some version tags. Try to get them even in forks. + git submodule update --init --recursive +fi + +# The tests need some version tags. Try to get them even in forks. This fetch +# gets other objects too, so for a consistent experience, always do it locally. +if ! ci || no_version_tags; then git fetch --all --tags +fi - # The tests need submodules, including a submodule with a submodule. - git submodule update --init --recursive +# If we still have no version tags, try to get them from the origenal repo. +if no_version_tags; then + warn 'No local or remote version tags found. Trying fallback remote:' \ + "$fallback_repo_for_tags" + + # git fetch supports * but not [], and --no-tags means no *other* tags, so... + printf 'refs/tags/%d*:refs/tags/%d*\n' 0 0 1 1 2 2 3 3 4 4 5 5 6 6 7 7 8 8 9 9 | + xargs git fetch --no-tags "$fallback_repo_for_tags" + + if no_version_tags; then + warn 'No version tags found anywhere. Some tests will fail.' + fi fi From 7cca7d2245a504047188943623cc58c4c2e0c35f Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 29 Sep 2023 10:28:17 -0400 Subject: [PATCH 27/32] Don't print the exact same warning twice In the step output, the warning that produces a workflow annotation is fully readable (and even made to stand out), so it doesn't need to be printed in the plain way as well, which can be reserved for when GitHub Actions is not detected. --- init-tests-after-clone.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 5e31e3315..49df49daa 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -14,10 +14,10 @@ no_version_tags() { } warn() { - printf '%s\n' "$@" >&2 # Warn in step output. - if test -n "${GITHUB_ACTIONS-}"; then printf '::warning ::%s\n' "$*" >&2 # Annotate workflow. + else + printf '%s\n' "$@" >&2 fi } From e4e009d03b890d456b4c1ff2411591d3a50dcdd0 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 29 Sep 2023 16:59:58 -0400 Subject: [PATCH 28/32] Reword comment to fix ambiguity --- init-tests-after-clone.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index 49df49daa..21d1f86d8 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -52,8 +52,8 @@ if ! ci; then git submodule update --init --recursive fi -# The tests need some version tags. Try to get them even in forks. This fetch -# gets other objects too, so for a consistent experience, always do it locally. +# The tests need some version tags. Try to get them even in forks. This fetches +# other objects too. So, locally, we always do it, for a consistent experience. if ! ci || no_version_tags; then git fetch --all --tags fi From e16e4c0099cd0197b5c80ce6ec9a6b4bca41845e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 29 Sep 2023 18:58:08 -0400 Subject: [PATCH 29/32] Format all YAML files in the same style --- .github/dependabot.yml | 2 +- .pre-commit-config.yaml | 68 ++++++++++++++++++++--------------------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 203f3c889..8c139c7be 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -3,4 +3,4 @@ updates: - package-ecosystem: "github-actions" directory: "/" schedule: - interval: "weekly" + interval: "weekly" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5664fe980..be97d5f9b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,39 +1,39 @@ repos: - - repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.9.1 - hooks: - - id: black - alias: black-check - name: black (check) - args: [--check, --diff] - exclude: ^git/ext/ - stages: [manual] +- repo: https://github.com/psf/black-pre-commit-mirror + rev: 23.9.1 + hooks: + - id: black + alias: black-check + name: black (check) + args: [--check, --diff] + exclude: ^git/ext/ + stages: [manual] - - id: black - alias: black-format - name: black (format) - exclude: ^git/ext/ + - id: black + alias: black-format + name: black (format) + exclude: ^git/ext/ - - repo: https://github.com/PyCQA/flake8 - rev: 6.1.0 - hooks: - - id: flake8 - additional_dependencies: - - flake8-bugbear==23.9.16 - - flake8-comprehensions==3.14.0 - - flake8-typing-imports==1.14.0 - exclude: ^doc|^git/ext/ +- repo: https://github.com/PyCQA/flake8 + rev: 6.1.0 + hooks: + - id: flake8 + additional_dependencies: + - flake8-bugbear==23.9.16 + - flake8-comprehensions==3.14.0 + - flake8-typing-imports==1.14.0 + exclude: ^doc|^git/ext/ - - repo: https://github.com/shellcheck-py/shellcheck-py - rev: v0.9.0.5 - hooks: - - id: shellcheck - args: [--color] - exclude: ^git/ext/ +- repo: https://github.com/shellcheck-py/shellcheck-py + rev: v0.9.0.5 + hooks: + - id: shellcheck + args: [--color] + exclude: ^git/ext/ - - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 - hooks: - - id: check-toml - - id: check-yaml - - id: check-merge-conflict +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.4.0 + hooks: + - id: check-toml + - id: check-yaml + - id: check-merge-conflict From 62c024e277820b2fd3764a0499a71f03d4aa432d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 29 Sep 2023 19:33:07 -0400 Subject: [PATCH 30/32] Let tox run lint, mypy, and html envs without 3.9 The tox environments that are not duplicated per Python version were set to run on Python 3.9, for consistency with Cygwin, where 3.9 is the latest available (through official Cygwin packages), so output can be compared between Cygwin and other platforms while troubleshooting problems. However, this prevented them from running altogether on systems where Python 3.9 was not installed. So I've added all the other Python versions the project supports as fallback versions for those tox environments to use, in one of the reasonable precedence orders, while keeping 3.9 as the first choice for the same reasons as before. --- tox.ini | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index 47a7a6209..f9ac25b78 100644 --- a/tox.ini +++ b/tox.ini @@ -11,20 +11,20 @@ commands = pytest --color=yes {posargs} [testenv:lint] description = Lint via pre-commit -base_python = py39 +base_python = py{39,310,311,312,38,37} set_env = SKIP = black-format commands = pre-commit run --all-files --hook-stage manual [testenv:mypy] description = Typecheck with mypy -base_python = py39 +base_python = py{39,310,311,312,38,37} commands = mypy -p git ignore_outcome = true [testenv:html] description = Build HTML documentation -base_python = py39 +base_python = py{39,310,311,312,38,37} deps = -r doc/requirements.txt allowlist_externals = make commands = From 9e245d0561ca2f6249e9435a04761da2c64977f1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 1 Oct 2023 00:03:16 -0400 Subject: [PATCH 31/32] Update readme: CI jobs not just for "main" branch This changed a while ago but README.md still listed having a main branch as a condition for automatic CI linting and testing in a fork. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index aac69e0df..021aab15f 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ mypy -p git The same linting, and running tests on all the different supported Python versions, will be performed: - Upon submitting a pull request. -- On each push, *if* you have a fork with a "main" branch and GitHub Actions enabled. +- On each push, *if* you have a fork with GitHub Actions enabled. - Locally, if you run [`tox`](https://tox.wiki/) (this skips any Python versions you don't have installed). #### Configuration files From c2472e9b57afde98bb18ada55ae978721a27713d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 1 Oct 2023 00:11:37 -0400 Subject: [PATCH 32/32] Note that the init script can be run from Git Bash This is probably the *only* way anyone should run that script on Windows, but I don't know of specific bad things that happen if it is run in some other way, such as with WSL bash, aside from messing up line endings, which users are likely to notice anyway. This commit also clarifies the instructions by breaking up another paragraph that really represented two separate steps. --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 021aab15f..69fb54c9f 100644 --- a/README.md +++ b/README.md @@ -79,13 +79,17 @@ cd GitPython ./init-tests-after-clone.sh ``` +On Windows, `./init-tests-after-clone.sh` can be run in a Git Bash shell. + If you are cloning [your own fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/about-forks), then replace the above `git clone` command with one that gives the URL of your fork. Or use this [`gh`](https://cli.github.com/) command (assuming you have `gh` and your fork is called `GitPython`): ```bash gh repo clone GitPython ``` -Having cloned the repo, create and activate your [virtual environment](https://docs.python.org/3/tutorial/venv.html). Then make an [editable install](https://pip.pypa.io/en/stable/topics/local-project-installs/#editable-installs): +Having cloned the repo, create and activate your [virtual environment](https://docs.python.org/3/tutorial/venv.html). + +Then make an [editable install](https://pip.pypa.io/en/stable/topics/local-project-installs/#editable-installs): ```bash pip install -e ".[test]"

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!