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

Fixes #2722 Adds an environment variable SETUPTOOLS_EXT_SUFFIX #2723

Merged
merged 3 commits into from
Jul 19, 2021

Conversation

zooba
Copy link
Contributor

@zooba zooba commented Jul 9, 2021

Adds an environment variable SETUPTOOLS_EXT_SUFFIX to override the suffix inferred from the host Python runtime.

Closes #2722

Pull Request Checklist

@zooba
Copy link
Contributor Author

zooba commented Jul 9, 2021

I went looking for somewhere good to add this to the docs, but nothing stood out. Any suggestions?

@zooba
Copy link
Contributor Author

zooba commented Jul 9, 2021

Not sure whether you normally squash merge these, but please do. Otherwise, I'll merge all the changes together - there's no interesting progression in the commits, just me making mistakes ;)

@zooba
Copy link
Contributor Author

zooba commented Jul 9, 2021

Did my own rebase (to cover up my shame...)

…o override the suffix inferred from the host Python runtime.
filename = os.path.join(*fullname.split('.')) + so_ext
else:
filename = _build_ext.get_ext_filename(self, fullname)
so_ext = get_config_var('EXT_SUFFIX')
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered adding a hook inside get_config_var or get_ext_filename that would address the need more fundamentally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's where I started. Obviously that only works when you also opt in to using the distutils that comes with setuptools, but that's fine.

The bigger issue is that the code later in this function tries to strip off the suffix later, which is not possible to do if get_ext_filename uses anything other than get_condig_var('EXT_SUFFIX') (at least without the same logic as what I added here anyway).

Adding the hook in setuptools._distutils.sysconfig.get_config_var would have made some sense. However, I assume at some point in the future that the patches for build_ext will eventually merge somewhere, and at some point distutils.sysconfig will be replaced with the standard library's sysconfig module. If the hook were in _distutils, there's a pretty good chance it would be forgotten later and the functionality would be lost. This way, there's no chance of it being forgotten in any refactorings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(And yes, the tests would detect the functionality being lost in a refactoring. But tracking down the root cause and handling it - probably just like this - would take up someone's day and I don't want to inflict that on them!)

@jaraco jaraco merged commit f2de5dc into pypa:main Jul 19, 2021
@zooba zooba deleted the ext_suffix branch July 26, 2021 20:43
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.

[FR] Allow environment variable override of EXT_SUFFIX
2 participants