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

gh-121103: Put free-threaded libraries in lib/python3.14t #121293

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Jul 2, 2024

On POSIX systems, excluding macOS framework installs, the lib directory for the free-threaded build now includes a "t" suffix to avoid conflicts with a co-located default build installation.

On POSIX systems, excluding macOS framework installs, the lib directory
for the free-threaded build now includes a "t" suffix to avoid conflicts
with a co-located default build installation.
@colesbury colesbury marked this pull request as ready for review July 2, 2024 20:42
@colesbury colesbury requested review from encukou, Yhg1s and ned-deily July 2, 2024 20:42
@colesbury
Copy link
Contributor Author

pip, uv, venv, setuptools, seem to work, but I don't entirely know what I'm doing, so I'd appreciate a thorough review.

This also fixes the test_sysconfig failure on Windows.
@hroncok
Copy link
Contributor

hroncok commented Jul 3, 2024

Does this mean that all of the pure Python standard library files would need to be installed in lib/python3.14t
as well? I.e. when I have both the "normal" and the freethreaded Python installed, do I need all the standard library modules twice?

@encukou
Copy link
Member

encukou commented Jul 3, 2024

I don't have very deep knowledge in this area, so I can't easily do a thorough review :(
I'd trust @FFY00 and @hroncok's reviews if they can contribute their time for it.

@erlend-aasland
Copy link
Contributor

Does this mean that all of the pure Python standard library files would need to be installed in lib/python3.14t as well? I.e. when I have both the "normal" and the freethreaded Python installed, do I need all the standard library modules twice?

As far as I can tell, from a quick look at this PR: yes.

@hroncok
Copy link
Contributor

hroncok commented Jul 3, 2024

As far as I can tell, from a quick look at this PR: yes.

I thought so.

The real question is: Is this the outcome we anticipated?

If not, I suggest keeping the stdlib and platstdlib values intact.

@colesbury
Copy link
Contributor Author

Yes, that was the intention. The goal is to separate the installation. We run into a lot of potential problems if we have partially overlapping stdlib files.

@erlend-aasland
Copy link
Contributor

Yes, that was the intention. The goal is to separate the installation. We run into a lot of potential problems if we have partially overlapping stdlib files.

FTR, I totally agree with this.

@colesbury
Copy link
Contributor Author

To expand on my previous comment: if we have kept a common stdlib and platstdlib, the Python files would be shared, but the .so files would not. So I'd be concerned that if someone installed, e.g. 3.13.0 default and 3.13.8 free-threaded to the same prefix, then the .py files and native .so libraries might not match. Keeping things separate in lib/python3.13t vs. lib/python3.13 avoids this problem.

@colesbury
Copy link
Contributor Author

I plan to merge this tomorrow if there are no objections

Copy link
Member

@ned-deily ned-deily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the nit comment, +1. This looks like it does what it promises. We still have a bit more to do before a free-threaded build can be installed to the same prefix as a non-free-threaded build without conflicts, primarily due to duplicate symlinked names in the bin and lib/pkgconfig directories. But this isn't really a new problem as there are similar conflicts when sharing with a debug build prior to this PR. I'd say let's merge this PR now and we can follow up in another PR for which I want to do some more investigation.

Makefile.pre.in Outdated Show resolved Hide resolved
@colesbury colesbury merged commit e8c91d9 into python:main Jul 11, 2024
36 checks passed
@miss-islington-app
Copy link

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@colesbury colesbury deleted the gh-121103-libdir branch July 11, 2024 20:21
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 11, 2024
…thonGH-121293)

On POSIX systems, excluding macOS framework installs, the lib directory
for the free-threaded build now includes a "t" suffix to avoid conflicts
with a co-located default build installation.
(cherry picked from commit e8c91d9)

Co-authored-by: Sam Gross <colesbury@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jul 11, 2024

GH-121631 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 11, 2024
colesbury added a commit that referenced this pull request Jul 11, 2024
…H-121293) (#121631)

On POSIX systems, excluding macOS framework installs, the lib directory
for the free-threaded build now includes a "t" suffix to avoid conflicts
with a co-located default build installation.
(cherry picked from commit e8c91d9)

Co-authored-by: Sam Gross <colesbury@gmail.com>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
…thon#121293)

On POSIX systems, excluding macOS framework installs, the lib directory
for the free-threaded build now includes a "t" suffix to avoid conflicts
with a co-located default build installation.
Comment on lines +397 to +408
if hasattr(sys, 'abiflags') and 't' in sys.abiflags:
abi_thread = 't'
else:
abi_thread = ''
if os.sep == '/':
libdirs = [sys.platlibdir]
if sys.platlibdir != "lib":
libdirs.append("lib")

for libdir in libdirs:
path = os.path.join(prefix, libdir,
f"{implementation}{ver[0]}.{ver[1]}",
f"{implementation}{ver[0]}.{ver[1]}{abi_thread}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colesbury -- would you be able to update the docs for the site module to reflect these changes to how Python finds the site-packages directory?

(The context is I'm currently working on reimplementing Python's logic here for Astral's work-in-progress type checker. I was reading through the site.py source code, and it surprised me to find logic here that wasn't documented!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. I put up a PR to update the docs: #122737

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants