Remove distutils by driazati · Pull Request #57040 · pytorch/pytorch

driazati · 2021-04-27T18:16:39Z

distutils is on its way out and will be deprecated-on-import for Python 3.10+ and removed in Python 3.12 (see PEP 632). There's no reason for us to keep it around since all the functionality we want from it can be found in setuptools / sysconfig.

Fixes #56527

Differential Revision: D28051356

[distutils](https://docs.python.org/3/library/distutils.html) is on its way out and will be deprecated-on-import for Python 3.10+ and removed in Python 3.12 (see [PEP 632](https://www.python.org/dev/peps/pep-0632/)). There's no reason for us to keep it around since all the functionality we want from it can be found in `setuptools`.t s

facebook-github-bot · 2021-04-27T18:16:46Z

💊 CI failures summary and remediations

As of commit 82bcc49 (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

ezyang · 2021-04-28T14:40:05Z

caffe2/CMakeLists.txt

-      print(sysconfig.get_python_lib(prefix=''))
+      import sysconfig
+      relative_site_packages = sysconfig.get_path('purelib').replace(sysconfig.get_path('data'), '').lstrip(os.path.sep)
+      print(relative_site_packages)


the heck is this

sysconfig hardcodes these paths with templates, but distutils.sysconfig doesn't and lets you supply your own prefix (i.e. base in the link), so here we end up having to remove the base (and its trailing slash) prefix manually.

>>> from distutils import sysconfig >>> sysconfig.get_python_lib() '/home/davidriazati/local/miniconda3/envs/pytorch/lib/python3.9/site-packages' >>> sysconfig.get_python_lib(prefix='') 'lib/python3.9/site-packages'

>>> import sysconfig >>> sysconfig.get_path('purelib') '/home/davidriazati/local/miniconda3/envs/pytorch/lib/python3.9/site-packages' >>> sysconfig.get_path('data') '/home/davidriazati/local/miniconda3/envs/pytorch'

ezyang · 2021-04-28T14:42:40Z

For the most part things look simpler. But two spots look very suspect, both in caffe2/CMakeLists.txt, where the logic has gotten substantially longer. What's going on?

driazati · 2021-04-28T20:04:53Z

See comment about the first weird thing, for the second it's only to work around that bug linked in the comments (the fix for which is in 3.8+ but isn't getting backported any further than that). There are definitely some un-addressed gaps in the deprecation PEP but it's accepted so 🤷

ezyang

okey dokey

facebook-github-bot · 2021-04-29T19:11:34Z

@driazati merged this pull request in 4b96fc0.

xkszltl · 2021-05-01T11:05:27Z

I noticed on Ubuntu 18.04, caffe2 has been moved from /usr/local/lib/python3/dist-packages to /usr/local/lib/python3.6/site-packages recently, with dist->site and 3->3.6.
I love the move because it's finally consistent with other distros, but not sure which commit introduced the behavior, to check whether it is a reliable change.
So far I only found this PR to be suspicious.
IIRC Ubuntu has some special patches for distutils to create things like dist-packages vs. site-packages.
If so, could you have confirm?

Thanks in advance.

xkszltl · 2021-05-01T15:30:20Z

I can confirm the change is related to this PR:

# lsb_release -d
Description:    Ubuntu 18.04.5 LTS
# python3 -c 'import sysconfig, distutils.sysconfig; print(sysconfig.get_path("purelib"), distutils.sysconfig.get_python_lib())'
/usr/lib/python3.6/site-packages /usr/lib/python3/dist-packages

Another thing I want to mention is, seems by default Debian python only load from dist-packages:

# python3 -c 'import sys; print(sys.path)'
['', '/usr/lib/python36.zip', '/usr/lib/python3.6', '/usr/lib/python3.6/lib-dynload', '/usr/local/lib/python3.6/dist-packages', '/usr/lib/python3/dist-packages']

- pytorch/pytorch#57040

ezyang · 2021-05-03T02:43:03Z

@xkszltl Thanks for the diagnosis. Do you think there's anything we need to do?

xkszltl · 2021-05-03T03:16:49Z

Do you think there's anything we need to do?

Confirm if this is an intentional change (so that it won't be "fixed"). If so I can start depending on the new result.
Double check if Debian/Ubuntu can pick it up from site-packages without extra PYTHONPATH.

driazati · 2021-05-03T17:16:03Z

This shouldn't affect wheel installs since those are all relative from the .whl file zip structure, but I think it would mess up builds from source on Debian/Ubuntu that don't use conda (i.e. just using the bare system Python). I'm not sure if we even support this (@malfet ?) and it wasn't caught by the CI/my local development since the Python versions there come from CentOS as described in PEP 513.

It seems to be an unresolved issue, see pypa/distutils#17, looks like setuptools reverted their move away from distutils due to that issue. The SETUPTOOLS_USE_DISTUTILS fix also may not here since we manually specify the install location in cmake.

So if we require conda then this isn't a problem (the language in the README seems to only strongly suggest it, but then assumes it's there), but if we don't then the total removal of distutils might be premature until setuptools comes up with a better fix.

xkszltl · 2021-05-04T16:40:28Z

@driazati
If for backward compatibility you may simply symlink between python3/dist-packages/torch and python3.x/site-packages/torch.

We've been using cmake+pip build for a long while without issue, so I believe that's supported in reality, no matter official or not.
Requiring conda would be a bad idea, because its own environment won't be compatible with other things on system.
IMHO conda is kind of "evil", in the sense of env contamination.

Summary: [distutils](https://docs.python.org/3/library/distutils.html) is on its way out and will be deprecated-on-import for Python 3.10+ and removed in Python 3.12 (see [PEP 632](https://www.python.org/dev/peps/pep-0632/)). There's no reason for us to keep it around since all the functionality we want from it can be found in `setuptools` / `sysconfig`. `setuptools` includes a copy of most of `distutils` (which is fine to use according to the PEP), that it uses under the hood, so this PR also uses that in some places. Fixes pytorch#56527 Pull Request resolved: pytorch#57040 Pulled By: driazati Reviewed By: nikithamalgifb Differential Revision: D28051356 fbshipit-source-id: 1ca312219032540e755593e50da0c9e23c62d720

This fixes regression introduced by pytorch#57040 Somehow importing `distutils` from `setuptool` caused import of `distutils.versions`, which is not a documented dependency and got change with the release of [setuptools-59.6.0](https://github.com/pypa/setuptools/tree/v59.6.0) We should not rely on that, as `import distutils` never re-imports `distutils.version`, which one can see by observing https://github.com/python/cpython/blob/3.9/Lib/distutils/__init__.py or by running: ``` % python3 -c "import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'sys'] % python3 -c "from setuptools import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'archive_util', 'ccompiler', 'cmd', 'config', 'core', 'debug', 'dep_util', 'dir_util', 'dist', 'errors', 'extension', 'fancy_getopt', 'file_util', 'filelist', 'log', 'spawn', 'sys', 'sysconfig', 'util', 'version'] ```

Summary: This fixes regression introduced by #57040 Somehow importing `distutils` from `setuptool` caused import of `distutils.versions`, which is not a documented dependency and got change with the release of [setuptools-59.6.0](https://github.com/pypa/setuptools/tree/v59.6.0) We should not rely on that, as `import distutils` never re-imports `distutils.version`, which one can see by observing https://github.com/python/cpython/blob/3.9/Lib/distutils/__init__.py or by running: ``` % python3 -c "import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'sys'] % python3 -c "from setuptools import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'archive_util', 'ccompiler', 'cmd', 'config', 'core', 'debug', 'dep_util', 'dir_util', 'dist', 'errors', 'extension', 'fancy_getopt', 'file_util', 'filelist', 'log', 'spawn', 'sys', 'sysconfig', 'util', 'version'] ``` Pull Request resolved: #69904 Reviewed By: albanD, atalman, janeyx99 Differential Revision: D33094453 Pulled By: malfet fbshipit-source-id: aaf1adb7c6f293c4e376ccff21c64cd6ba625e97

Summary: This fixes regression introduced by pytorch#57040 Somehow importing `distutils` from `setuptool` caused import of `distutils.versions`, which is not a documented dependency and got change with the release of [setuptools-59.6.0](https://github.com/pypa/setuptools/tree/v59.6.0) We should not rely on that, as `import distutils` never re-imports `distutils.version`, which one can see by observing https://github.com/python/cpython/blob/3.9/Lib/distutils/__init__.py or by running: ``` % python3 -c "import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'sys'] % python3 -c "from setuptools import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'archive_util', 'ccompiler', 'cmd', 'config', 'core', 'debug', 'dep_util', 'dir_util', 'dist', 'errors', 'extension', 'fancy_getopt', 'file_util', 'filelist', 'log', 'spawn', 'sys', 'sysconfig', 'util', 'version'] ``` Pull Request resolved: pytorch#69904 Reviewed By: albanD, atalman, janeyx99 Differential Revision: D33094453 Pulled By: malfet fbshipit-source-id: aaf1adb7c6f293c4e376ccff21c64cd6ba625e97

Properly import LooseVersion (pytorch#69904) Summary: This fixes regression introduced by pytorch#57040 Somehow importing `distutils` from `setuptool` caused import of `distutils.versions`, which is not a documented dependency and got change with the release of [setuptools-59.6.0](https://github.com/pypa/setuptools/tree/v59.6.0) We should not rely on that, as `import distutils` never re-imports `distutils.version`, which one can see by observing https://github.com/python/cpython/blob/3.9/Lib/distutils/__init__.py or by running: ``` % python3 -c "import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'sys'] % python3 -c "from setuptools import distutils;print(distutils.__version__, dir(distutils))" 3.7.5 ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'archive_util', 'ccompiler', 'cmd', 'config', 'core', 'debug', 'dep_util', 'dir_util', 'dist', 'errors', 'extension', 'fancy_getopt', 'file_util', 'filelist', 'log', 'spawn', 'sys', 'sysconfig', 'util', 'version'] ``` Pull Request resolved: pytorch#69904 Reviewed By: albanD, atalman, janeyx99 Differential Revision: D33094453 Pulled By: malfet fbshipit-source-id: aaf1adb7c6f293c4e376ccff21c64cd6ba625e97

xkszltl · 2022-12-02T18:56:35Z

On Ubuntu 22.04, saw it moved to /usr/local/lib/python3.10/dist-packages recently.

driazati added 5 commits April 26, 2021 16:12

more fixes

eca695d

fix android

cf97c40

Fix windows

49f9599

fix EXT_SUFFIX

82bcc49

driazati requested review from ezyang, fmassa and soumith as code owners April 27, 2021 18:16

facebook-github-bot added the cla signed label Apr 27, 2021

driazati removed request for ezyang, fmassa and soumith April 27, 2021 18:51

driazati mentioned this pull request Apr 28, 2021

Remove distutils #56407

Closed

driazati changed the title ~~ci all/test distutils~~ Remove distutils Apr 28, 2021

driazati requested review from ezyang, fmassa, malfet and soumith April 28, 2021 04:23

ezyang reviewed Apr 28, 2021

View reviewed changes

ezyang approved these changes Apr 29, 2021

View reviewed changes

facebook-github-bot closed this in 4b96fc0 Apr 29, 2021

facebook-github-bot added the Merged label Apr 29, 2021

xkszltl added a commit to xkszltl/Roaster that referenced this pull request May 1, 2021

Update pytorch relocation after removing distutils.

8833e6d

- pytorch/pytorch#57040

thinking-tower mentioned this pull request May 27, 2021

Set PYTHON_INCLUDE_DIR and PYTHON_LIBRARY during installation KomputeProject/kompute#222

Merged

malfet mentioned this pull request Dec 14, 2021

Properly import LooseVersion #69904

Closed

xuzhao9 mentioned this pull request Dec 14, 2021

Remove the use of distutils in gen_summary_metadata.py pytorch/benchmark#650

Closed

cswinter mentioned this pull request Mar 14, 2022

Use torch 1.11.0 entity-neural-network/incubator#196

Merged

github-actions bot deleted the ci-all/test_distutils branch February 11, 2024 01:55

Conversation

driazati commented Apr 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Apr 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

ezyang Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

driazati Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

ezyang commented Apr 28, 2021

Uh oh!

driazati commented Apr 28, 2021

Uh oh!

ezyang left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Apr 29, 2021

Uh oh!

xkszltl commented May 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xkszltl commented May 1, 2021

Uh oh!

ezyang commented May 3, 2021

Uh oh!

xkszltl commented May 3, 2021

Uh oh!

driazati commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xkszltl commented May 4, 2021

Uh oh!

xkszltl commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

driazati commented Apr 27, 2021 •

edited

Loading

facebook-github-bot commented Apr 27, 2021 •

edited

Loading

xkszltl commented May 1, 2021 •

edited

Loading

driazati commented May 3, 2021 •

edited

Loading

xkszltl commented Dec 2, 2022 •

edited

Loading