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

Fixed Issue on Windows+MSYS2 with unsafe handling of CC config variable. #144

Merged
merged 4 commits into from
Jun 5, 2022

Conversation

MilchRatchet
Copy link
Contributor

On some platforms like Windows in combination with MSYS2 this part of code gets executed but the CC config variable does not exist.

I added a check for if the CC config variable was found and moved the code to the only platform case that requires it.

@jaraco
Copy link
Member

jaraco commented Jun 5, 2022

I added a check for if the CC config variable was found and moved the code to the only platform case that requires it.

It seems to me you fixed the problem twice, once by putting a conditional on the CC var and again by moving the check outside the scope where it was missing, thereby creating new logic that's no longer relevant. Especially since this logic is not tested, let's limit the change to the minimum suitable. I'll remove the conditional on the CC var.

@jaraco jaraco merged commit 3f6a9ad into pypa:main Jun 5, 2022
This was referenced Jun 13, 2022
This was referenced Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants