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-119132: Update sys.version to identify free-threaded or not. #119134

Merged
merged 13 commits into from
May 18, 2024

Conversation

corona10
Copy link
Member

@corona10 corona10 commented May 17, 2024

@corona10 corona10 requested review from vstinner and hugovk May 17, 2024 23:54
@corona10 corona10 changed the title gh-119132: Update buildinfo to identify free-threaded or not. [WIP] gh-119132: Update buildinfo to identify free-threaded or not. May 17, 2024
@corona10 corona10 marked this pull request as draft May 18, 2024 00:07
@nineteendo
Copy link
Contributor

nineteendo commented May 18, 2024

Could we maybe leave out "default" for the normal build? It doesn't really add value:

Python 3.14.0a0 (heads/main-dirty:31a28cbae0, May 17 2024, 17:30:32) [GCC 14.1.1 20240507 (Red Hat 14.1.1-1)]
Python 3.14.0a0 (heads/main-dirty:31a28cbae0, May 17 2024, 17:30:32, free-threading) [GCC 14.1.1 20240507 (Red Hat 14.1.1-1)]

The tests on GitHub don't include "(default)" either. Maybe even like this:

Python 3.14.0a0 (heads/main-dirty:31a28cbae0, May 17 2024, 17:30:32) [GCC 14.1.1 20240507 (Red Hat 14.1.1-1)]
Python 3.14.0a0 (free-threading) (heads/main-dirty:31a28cbae0, May 17 2024, 17:30:32) [GCC 14.1.1 20240507 (Red Hat 14.1.1-1)]

This could even work for --version, but some people might rely on the format:

Python 3.14.0a0
Python 3.14.0a0 (free-threading)

@nineteendo
Copy link
Contributor

You still need to update this:

cpython/Lib/platform.py

Lines 1156 to 1161 in 81c3130

sys_version_parser = re.compile(
r'([\w.+]+)\s*' # "version<space>"
r'\(#?([^,]+)' # "(#buildno"
r'(?:,\s*([\w ]*)' # ", builddate"
r'(?:,\s*([\w :]*))?)?\)\s*' # ", buildtime)<space>"
r'\[([^\]]+)\]?', re.ASCII) # "[compiler]"

@nineteendo
Copy link
Contributor

Currently this test is failing:

def test_sys_version(self):

Do you have an opinion on not including "default"?

@corona10
Copy link
Member Author

Do you have an opinion on not including "default"?

Hey, I am under testing. Would you like to leave a comment once I convert the PR into the official PR?

@nineteendo
Copy link
Contributor

Old regex:

([\w.+]+)\s*\(#?([^,]+)(?:,\s*([\w ]*)(?:,\s*([\w :]*))?)?\)\s*\[([^\]]+)\]?

New regex:

([\w.+]+)\s*\(#?([^,]+)(?:,\s*([\w ]*)(?:,\s*([\w :]*))?)(?:,\s*(free-threading))?\)*\s*\[([^\]]+)\]?

Unmatched string:

2.4.3 (truncation) 
[GCC]

Lib/platform.py Outdated Show resolved Hide resolved
Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
Lib/platform.py Outdated Show resolved Hide resolved
Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
@corona10
Copy link
Member Author

@nineteendo Thanks for the comment

Still issue (nah I am really bad at regex)

- ('CPython', '2.4.3', '', '', 'truncation) \n', '', 'GCC')
?                                         ----

+ ('CPython', '2.4.3', '', '', 'truncation', '', 'GCC')

@vstinner
Copy link
Member

I proposed a similar idea last August, but it was rejected at that time: #108239

@vstinner
Copy link
Member

I suggest to mention sys.version in the PR title.

The "(...)" part of sys.version is the Git information, I would prefer to not touch it.

If we change sys.version, I would prefer to "add a new field" in sys.version. For example, add free-threading or [free-threading] after the version. Example:

Python 3.14.0a0 free-threading (heads/main:81c3130c51, May 18 2024, 07:29:44) [GCC 14.0.1 20240411 (Red Hat 14.0.1-0)] on linux

The drawback of changing sys.version is that it breaks tooling parsing it in a strict way.

@corona10
Copy link
Member Author

corona10 commented May 18, 2024

Then we need to discuss with @hugovk

@nineteendo
Copy link
Contributor

You accidentally added * after \):

-([\w.+]+)\s*\(#?([^,]+)(?:,\s*([\w ]*)(?:,\s*([\w :]*))?)?(?:,\s*(free-threading))?\)*\s*\[([^\]]+)\]?
+([\w.+]+)\s*\(#?([^,]+)(?:,\s*([\w ]*)(?:,\s*([\w :]*))?)?(?:,\s*(free-threading))?\)\s*\[([^\]]+)\]?

But I would prefer a new field as well.

Lib/platform.py Outdated Show resolved Hide resolved
Modules/getbuildinfo.c Outdated Show resolved Hide resolved
@corona10 corona10 changed the title [WIP] gh-119132: Update buildinfo to identify free-threaded or not. gh-119132: Update sys.version to identify free-threaded or not. May 18, 2024
@corona10
Copy link
Member Author

@Yhg1s What do you think about backporting it into the 3.13 since it will help the free-threading ecosystem?

Lib/platform.py Outdated Show resolved Hide resolved
Python/getversion.c Outdated Show resolved Hide resolved
Lib/platform.py Outdated Show resolved Hide resolved
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, but see my comment on regex.

Co-authored-by: Victor Stinner <vstinner@python.org>
@corona10 corona10 added the needs backport to 3.13 bugs and security fixes label May 18, 2024
@corona10 corona10 enabled auto-merge (squash) May 18, 2024 19:20
@corona10 corona10 merged commit c141d43 into python:main May 18, 2024
37 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 18, 2024
…pythongh-119134)

(cherry picked from commit c141d43)

Co-authored-by: Donghee Na <donghee.na@python.org>
@bedevere-app
Copy link

bedevere-app bot commented May 18, 2024

GH-119153 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 May 18, 2024
corona10 added a commit that referenced this pull request May 18, 2024
gh-119134) (#119153)

gh-119132: Update sys.version to identify free-threaded or not. (gh-119134)
(cherry picked from commit c141d43)

Co-authored-by: Donghee Na <donghee.na@python.org>
@smontanaro
Copy link
Contributor

Sorry, I haven't been paying close attention, but was there a decision not to tweak sys.version_info as well?

>>> sys.version
'3.14.0a0 experimental free-threading build (heads/main:697465ff88e, May 19 2024, 07:40:21) [Clang 15.0.0 (clang-1500.3.9.4)]'
>>> sys.version_info
sys.version_info(major=3, minor=14, micro=0, releaselevel='alpha', serial=0)

It seems that since it's a named tuple there'd be little problem adding a new field which distinguishes the two builds. Something like:

>>> sys.version_info
sys.version_info(major=3, minor=14, micro=0, releaselevel='alpha', serial=0, free_threading=True)

@gpshead
Copy link
Member

gpshead commented May 19, 2024

was there a decision not to tweak sys.version_info as well?

build information is not part of version_info. the version string is meant for humans and includes other detailed things that are also not in version_info such as compiler details.

there's the https://docs.python.org/3.13/library/sys.html#sys._is_gil_enabled may-change-in-the-future API for people wanting to know the current status of the GIL regardless of build type.

@smontanaro
Copy link
Contributor

Thanks. Just checking. As long as there is somewhere to query the status at run-time without parsing a human-readable string.

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.

None yet

7 participants