It looks like the script currently expectspython to be present, even though for me it ispython3.

Maybe I am holding it wrong. Should I follow the installation instructions to get a local dev environment, and then try again from there? I might add thattwine is used to perform the release, is this something that should then be added to the venv? What's the standard for that in the python world?

Thanks for your help.

Thanks for elaborating.

I have changed theMakefile topython3 for convenience, right after installing dependencies withpip install -e ".[test]". That seems to have installedbuild for the current user (Collecting build (from GitPython==3.1.36) Downloading build-1.0.3-py3-none-any.whl (18 kB)), but it would still not find it when runningpython3 -m build.

In order to runtwine on my system, I already have to set thePATH accordingly. Now I also set thePYTHON_PATH to/Users/byron/Library/Python/3.9/lib/python/site-packages:/Users/byron/Library/Python/3.9/lib which seemingly is wherebuild was installed into, but to no avail.

Maybe a venv installation is best to solve both problems. Could you add a commit that adjusts theREADME.md and/or scripts that would make it work assuming onlyvirtualenv? That at least I can execute successfully.

Thank you 😅

Could you add a commit that adjusts theREADME.md and/or scripts that would make it work assuming onlyvirtualenv?

Good idea. I have now done this in5343aa0.

There are various approaches that could be taken for this. The approach I picked was:

• Havemake release/make force_release check if a virtual environment is active, and install thebuild andtwine packages in it if so. (The environment does not need to have the project or any dependencies installed in it, though a project virtual environment can be reused for this purpose.)
• Update the publishing instructions inREADME.md with the optional step of creating and activating a virtual environment, and to mention that doing so causes themake release step to automatically installbuild andtwine.
• Removebuild fromtest-requirements.txt, since no tests currently use it andmake now automates its installation.

I have tested this (as best as I can) on Ubuntu with Python 3.11, in virtual environments created withvirtualenv, as well as withpython -m venv as I am more accustomed to using. As expected, both worked, within the limits of my testing. Besides running various commands manually, I testedmake force_release (and pressedCtrl+C whentwine prompted me for credentials).

I havekept the change ofpython topython3 that you made inf86f09e. I considered having it usepython if in a virtual environment andpython3 otherwise, but I don't think that would increase portability yet. I believe--and testing seems to confirm--that the ref checking inmake release is currently nonportable. It passeshead togit whereHEAD would be required on most other *nix systems due to case sensitivity. It also usessort -n with tag names of the formx.y.z (on systems I have tested this on, only thex andy parts are ordered numerically). MakingMakefile portable is something that could be done later. Even though portability is the theme of this PR, I think it's better to defer that, providedmake release is working at least on macOS.

The installation instructions and release instructions link to two different pages on virtual environments. This is intentional, but I'd be pleased to change if it you prefer. Becausevenv is more common thanvirtualenv and more widely recommended today, and to avoid overwhelming developers less familiar with virtual environments with choices they may not be interested in, I have retained the link tovenv-only documentation that I previously added to the installation instructions. But in the new step of the release instructions, I linked to another (also official) page that talks both aboutvenv andvirtualenv.

Thank you so much! The trick really was to runpython3 -m venv env to get a virtual-env, and then installing any package actually works.

I just created a new release which, in hindsight, I was unnecessary as it doesn't contain user-facing changes.

Regardingmake release, I haven't been using this make target in a long time as it didn't really work and I could just usemake force_release while ideally not forgetting to create a tag. Automating this further isn't needed, I believe, but won't hurt either. In theory, a python project about git should be able to use itself to deal with tags and tag creation, I'd think 😁.

I just created a new release which, in hindsight, I was unnecessary as it doesn't contain user-facing changes.

I'm not sure if this should've had a release or not.

Starting in 3.1.36, it is possible to runpip install "GitPython[test]", which downloads and installs a GitPython wheel from PyPI, along with its test dependencies.Users should not actually do this, because thetest extra is not useful to use in that particular way, but it is possible. Since thetest extra is only useful for development, I think it makes sense that it is documented only for that, and referenced only indirectly in the release description.

When I sawd99b2d4, I thought to ask if it should be apost release instead. But I realized that, due to the new extra, that should probably not be done (since I think it is best for post releases to carry zero installed difference, even in situations that are unusual or not recommended).

On the other hand, it might've been best to avoid it, because it's not necessary to upgrade to it, and because the resulting downstream 3.1.36in conda-forge really is identical to the corresponding downstream 3.1.35. I don't know what the best practice is in this situation.

Regardingmake release, I haven't been using this make target in a long time as it didn't really work and I could just usemake force_release while ideally not forgetting to create a tag.

make release usesgit tag | sort -nr. Do you happen to know if, on macOS, that sorts according to SemVer, at least for the tag names that actually exist in this repository? On my Ubuntu system, it lists2.1.2 above2.1.15 and, more subtly, lists0.3.2-RC1 above0.3.2 and lists0.1.4-pre above0.1.4. Because that's supposed to be later releases above earlier releases (as-r reverses the order), this behavior is not correct.

On GNU/Linux systems including my Ubuntu system,sort is GNU sort, which supports-V/--version-sort. When used in place of-n, this does the right thing with2.1.2/2.1.15, but still not with0.3.2-RC1/0.3.2 or0.1.4-pre/0.1.4. This is the same problem the otherwise more elegantgit tag --sort=-v:refname has.

Ifgit tag | sort -nr does whatMakefile intends it to do on macOS, then I'd be interested in ways to preserve something like the currently writtenrelease target while making it more portable. If not, I'd be more interested in replacing it with something else.

Replying to#1654 (review):

I find your ability to explain changes downrightsuper-human, along with keeping track of their internal and external dependencies, combining it all in various orthogonal issues along with a PR to fixing all of them, explaining even why this is chosen over alternatives. This is certainly appreciated, as it forces me to follow your train of thought while catching up with python.

Thanks! :) :)

There is still a part of me that simply wants to give you write access tomain and say: You know what's best, go ahead :).

It's an honor to hear that such a thing has even crossed your mind! However, I recommend against doing any such thing, at least at this time. The reason is that I actually have only minimal knowledgeof GitPython. I use GitPythonindirectly--because, as a very popular library, it is a dependency of other projects I use directly, especiallynbdime. As a result, if I did have write access, it would be the current situation, just with extra steps: I would not be comfortable making any unreviewed changes.

That's the origin of my interest in GitPython. I learned ofCVE-2023-40590 (#1635) from security alerts on a couple of my repositories that declarenbdime as a development dependency, so I decided to contribute#1636. Unfortunately, even once amended in#1650, my fix also introducesanos.environ race condition, so maybe itcould be done better. Given that the still-flawed fix (like the CVE it fixes)affects native Windows only, I would feel better experimenting with such improvements once we have...

With that said, I would definitely appreciated if native Windows testing could, one day, be added to CI. The time budget of about 20 minutes seems quite feasible, a bound that is currently set by cygwin, and I would be surprised if native windows manages to be worse.

Yes, this is something I am very much interested in doing. (Though I certainly do not want to stop anybody else who wants to work on it!)

I'm interested in figuring out(a) which, of tests that do not pass for me on Windows,should pass because they fail due to a problem specific to my environment or fork, and,(b) of others that are not already skipped on Windows, if the bugs that break them on native Windowspython can simply be fixed.#1651 is an example of such a bug. I'd rather find and fix more bugs like that--if there are any--than obscure them to get CI to pass.

I'm also hoping to find some way to make it significantly faster than the Cygwin test job (currently) is, even if not nearly as fast as the Ubuntu test jobs. If the tests can be made to run fast enough on native Windows, then a matrix strategy could be used as in the Ubuntu test workflow to test on all Python versions GitPython supports (possibly even as part of that workflow, by adding an OS dimension to the existing matrix, depending on how much Window-specific preparation ends up having to be done). Because each job gets its own runner and the runners operate in parallel, six 20-minute checks are no problem for one push, but could slow things down a lot with many pushes in a short time, due to limits on the number of GitHub Actions runners per user or organization.

If worse comes to worse, there is always the middle ground of testing a smaller number of versions on native Windows but still more than one. Perhaps 3.8 and 3.12.

make release usesgit tag | sort -nr. Do you happen to know if, on macOS, that sorts according to SemVer, at least for the tag names that actually exist in this repository?

macOS Ventura doesn't sort according to SemVer:

❯sort --version2.3-Apple (154)❯git tag| sort -nr3.1.93.1.83.1.73.1.63.1.53.1.43.1.363.1.353.1.343.1.333.1.323.1.313.1.303.1.33.1.293.1.283.1.273.1.263.1.253.1.243.1.233.1.223.1.203.1.23.1.193.1.183.1.173.1.163.1.153.1.143.1.133.1.123.1.113.1.103.1.13.1.03.0.93.0.83.0.73.0.63.0.53.0.43.0.33.0.23.0.13.0.02.1.92.1.82.1.72.1.62.1.52.1.42.1.32.1.22.1.152.1.142.1.132.1.122.1.112.1.102.1.12.1.02.0.92.0.82.0.72.0.62.0.52.0.42.0.32.0.22.0.12.0.01.0.21.0.11.0.00.3.70.3.60.3.50.3.40.3.30.3.2.1-patched0.3.2.10.3.2-RC10.3.20.3.1-beta20.3.1-beta10.3.0-beta20.3.0-beta10.2.0-beta10.1.70.1.60.1.50.1.4-pre0.1.4winerr_show

Thanks for sharing your "origin story" - I start liking CVEs much more now, given they made you aware of this project and caused the first contribution :).
I understand your position regarding becoming a maintainer, and if there is ever a change to that so there is benefit to make you one, just let me know :).

I'd rather find and fix more bugs like that--if there are any--than obscure them to get CI to pass.

If I recall correctly, most test-failures on windows arise from its 'unique' way of handling open files, which is quite incompatible with the way files are handled on Unix systems. That said, by now things might have changed and it's probably worth to at least retry the ignored tests to see if some of them work now.

I also think that having any kind of native windows CI job is orthogonal to making more tests pass - the latter is entirely optional to me and definitely nothing more than 'nice to have' at this point. CI on native python on Windows would be a great step forward already.

[..] but could slow things down a lot with many pushes in a short time, due to limits on the number of GitHub Actions runners per user or organization.

These days there isn't too much CI activity here, but if it was a problem I am willing to cancel some jobs by hand. It's something I do quite regularly ingitoxide where CI takes 30 to 40 minutes.

@hugovk Thanks--I find that very helpful, because it tells mesort -n doesn't work for this on any common systems and should be abandoned.

Based on that, I looked for alternatives tosort -n andsort -V that better support SemVer, and foundversionsort.suffix, which will letgit tag --sort=-v:refname do it well enough:

git -c versionsort.suffix=-alpha -c versionsort.suffix=-beta -c versionsort.suffix=-pre -c versionsort.suffix=-rc -c versionsort.suffix=-RC tag --sort=-v:refname

If git's-c supported a form that allowed it and its operand to be passed as a single command-line argument, then brace expansion (in a shell that has it) could abbreviate that nicely. Unfortunately it doesn't. But:

git$(printf' -c versionsort.suffix=-%s' alpha beta pre rc RC) tag --sort=-v:refname

Then, to also filter out non-version tags (they're at the top now, so must be dropped before| head -n1):

git$(printf' -c versionsort.suffix=-%s' alpha beta pre rc RC) tag -l'[0-9]*' --sort=-v:refname

(Where the[0-9]* is a glob, not a regex, and matches the entire tag name, so it's likegreping for^[0-9].)

@Byron

I understand your position regarding becoming a maintainer, and if there is ever a change to that so there is benefit to make you one, just let me know :).

Thanks! That might make sense if/when I have learned more about the codebase (though I would still likely request reviews on many changes).

I also think that having any kind of native windows CI job is orthogonal to making more tests pass - the latter is entirely optional to me and definitely nothing more than 'nice to have' at this point. CI on native python on Windows would be a great step forward already.

That makes sense. If it's okay for native Windows CI to be set up even with tests failing that are not currently skipped or documented to fail on Windows, then I may be able to do it sooner. I think there's still some stuff I need to figure out, though. I think some of the tests that fail on my Windows system are failing only because not everything is set up properly. Furthermore, none of the changes I've made that needed to be tested on Windows required me to set upgit-daemon, and I have never done so.

Since I believe Windows GitHub Actions runners include fullGit for Windows installations, and Git for Windows does shipgit-daemon.exe in its/mingw64/libexec/git-core directory (where/ is not really the root of a Windows drive, but somewhere deeper, for example that path on my Windows system is reallyC:\Users\ek\scoop\apps\git\2.42.0.2\mingw64\libexec\git-core), any insights from figuring that out locally should carry over.

git $(printf ' -c versionsort.suffix=-%s' alpha beta pre rc RC) tag -l '[0-9]*' --sort=-v:refname

This is absolutely amazing! I knowgit can do a lot, but wasn't aware of this configuration option.

That makes sense. If it's okay for native Windows CI to be set up even with tests failing that are not currently skipped or documented to fail on Windows, then I may be able to do it sooner.

Yes, absolutely. I think it's better to split the task into CI Setup and Tests themselves. That way, Tests can simply be ignored when found failing at first until the is time to see if they could also work. Additionally, I think it's perfectly alright to only test the borders of the supported version range of Python, and for that test different Windows versions. I know that GHA supports quite some parallelism, but I'd prefer to conserve the compute anyway and optimize for actual coverage. Limiting oneself somewhat when setting up additional jobs seems particularly easy here since this GitPython seems to have been fine without any of it for more than a decade now.