-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Pypy fixes #7839
Pypy fixes #7839
Conversation
@@ -126,8 +126,10 @@ def get_flags_linker_so(self): | |||
# from it. | |||
import distutils.sysconfig as sc | |||
g = {} | |||
filename = sc.get_makefile_filename() | |||
sc.parse_makefile(filename, g) | |||
if '__pypy__' not in sys.builtin_module.names: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like a try/except might be more appropriate than an explicit pypy check? That way we're robust against pypy adding a makefile, or being used with some other python implementation that also doesn't have a makefile.
Please squash the commits and follow the commit message template in |
filename = sc.get_makefile_filename() | ||
sc.parse_makefile(filename, g) | ||
except AttributeError as e: | ||
if 'get_makefile_filename' in e.message: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of checking the error message, would using an else
block work? e.g.,
try:
get_filename = sc.get_makefile_filename
except AttributeError:
pass # no Makefile
else:
sc.parsefile(get_filename(), g)
It's generally a good idea to contain no more than the single line at which the exception can occur in the try
block.
Updated as per shoyer's comment |
LGTM, thanks @mattip |
two simple fixes for PyPy: