-
Notifications
You must be signed in to change notification settings - Fork 26
Port _guess_toolchain_kwargs_from_python_config away from distutils #28
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
Conversation
| object_names = [ | ||
| oname for oname in make_vars["OBJECT_OBJS"].split() | ||
| if "(" not in oname and ")" not in oname] | ||
|
|
||
| object_suffix = ".{}".format(object_names[0].split(".")[1]) |
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.
This just tries to get the extension for objects on the system, right?
It seems sysconfig has a dumber parse_makefile that doesn't parse multiline variables. This switched to check MODOBJS, which seems to be the only variable in the makefile that's defined on a single line :\
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.
The new code isn't particularly worse than the old code, and it seems to do the job at least on Linux. So, close enough, to me.
codepy/toolchain.py
Outdated
| cflags.append(lib) | ||
|
|
||
| ld, *ldflags = config_vars["LDSHARED"].split() | ||
| so_ext = os.path.splitext(config_vars["EXT_SUFFIX"])[-1] |
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.
The previous version seemed to assume that make_vars["SO"] returned an extension, but at least on my system it doesn't exist at all. config_vars["SO"] exists though (from sysconfig), but that returns the cpython-[arch].so.
It also says that "SO" is deprecated in favor of "EXT_SUFFIX".
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.
Actually, it's .cpython-[arch].so.. was that the intended full suffix here?
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 think that's OK. As long as CPython looks for xyz.cpython-[arch].so for import xyz, that's all that's needed.
32a453b to
3137b14
Compare
|
My biggest worry here is Mac compatibility. Could you add a CI that tries that? (Maybe on |
3137b14 to
d3bff9e
Compare
At least on my system, all these variables seemed to return the same things, so hopefully it doesn't break too much. I also ran the
loopytests and they still pass.