X Tutup
The Wayback Machine - https://web.archive.org/web/20260130083607/https://github.com/python/cpython/pull/32178
Skip to content

bpo-47154: make the macos detection of archs more robust #32178

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.

Sign up for GitHub

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

Jump to bottom
Open
isuruf wants to merge 3 commits into
base: main
Choose a base branch
from
Open

bpo-47154: make the macos detection of archs more robust #32178

isuruf wants to merge 3 commits into from
+8 −4

Conversation

@isuruf
Copy link
Contributor

@isuruf isuruf commented Mar 29, 2022 •
edited by bedevere-bot
Loading

@isuruf isuruf requested a review from a team as a code owner March 29, 2022 17:36
ronaldoussoren
ronaldoussoren reviewed Apr 18, 2022
View reviewed changes
Copy link
Contributor

@ronaldoussoren ronaldoussoren left a comment

Choose a reason for hiding this comment

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

The PR looks sane, but...

  • _osx_support contains more processing of command-line flags than the bit changed by this PR (for example: look for _UNIVERSAL_CONFIG_VARS), and those may be affected by this issue of similar ones
  • It might be better to bite the bullet and switch to properly splitting the value using shlex.split instead of using regular expressions, to avoid the (pretty unlikely) scenario of a build that uses a preprocessor define that matches, e.g.:
     CFLAGS=... -DCOMPILER_ARGS="-O3 -arch arm64 -g" ...

@isuruf
Copy link
Contributor Author

isuruf commented Apr 18, 2022

_osx_support contains more processing of command-line flags than the bit changed by this PR (for example: look for _UNIVERSAL_CONFIG_VARS), and those may be affected by this issue of similar ones

I guess you are referring to

cpython/Lib/_osx_support.py

Line 352 in 8c54c3d

flags = re.sub(r'-isysroot\s*\S+(?:\s|$)', ' ', flags)
, right?

@ronaldoussoren
Copy link
Contributor

ronaldoussoren commented Apr 19, 2022

_osx_support contains more processing of command-line flags than the bit changed by this PR (for example: look for _UNIVERSAL_CONFIG_VARS), and those may be affected by this issue of similar ones

I guess you are referring to

cpython/Lib/_osx_support.py

Line 352 in 8c54c3d

flags = re.sub(r'-isysroot\s*\S+(?:\s|$)', ' ', flags)

, right?

That, and

cpython/Lib/_osx_support.py

Line 267 in 8c54c3d

flags = re.sub(r'-arch\s+\w+\s', ' ', flags, flags=re.ASCII)

There may be others as well, I haven't done an exhaustive search yet.

@bendnorman
Copy link

bendnorman commented Jan 24, 2023

Hello! What is the status of the PR? We are running into this issue when trying to install our packages. Thank you!

@jdangerx jdangerx mentioned this pull request Nov 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ronaldoussoren ronaldoussoren ronaldoussoren left review comments

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@isuruf @ronaldoussoren @bendnorman @the-knights-who-say-ni @ezio-melotti @bedevere-bot
X Tutup