3.1.38 many new failing tests

[dvzrv]

Hi! I maintain this package for Arch Linux. Many new failing test with 3.1.38:

=========================== short test summary info ============================
FAILED ../../../../dev/test/test_docs.py::Tutorials::test_references_and_objects
FAILED ../../../../dev/test/test_docs.py::Tutorials::test_submodules - IndexE...
FAILED ../../../../dev/test/test_repo.py::TestRepo::test_clone_from_with_path_contains_unicode
FAILED ../../../../dev/test/test_repo.py::TestRepo::test_submodule_update - g...
FAILED ../../../../dev/test/test_repo.py::TestRepo::test_submodules - Asserti...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_add_clone_multi_options_argument
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_add_no_clone_multi_options_argument
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_base_rw - ...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_branch_renames
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_depth - gi...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_git_submodule_compatibility
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_remove_norefs
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_rename - g...
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_root_module
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_update_clone_multi_options_argument
FAILED ../../../../dev/test/test_submodule.py::TestSubmodule::test_update_no_clone_multi_options_argument
==== 16 failed, 468 passed, 42 skipped, 2 deselected, 5 warnings in 47.69s =====

python-gitpython-3.1.38-1-x86_64-check.log

As I neither use this package, nor really know how to fix these tests (or have the time to do so), I'll disable them.

[Byron]

Thanks for reporting.

Is it possible to add Arch Linux to CI here? If you happen to know existing GitHub CI setups, I'd appreciate a pointer so maybe one day this platform can also be tested here.

Maybe this is also related to recent changes though, so CC @EliahKagan .

[EliahKagan]

I'm pretty sure I inadvertently introduced this bug in #1693, when I had init-tests-after-clone.sh refrain from cloning submodules when it detected it was running on CI, giving the current code:

GitPython/init-tests-after-clone.sh

Lines 50 to 53 in 6f765a2

	# The tests need submodules. (On CI, they would already have been checked out.)
	if ! ci; then
	git submodule update --init --recursive
	fi

I reasoned, incorrectly, that this was safe, due to the CI test workflows in this repository (and thus forks, reuploads, etc.) taking care of cloning submodules (this shows pythonpackage.yml, but cygwin-test.yml has the same thing):

GitPython/.github/workflows/pythonpackage.yml

Lines 27 to 30 in 6f765a2

	- uses: actions/checkout@v4
	with:
	fetch-depth: 0
	submodules: recursive

The problem is that checking for CI--even conservatively, as done here--is not the same as checking for CI workflows originating in this repository:

GitPython/init-tests-after-clone.sh

Lines 7 to 10 in 6f765a2

	ci() {
	# For now, check just these, as a false positive could lead to data loss.
	test -n "${TRAVIS-}" || test -n "${GITHUB_ACTIONS-}"
	}

Confusing the two does not cause data loss--any CI will want to skip the interactive prompt--but it does cause brittleness. GitPython's tests can be run on CI through workflows unrelated to the ones we have here, and projects distributing downstream packages of GitPython, including operating systems such as Arch Linux, are examples of where it makes sense to do that.

init-tests-after-clone.sh is intended to be sufficient, after an ordinary (non-shallow) clone, to get a cloned repository to a state where the tests are ready to be run. I think this is both an intended and reasonable assumption from:

GitPython/README.md

Lines 120 to 122 in 6f765a2

	_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.

The need to clone submodules is intentionally not mentioned in README.md, because it is intended that the init script take care of that whenever it is necessary. Therefore, this really is a regression in GitPython's tests--or more precisely in code that supports them--which I introduced in #1693 due to a mistake about what we are in practice checking for when we check for CI.

The fix should be simple: the init script should unconditionally clone submodules. More granular solutions that still avoid cloning submodules on some CI configurations or if they appear already present are possible, but if that is to be done then I suggest deferring it, so the simpler fix can be applied sooner. I've proposed that fix in #1715, tested it (though not with Arch Linux), and included a more comprehensive analysis in the description.

[Byron]

This issue should now be fixed with the latest release. Please let us know if that's not the case here.