Skip to content

{Packaging} Add setuptools to dependency #27196

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

Merged
bebound merged 5 commits into from
Sep 11, 2024
Merged

{Packaging} Add setuptools to dependency #27196

Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Init
  • Loading branch information
@bebound
bebound committed Aug 18, 2023
commit 8d6ea037135d536aff017a22e4cb8b6b7f70a205
2 changes: 1 addition & 1 deletion build_scripts/windows/scripts/build.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ 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
Copy link
Contributor Author

@bebound bebound Aug 18, 2023

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.

Copy link
Member

@jiasli jiasli Aug 24, 2023

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.

%BUILDING_DIR%\python.exe -m pip install --upgrade --no-warn-script-location setuptools
for %%a in (%CLI_SRC%\azure-cli %CLI_SRC%\azure-cli-core %CLI_SRC%\azure-cli-telemetry) do (
pushd %%a
%BUILDING_DIR%\python.exe -m pip install --no-warn-script-location --no-cache-dir --no-deps .
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/requirements.py3.Darwin.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ requests-oauthlib==1.2.0
requests[socks]==2.31.0
scp==0.13.2
semver==2.13.0
setuptools
Copy link
Member

@jiasli jiasli Aug 24, 2023 •
edited
Loading

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.

Copy link
Contributor Author

@bebound bebound Aug 31, 2023

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

six==1.16.0
sshtunnel==0.1.5
tabulate==0.8.9
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/requirements.py3.Linux.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ requests-oauthlib==1.2.0
requests[socks]==2.31.0
scp==0.13.2
semver==2.13.0
setuptools
six==1.16.0
sshtunnel==0.1.5
tabulate==0.8.9
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/requirements.py3.windows.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ requests-oauthlib==1.2.0
requests[socks]==2.31.0
scp==0.13.2
semver==2.13.0
setuptools
six==1.16.0
sshtunnel==0.1.5
tabulate==0.8.9
Expand Down
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 @@ -145,6 +145,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

@edmorley edmorley Jul 11, 2024

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

@agronholm agronholm Jul 11, 2024

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

@agronholm agronholm Jul 12, 2024

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',
'urllib3',
Expand Down