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

Setting SETUPTOOLS_USE_DISTUTILS in setup.py? #584

Open
jakirkham opened this issue Nov 5, 2021 · 7 comments
Open

Setting SETUPTOOLS_USE_DISTUTILS in setup.py? #584

jakirkham opened this issue Nov 5, 2021 · 7 comments

Comments

@jakirkham
Copy link

Currently setup.py is setting SETUPTOOLS_USE_DISTUTILS environment variable. See snippet below:

pyyaml/setup.py

Lines 69 to 70 in 8cdff2c

# for newer setuptools, enable the embedded distutils before importing setuptools/distutils to avoid warnings
os.environ['SETUPTOOLS_USE_DISTUTILS'] = 'local'

This has been causing us some issues when packaging PyYAML. Am curious if some other solution could be used here like...

  1. Could it be dropped?
  2. Could it check whether the environment variable is already set?
  3. ...?
@nitzmahone
Copy link
Member

You didn't specify what the actual issue was, and it wasn't clear from a quick scan of the linked issue...

It definitely can't be dropped entirely at the moment, since the stdlib distutils is going away and pyyaml setup is still relying on a bunch of ancient custom distutils bits for (among other things) the silent pure-Python fallback build when the extension build fails. Other folks have complained loudly about the deprecation warnings that fire when we don't set it (allowing setuptools to import stdlib distutils), and IIRC there are a couple of important bugfixes in the embedded one as well.

We could probably make it conditional on the envvar not being set, but I'd like to understand what the actual issue is before embarking down that path.

@nitzmahone
Copy link
Member

(it sounds like something to do with the universal2 compile/link flags dumpster fire, which we just punted on entirely in favor of arm64-only Mac builds because it was such a PITA)

@jakirkham
Copy link
Author

Sorry for being unclear.

IIUC the option isn't available for older Python versions. IOW we build for Python 3.7 & 3.8 and it doesn't work until 3.9.

Here's the relevant issue ( conda-forge/pyyaml-feedstock#38 ) and PR ( conda-forge/pyyaml-feedstock#38 ) where we have needed to patch it out.

cc @erykoff (in case you have more thoughts here)

@erykoff
Copy link

erykoff commented Nov 5, 2021

The problem is that the option is available for older pythons, but it is broken if you have any overrides of system default compiler flags (which we do for conda forge, particularly when cross compiling). This particular problem was fixed in python 3.9 but apparently not backported. (see pypa/setuptools#2257 ).

@nitzmahone
Copy link
Member

Ah, so you're monkeypatching the stdlib distutils and setuptools is using it's embedded copy?

@erykoff
Copy link

erykoff commented Nov 5, 2021

I don't believe so, though I don't know exactly how cross-python works. But in general, we just have compiler flags set that are ignored when that env variable is set to local.

@jakirkham
Copy link
Author

cc @chrisburr (who may be able to shed more light on how cross compiling is being done here)

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

No branches or pull requests

3 participants