-
Notifications
You must be signed in to change notification settings - Fork 3k
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
{Packaging} Add setuptools
to dependency
#27196
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
Packaging |
9a9113d
to
8d6ea03
Compare
@@ -122,7 +122,6 @@ set PYTHON_EXE=%PYTHON_DIR%\python.exe | |||
robocopy %PYTHON_DIR% %BUILDING_DIR% /s /NFL /NDL | |||
|
|||
set CLI_SRC=%REPO_ROOT%\src | |||
%BUILDING_DIR%\python.exe -m pip install --no-warn-script-location --force-reinstall pycparser==2.18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pycparser==2.18
does not work. It is in the requirements.txt and version is 2.19
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pycparser==2.18
was added by #7662. Funny thing is that no one knows why it was introduced in the first place.
@@ -129,6 +129,7 @@ requests-oauthlib==1.2.0 | |||
requests[socks]==2.31.0 | |||
scp==0.13.2 | |||
semver==2.13.0 | |||
setuptools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think items under requirements.*.txt
should always be pinned as requirements.txt
is the output of pip freeze
. This gives us a stable dependency tree when building packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have a pinned version in requirements.*.txt
On the one hand, we always use latest setuptools
and pip
when build packages for now. We've used this strategy for years. #24992 (comment)
On the other hand, unpinned setuptools
caused a problem before: #25869
What dependencies still use the deprecated |
Some packages are installed from source code on MacOS. Not sure if this problem still exists. |
How does getting installed from source relate to my question? |
This is only related to MacOS. Details are provided in the PR. |
In 2.61.0, these two files still exists in HomeBrew, which means They might be created by |
@@ -144,6 +144,7 @@ | |||
'PyNaCl~=1.5.0', | |||
'scp~=0.13.2', | |||
'semver==2.13.0', | |||
'setuptools', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once it's added in setup.py
, brew will install setuptools
while installing azure-cli
.
The command is python3.xx -m pip --python=xxxxx/Cellar/azure-cli/2.xx.0/libexec/bin/python install buildpath/src/azure-cli
.
No, homebrew install packages with --no-deps
, have to add pip
and setuptools
into resource list explicitly: #29887 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bebound Hi! I don't believe install_requires
is the correct place to add setuptools
- instead you should use setup_requires
, since the former isn't installed before the rest of the other packages. Also note that even after the get-pip changes, pip still injects setuptools/wheel via the isolated build environments, so it may not be necessary for you to add setuptools
at all. Lastly, you may want to consider migrating to pyproject.toml
instead of setup.py
and declaring an explicit build backend per PEP 518.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why this PR seeks to add setuptools to run-time dependencies is the use of pkg_resources
to declare an old-style namespace package. This, of course, is something that should be eliminate entirely instead of adding setuptools
to run-time dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the search result. I'm not sure if any extension uses it.
azure/multiapi/storagev2/queue/__init__.py
1:__import__('pkg_resources').declare_namespace(__name__)
azure/multiapi/storagev2/blob/__init__.py
1:__import__('pkg_resources').declare_namespace(__name__)
azure/multiapi/storagev2/filedatalake/__init__.py
1:__import__('pkg_resources').declare_namespace(__name__)
azure/multiapi/storagev2/fileshare/__init__.py
1:__import__('pkg_resources').declare_namespace(__name__)
azure/multiapi/cosmosdb/v2017_04_17/__init__.py
1:__import__('pkg_resources').declare_namespace(__name__)
azure/cli/command_modules/serviceconnector/_utils.py
382: import pkg_resources
383: installed_packages = pkg_resources.working_set
PS: import__('pkg_resources').declare_namespace(__name__)
should not exists in the installed packages. Their packaging logic is wrong.
If they use pkg_resources
namespace package, they forget to add namespace_packages=['azure.multipai.xxxxxx']
in setup.py
.
But they have already used pkgutil
namespace pacakge, pkg_resources
specific config were not removed in this pr: Azure/azure-multiapi-storage-python#28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the storage-multiapi package should be fixed first. There's no point in it supporting Python 2.7 anymore, as its only downstream package (this one) requires Python 3.8 or later.
I'm a bit disappointed that the problem was fixed the wrong way (adding setuptools instead of eliminating old-style namespace packages). |
setuptools
into dependencysetuptools
to dependency
Description
setuptools
will not be installed when create venv in Python 3.12, see python/cpython#95299.But some dependencies still use
__import__('pkg_resources').declare_namespace(__name__)
to declare the namespace package, which requiressetuptools
.Adding it into
setup.py
to make sure it is installed correctly when installazure-cli
in Homebrew.setuptools
can be removed if azure-cli and dependencies do not usepkg_resources
anymore.This PR ensure the
setuptools
can be install in venv.The
build.cmd
andformula_generate.py
were updated to be explicitly installed in the package: #29887setuptools
in all platforms:pip
andsetuptools
in build script.get-pip.py
to installsetuptools
inexplicitly. it's removed in 3.12: Stop installing setuptools and wheel on Python 3.12+ pypa/get-pip#218get-pip.py
to installsetuptools
inexplicitly, it's removed in 3.12setuptools
when create venv, it's removed in 3.12.Related issue:
--without-pip
Homebrew/brew#15792Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a
: Make some customer-facing breaking change[Component Name 2]
az command b
: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.