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.

Sign up for GitHub

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

Jump to bottom

{Packaging} Add setuptools to dependency #27196

Merged
merged 5 commits into from
Sep 11, 2024
Merged

{Packaging} Add setuptools to dependency #27196

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d6ea03
Init
bebound Aug 18, 2023
04bff56
Minor fix
bebound Aug 18, 2023
794489a
Remove it from requirements.txt
bebound May 29, 2024
4801ee2
Merge branch 'dev' into add-setuptools
bebound May 29, 2024
cf460e9
Merge branch 'dev' into add-setuptools
bebound Sep 11, 2024
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
View file
Open in desktop
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 •
edited
Loading

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)

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

Copy link

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.

Copy link

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

@bebound bebound Jul 12, 2024 •
edited
Loading

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

Ref: https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#pkg-resources-style-namespace-packages

Copy link

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