Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should test with free-threaded interpreter on CI #2005

Closed
EliahKagan opened this issue Feb 20, 2025 · 2 comments · Fixed by #2011
Closed

Should test with free-threaded interpreter on CI #2005

EliahKagan opened this issue Feb 20, 2025 · 2 comments · Fixed by #2011

Comments

@EliahKagan
Copy link
Member

EliahKagan commented Feb 20, 2025

Since Python 3.13, CPython supports building free-threaded interpreters, i.e. builds that support running with no global interpreter lock and mostly using a combination of narrower locks and lock-free synchronization instead. I think we should test with this on CI, if feasible.

Often the reason to do so would be to find concurrency bugs that might be surfaced or exacerbated by running without a GIL. However, for GitPython, that is not my main motivation. Rather, GitPython has observable behavioral changes under some changes to the interpreter's garbage collector implementation. It seems like this sometimes happens even with modest changes to the garbage collector. For example, from #1767, referencing python/cpython#97922:

# This is needed to work around a PermissionError on Windows, resembling others,
# except new in Python 3.12. (*Maybe* this could be due to changes in CPython's
# garbage collector detailed in https://github.com/python/cpython/issues/97922.)
if sys.platform == "win32" and sys.version_info >= (3, 12):
gc.collect()

I expect that differences in GC details arising from the difference between a GIL and free-threaded builds may also have relevant effects. While the need to call gc.collect in the test suite is usually confined to Windows (to force objects' __del__ method to be called, to close files, so the files can be deleted), I think it's valuable to test this on other systems as well.

Actually, I think it may be reasonable to test free-threaded builds only on Linux-based systems, at least initially. (If problems on Windows arise, then it will become more important to test them on Windows to detect regressions.) This is because such builds are easier to set up on CI on such systems, and because if something breaks on free-threaded interpreters that was already, in practice, working very well before, then that should be known.

So I think that while #1990 was certainly a blocker for this, #1955 is not, and that this could therefore be contributed at any time. However, in my opinion, #1955 is still higher priority than this, as are the other portability maintenance issues #2003 and #2004. I don't know if and when I would get to those, and I hope others will be able to help out; in view of that, it could be a while until I get to this, so I'm opening an issue so it is not forgotten.

It may also make sense to wait until the setup-python action gains the ability to install free-threaded builds. Based on actions/setup-python#771, I think that's not available yet, though based on actions/python-versions#319 being merged earlier today, it may be coming soon. However, there are also alternative actions that can install a free-threaded build, and various other installation methods that use prebuilt binaries.

@Byron
Copy link
Member

Byron commented Feb 21, 2025

Thanks for bringing this up.
I also agree that it would be sufficient to start minimal and run the test-suite on threaded python on Linux only until there is a need for more.

EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Mar 7, 2025
See gitpython-developers#2005.

(This may change by removing some of the jobs, or in other ways.)
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Mar 7, 2025
See gitpython-developers#2005.

(This may change by removing some of the jobs, or in other ways.)
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Mar 7, 2025
For now, this omits macOS and Windows from the 3.13t ("threaded")
tests.

The plan in gitpython-developers#2005 is to start without them, and no OS-specific
problems have been identified so far. In particular, in the
previous commit that adds 3.13t without excluding any operating
systems, all tests in the macOS job passed as expected, and the
Windows job had the same failure with the same message as in gitpython-developers#1955,
with no XFAIL changed to XPASS (which, if present, would suggest
GC differences meriting further exploration of 3.13t on Windows).
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Mar 7, 2025
See gitpython-developers#2005. Right now, this does not limit by operating system, but
that is just to verify that there are no OS-specific 3.13t problems
we should know about right now; once that is verified, the macOS
and Windows jobs will be removed (excluded) for the time being.

The 3.13t jobs added here use `Quansight-Labs/setup-python`, not
`actions/setup-python`. The latter also has the ability to use
3.13t since actions/python-versions#319
and actions/setup-python#973 (see also
actions/setup-python#771), but no version
tag includes this feature yet. It can be used by using `@main` or
`@...` where `...` is an OID. The former would risk pulling in
other untested features we're no trying to test with, while the
latter would not be easy to upgrade automatically as what we have
now (we would be deliberately keeping a hash not at any tag that is
already not the latest hash on any branch). In contrast, the
`Quansight-Labs/setup-python` fork adds this feature while staying
up to date with others. When `actions/setup-python` has a release
(stable or prerelease) with this feature, we can switch to it.

This could probably be done with less code duplication by using a
matrix variable for the action to use. Instead, the "Set up Python"
step is split in two, with opposite `if` conditions, so that each
is capable of being recognized and upgraded by Dependabot if a new
major version is released (in case this ends up remaining in place
longer than expected).
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Mar 7, 2025
For now, this omits macOS and Windows from the 3.13t ("threaded")
tests.

The plan in gitpython-developers#2005 is to start without them, and no OS-specific
problems have been identified so far. In particular, in the
previous commit that adds 3.13t without excluding any operating
systems, all tests in the macOS job passed as expected, and the
Windows job had the same failure with the same message as in gitpython-developers#1955,
with no XFAIL changed to XPASS (which, if present, would suggest
GC differences meriting further exploration of 3.13t on Windows).
@EliahKagan
Copy link
Member Author

I've opened #2011 for this.

Byron pushed a commit to EliahKagan/GitPython that referenced this issue Mar 12, 2025
See gitpython-developers#2005. Right now, this does not limit by operating system, but
that is just to verify that there are no OS-specific 3.13t problems
we should know about right now; once that is verified, the macOS
and Windows jobs will be removed (excluded) for the time being.

The 3.13t jobs added here use `Quansight-Labs/setup-python`, not
`actions/setup-python`. The latter also has the ability to use
3.13t since actions/python-versions#319
and actions/setup-python#973 (see also
actions/setup-python#771), but no version
tag includes this feature yet. It can be used by using `@main` or
`@...` where `...` is an OID. The former would risk pulling in
other untested features we're no trying to test with, while the
latter would not be easy to upgrade automatically as what we have
now (we would be deliberately keeping a hash not at any tag that is
already not the latest hash on any branch). In contrast, the
`Quansight-Labs/setup-python` fork adds this feature while staying
up to date with others. When `actions/setup-python` has a release
(stable or prerelease) with this feature, we can switch to it.

This could probably be done with less code duplication by using a
matrix variable for the action to use. Instead, the "Set up Python"
step is split in two, with opposite `if` conditions, so that each
is capable of being recognized and upgraded by Dependabot if a new
major version is released (in case this ends up remaining in place
longer than expected).
Byron pushed a commit to EliahKagan/GitPython that referenced this issue Mar 12, 2025
For now, this omits macOS and Windows from the 3.13t ("threaded")
tests.

The plan in gitpython-developers#2005 is to start without them, and no OS-specific
problems have been identified so far. In particular, in the
previous commit that adds 3.13t without excluding any operating
systems, all tests in the macOS job passed as expected, and the
Windows job had the same failure with the same message as in gitpython-developers#1955,
with no XFAIL changed to XPASS (which, if present, would suggest
GC differences meriting further exploration of 3.13t on Windows).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants