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

Remove old msvc modules (pypa/distutils@cc017c7) #3505

Merged
merged 3 commits into from
Aug 13, 2022
Merged

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Aug 12, 2022

Summary of changes

Closes

Pull Request Checklist

@jaraco
Copy link
Member Author

jaraco commented Aug 13, 2022

I'm a little uneasy releasing this without first a deprecation period, except for the fact that this code is only reachable by directly importing the modules, and not through any of the build processes. Moreover, these modules are for very old versions of MSVC. According to WindowsCompilers, only Visual C++ 14 is relevant to Python 3.7+. Therefore, I'd like to proceed with this change. I've marked it as breaking, just in case it is, even though it probably isn't in the vast majority of cases. If it turns out to break important use cases, we can consider rolling it back.

@jaraco jaraco merged commit 26d5d07 into main Aug 13, 2022
@jaraco jaraco deleted the distutils-cc017c7 branch August 13, 2022 16:20
@aminvakil
Copy link

Not sure if it's worth rolling back or not, just to let you know this has broke installing implicit 0.4.8:

Collecting implicit==0.4.8
  Downloading implicit-0.4.8.tar.gz (1.1 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.1/1.1 MB 17.8 MB/s eta 0:00:00
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'error'
  error: subprocess-exited-with-error
  
  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [19 lines of output]
      Traceback (most recent call last):
        File "/usr/local/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 363, in <module>
          main()
        File "/usr/local/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 345, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "/usr/local/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 130, in get_requires_for_build_wheel
          return hook(config_settings)
        File "/tmp/pip-build-env-b7m7tjjj/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 337, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=['wheel'])
        File "/tmp/pip-build-env-b7m7tjjj/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 319, in _get_build_requires
          self.run_setup()
        File "/tmp/pip-build-env-b7m7tjjj/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 474, in run_setup
          super(_BuildMetaLegacyBackend,
        File "/tmp/pip-build-env-b7m7tjjj/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 334, in run_setup
          exec(code, locals())
        File "<string>", line 10, in <module>
        File "/tmp/pip-install-ac5qgfu8/implicit_29f7b36130a84718bd14990aaa15bd44/cuda_setup.py", line 6, in <module>
          from distutils import ccompiler, errors, msvccompiler, unixccompiler
      ImportError: cannot import name 'msvccompiler' from 'distutils' (/tmp/pip-build-env-b7m7tjjj/overlay/lib/python3.8/site-packages/setuptools/_distutils/__init__.py)
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

@aminvakil
Copy link

I used pip install implicit==0.4.8 --no-build-isolation to overcome this issue, not sure if we will face any other issue later :)

@mgorny
Copy link
Contributor

mgorny commented Aug 15, 2022

This also broke gobject-introspection, so pretty much a lot of GNOME-related stuff: https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/438

@tacaswell
Copy link

This also broke numpy.distutils which in turn breaks building at least pywavelets

@cbeuw
Copy link

cbeuw commented Aug 15, 2022

numpy.distutils.system_info causes distutils.msvccompiler to be imported unconditionally. If setuptools is specified in a dependency package's pyproject.toml without version pinning (such as spams), then the downstream application cannot force an earlier version of setuptools to be used in requirements.txt.

However, having setuptools<65 in requirements.txt with pip install --no-use-pep517 does fix it

@tiran
Copy link
Contributor

tiran commented Aug 16, 2022

@jaraco Why were the modules removed? The PR does not give any reason for the removal.

Also distutils.msvccompiler is a public, documented API of distutils. The removal of the modules is an API breaking change. Since setuptools overrides Python stdlib's distutils with _distutils_hack the change is breaking code that depends on stable APIs.

IMHO setuptools must not remove or change any distutils APIs until it drops support for Python 3.11 (!).

@svajiraya
Copy link

svajiraya commented Aug 16, 2022

FYI: This is impacting installation of numpy==1.15.4 (dependency of statsmodels==0.12.2) on aarch64.

@pradyunsg
Copy link
Member

FWIW, users affected by setuptools at build time can set a PIP_CONSTRAINTS environment variable, pointing to a file restricting setuptools to a compatible version.

@esc
Copy link

esc commented Aug 16, 2022

FWIW: this broke numba too: numba/numba#8356

@cbeuw
Copy link

cbeuw commented Aug 16, 2022

This needs to be reverted ASAP as there is no workaround due to how broken PEP 517 is.

If a package has non-version-pinned setuptools in their pyproject.toml, there is no way to force pip to use a specific version of setuptools when running pip install on that package, until that package maintainer pins setuptools and releases a new version to pypi. requirements.txt does nothing, PIP_CONSTRAINTS does nothing, because pyproject.toml creates an isolated build environment that cannot be made deterministic by the downstream user of the package.

It's an appalling design. Likely relevant: jazzband/pip-tools#1396

@tacaswell
Copy link

tacaswell commented Aug 16, 2022

@cbeuw Please remember that almost everyone working on this is a volunteer and is doing there best to both move things forward while trying to keep everything working for everyone.

I understand you are frustrated, however invectives against the design and maintainers is not productive, and will not get the issue resolved any faster.


I believe that:

pip install -v --no-build-isolation ....

will avoid creating the isolated environment and give you full control over the versions used.

[edited to fix terrible markup error]

@cbeuw
Copy link

cbeuw commented Aug 16, 2022

I believe that:

pip install -v --no-build-isolation ....

will avoid creating the isolated environment and give you full control over the versions used.

That worked, thanks. Although you need to install build dependencies manually in a separate command before doing that. Cunningham's Law at work :)

@jaraco
Copy link
Member Author

jaraco commented Aug 16, 2022

Thanks everyone for the feedback. Based on the rationale I wrote above, I had not expected these modules to be in use anywhere. I see now they are and can roll it back.

Why were the modules removed? The PR does not give any reason for the removal.

I removed the modules because they're superseded by other modules, contain a lot of duplicated logic, and were presumed not to be in use (it's bad practice to have code around that's not called or even covered by tests). I should have included that rationale in the PR description.

IMHO setuptools must not remove or change any distutils APIs until it drops support for Python 3.11 (!).

(response moved to #3532)


I apologize for the disruption and I'll get a rollback out asap.

@jaraco
Copy link
Member Author

jaraco commented Aug 17, 2022

I'm having difficulty releasing the rollback. The CI tests for macOS on PyPy are failing, but only after the rollback.

@jaraco
Copy link
Member Author

jaraco commented Aug 17, 2022

I've gone ahead and just merged the changes and and pushing them out as v65.0.2. If the tests are still failing, I'll cut the release manually.

@jaraco
Copy link
Member Author

jaraco commented Aug 17, 2022

Now other tests are failing. It appears the tests are flaky. I've just pushed the release manually.

@esc
Copy link

esc commented Aug 17, 2022

Would it be possible to yank the "broken" releases from PyPi as a pre-cautionary measure?

@CAM-Gerlach
Copy link

Based on the rationale I wrote above, I had not expected these modules to be in use anywhere. I see now they are and can roll it back.

FYI, it may not always be conclusive, but a quick check of e.g. grep.app can be quite useful in these situations, both to see whether and how an existing API is used in the real world, beyond just theoretical considerations.

@EvgenKo423
Copy link

I guess, there would be much less packages broken if you would've done one more thing: renamed _msvccompiler into msvccompiler. ;-)

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.

None yet