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

{Core} Remove deprecated usages of distutils to support Python 3.12 #28796

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

svenstaro
Copy link
Contributor

@svenstaro svenstaro commented Apr 22, 2024

Related command

az --help

Description

Remove distutils to support Python 3.12
This is related to #27673.

from distutils.sysconfig import get_python_lib is slow. This PR also saves about 0.2s for each CLI command on Windows.

Testing Guide

On Python 3.12, do az --help. If it works, this is fixed.


This checklist is used to make sure that common guidelines for a pull request are followed.

Copy link

azure-client-tools-bot-prd bot commented Apr 22, 2024

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.11
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.11
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.11
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9

Copy link

Hi @svenstaro,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

Copy link

azure-client-tools-bot-prd bot commented Apr 22, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Apr 22, 2024

Packaging

@microsoft-github-policy-service microsoft-github-policy-service bot added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Apr 22, 2024
Copy link
Contributor

Thank you for your contribution svenstaro! We will review the pull request and get back to you soon.

@svenstaro
Copy link
Contributor Author

@microsoft-github-policy-service agree

@MThomassen
Copy link

Reaching out to @svenstaro : The current build on Arch, patched with the diff from this PR, still uses get_path("purepath") (instead of get_path("purelib")). The tool is essentially broken now on arch (extra/azure-cli 2.60.0-1)

@svenstaro
Copy link
Contributor Author

@MThomassen: Sorry, fixing right now.

@svenstaro
Copy link
Contributor Author

Should be fixed in Arch. Can we get this patch merged upstream?

@bebound
Copy link
Contributor

bebound commented May 29, 2024

from distutils.sysconfig import get_python_lib is slow.
This PR also saves about 0.2s for each CLI command on Windows.

@bebound bebound changed the title [Packaging] Remove deprecated usages of distutils {Core} Remove deprecated usages of distutils to support Python 3.12 May 29, 2024
@bebound
Copy link
Contributor

bebound commented May 29, 2024

In the official CLI package, we always install setuptools, and setuptools includes a ported version of distutils, ensuring that the related code continues to work in Python 3.12.

However, I believe eliminating its usage is the right move.

PS: In OS other than macOS, setuptools is not required. However, macOS needs to use setuptools to handle __import__('pkg_resources').declare_namespace(__name__) for packages installed from source.

Ref:
https://setuptools.pypa.io/en/latest/deprecated/distutils-legacy.html
pypa/setuptools#3533
#27196

@svenstaro
Copy link
Contributor Author

So are we good to merge? What's missing?

@@ -3,7 +3,7 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

from distutils.version import StrictVersion # pylint: disable=deprecated-module
from packaging.version import Version
Copy link
Contributor

@bebound bebound Jun 5, 2024

Choose a reason for hiding this comment

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

Recommended in pypa/packaging#520

Copy link
Member

Choose a reason for hiding this comment

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

Core migrated to packaging.version.parse long ago: #17667.

packaging.version.parse is the same as packaging.version.Version: https://github.com/pypa/packaging/blob/bb61b7bcbc429b49e6659a8b6c2966b6ebd046b6/src/packaging/version.py#L47-L56

@@ -22,7 +22,7 @@
EXTENSIONS_DIR = os.path.expanduser(_CUSTOM_EXT_DIR) if _CUSTOM_EXT_DIR else os.path.join(GLOBAL_CONFIG_DIR,
'cliextensions')
DEV_EXTENSION_SOURCES = _DEV_EXTENSION_SOURCES.split(',') if _DEV_EXTENSION_SOURCES else []
EXTENSIONS_SYS_DIR = os.path.expanduser(_CUSTOM_EXT_SYS_DIR) if _CUSTOM_EXT_SYS_DIR else os.path.join(get_python_lib(), 'azure-cli-extensions')
EXTENSIONS_SYS_DIR = os.path.expanduser(_CUSTOM_EXT_SYS_DIR) if _CUSTOM_EXT_SYS_DIR else os.path.join(get_path("purelib"), 'azure-cli-extensions')
Copy link
Member

@jiasli jiasli Jul 25, 2024

Choose a reason for hiding this comment

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

Sample result:

from distutils.sysconfig import get_python_lib
from sysconfig import get_paths
print(get_python_lib())
print(get_paths())
D:\cli\py311\Lib\site-packages
{
  'stdlib': 'C:\\Users\\xxx\\AppData\\Local\\Programs\\Python\\Python311\\Lib'
  'platstdlib': 'D:\\cli\\py311\\Lib'
  'purelib': 'D:\\cli\\py311\\Lib\\site-packages'
  'platlib': 'D:\\cli\\py311\\Lib\\site-packages'
  'include': 'C:\\Users\\xxx\\AppData\\Local\\Programs\\Python\\Python311\\Include'
  'platinclude': 'C:\\Users\\xxx\\AppData\\Local\\Programs\\Python\\Python311\\Include'
  'scripts': 'D:\\cli\\py311\\Scripts'
  'data': 'D:\\cli\\py311'
}

Also see python/cpython#85454 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between 'purelib' and 'platlib'? Will some extensions exist in platlib only, such as preinstalled extensions in cloudshell? Could you help verify it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@zhoxing-ms
Copy link
Contributor

@deveshdama Could you please help review the ACS related code?

@jiasli jiasli self-assigned this Jul 26, 2024
@jiasli jiasli merged commit b616adf into Azure:dev Jul 26, 2024
54 checks passed
@jiasli
Copy link
Member

jiasli commented Jul 26, 2024

@svenstaro, thanks a lot for your contribution. Great work!

@svenstaro svenstaro deleted the remove-distutils branch July 26, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot customer-reported Issues that are reported by GitHub users external to the Azure organization. Packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants