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

{Packaging} Add setuptools to dependency #27196

Merged
merged 5 commits into from
Sep 11, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/azure-cli/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@
'PyNaCl~=1.5.0',
'scp~=0.13.2',
'semver==2.13.0',
'setuptools',
Copy link
Contributor Author

@bebound bebound May 29, 2024

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.

Ref: https://github.com/Homebrew/brew/blob/cd651094878e88cb710bc24e253bff8b3595258e/Library/Homebrew/language/python.rb#L415-L418

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.

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.

Copy link
Contributor Author

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

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.

'six>=1.10.0', # six is still used by countless extensions
'sshtunnel~=0.1.4',
# Even though knack already depends on tabulate, profile module directly uses it for interactive subscription
Expand Down