Skip to content

bpo-34689: Prevent sysconfig._parse_makefile from expanding $${variables} #20439

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lepaperwan
Copy link
Contributor

@lepaperwan lepaperwan commented May 26, 2020

As described in [bpo-34689](https://www.bugs.python.org/issue34689), sysconfig._parse_makefile recognizes $${variable} as a variable to expand in the second half of the _parse_makefile function.
This pull-request aims to fix that by splitting strings on $$ before looking for variables in them and restoring the $$ in values that require more processing.
There is no change to the documentation as this is a bug-fix.

https://bugs.python.org/issue34689

Supersedes #9362

The sysconfig._parse_makefile function expanded $${} formatted text
if another variable was present in a value. This change splits values
on $$ before searching for variables to avoid that.
Lib/sysconfig.py Outdated
value = value[:m.start()] + item + after
if "$" in after:
after = value[offset + m.end():]
value = value[:offset + m.start()] + item.replace('$', '$$') + after
Copy link
Contributor

Choose a reason for hiding this comment

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

Please limit lines to 79 characters.

@@ -0,0 +1,2 @@
Split strings prior to parsing in :func:`sysconfig._parse_makefile` so it
Copy link
Contributor

Choose a reason for hiding this comment

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

sysconfig._parse_makefile is not a documented function, so I don't think the func role should be used here.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Lib/sysconfig.py was turned into package. Could you please resolve conflicts?

@bedevere-app
Copy link

bedevere-app bot commented Feb 28, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

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.

6 participants