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

Leftover code in sysconfig.expand_makefile_vars #128978

Closed
picnixz opened this issue Jan 18, 2025 · 5 comments
Closed

Leftover code in sysconfig.expand_makefile_vars #128978

picnixz opened this issue Jan 18, 2025 · 5 comments
Assignees
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@picnixz
Copy link
Member

picnixz commented Jan 18, 2025

Bug report

Bug description:

I think there is some leftover code in sysconfig.expand_makefile_vars (4a53a39):

while True:
m = re.search(_findvar1_rx, s) or re.search(_findvar2_rx, s)
if m:
(beg, end) = m.span()
s = s[0:beg] + vars.get(m.group(1)) + s[end:]
else:
break
return s

The _findvar1_rx and _findvar2_rx variables are not declared at all (they were moved to sysconfig/__main__.py). Since the function is publicly named (but not exported nor documented), I prefer backporting the changes of #110785, namely re-use the patterns as is.

cc @FFY00

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@picnixz picnixz added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 18, 2025
@picnixz picnixz self-assigned this Jan 18, 2025
@picnixz picnixz added 3.13 bugs and security fixes 3.14 new features, bugs and security fixes 3.12 only security fixes labels Jan 18, 2025
@vstinner
Copy link
Member

Is it time to deprecate this untested and undocumented function?

@picnixz
Copy link
Member Author

picnixz commented Jan 20, 2025

Probably. It was introduced in 2000 and fixed in 2016 but nothing else: https://github.com/search?q=repo%3Apython%2Fcpython+expand_makefile_vars&type=commits.

So how about I first fix the NameError, then in a follow-up PR, I deprecate the function (so that I can have a dedicated What's New entry)?

@picnixz
Copy link
Member Author

picnixz commented Jan 20, 2025

Also, distutils or graalpython seems to provide a (working) bundled version of that function so I don't think we need to worry about?

@vstinner
Copy link
Member

So how about I first fix the NameError, then in a follow-up PR, I deprecate the function (so that I can have a dedicated What's New entry)?

Sure, if we decide to deprecate the function, it should be done in separated change.

picnixz added a commit that referenced this issue Jan 20, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 20, 2025
pythonGH-128979)

This fixes a regression introduced by 4a53a39.
(cherry picked from commit df66ff1)

Co-authored-by: Bénédikt Tran <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 20, 2025
pythonGH-128979)

This fixes a regression introduced by 4a53a39.
(cherry picked from commit df66ff1)

Co-authored-by: Bénédikt Tran <[email protected]>
@picnixz picnixz removed the 3.12 only security fixes label Jan 20, 2025
picnixz added a commit that referenced this issue Jan 20, 2025
…s` (GH-128979) (#129065)

gh-128978: Fix a `NameError` in `sysconfig.expand_makefile_vars` (GH-128979)

This fixes a regression introduced by 4a53a39.
(cherry picked from commit df66ff1)

Co-authored-by: Bénédikt Tran <[email protected]>
@picnixz picnixz closed this as completed Jan 20, 2025
@FFY00
Copy link
Member

FFY00 commented Jan 20, 2025

Is it time to deprecate this untested and undocumented function?

It's sort-of tested through sysconfig.get_paths. Edit: It's not. sysconfig.get_paths uses sysconfig._expand_vars.

I am up to rename it to _expand_makefile_vars, and deprecate expand_makefile_vars. Searching Github for usages of this function (https://github.com/search?q=expand_makefile_vars%20language:Python&type=code&l=Python), I only very few unique usages of it in the wild. And for folks that still need it, as a quick fix, they can add a dependency on setuptools and use distutils.sysconfig.expand_makefile_vars instead.

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this issue Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants