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

[PyOV] Migrate wheel building interface to python -m build for main OV wheel #27190

Merged
merged 5 commits into from
Oct 23, 2024
Merged

[PyOV] Migrate wheel building interface to python -m build for main OV wheel #27190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec71622
Migrate wheel build to python -m build
p-wysocki Oct 22, 2024
f3e5ff7
Minor change
p-wysocki Oct 22, 2024
fc5123d
Requirements update
p-wysocki Oct 22, 2024
f6adc9e
Merge branch 'master' into setuppy_migration
p-wysocki Oct 23, 2024
0f91da7
Merge branch 'master' into setuppy_migration
p-wysocki Oct 23, 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/bindings/python/constraints.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pytest-html==4.1.1
pytest-timeout==2.3.1

# Python bindings
build<1.3
py>=1.9.0
pygments>=2.8.1
setuptools>=65.6.1,<75.3.0
Expand Down
33 changes: 8 additions & 25 deletions src/bindings/python/wheel/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,31 +100,14 @@ set(openvino_wheel_path "${openvino_wheels_output_dir}/${openvino_wheel_name}")
# create target for openvino.wheel
#

execute_process(COMMAND ${Python3_EXECUTABLE} -m pip --version
OUTPUT_VARIABLE pip_version OUTPUT_STRIP_TRAILING_WHITESPACE)

string(REGEX MATCH "pip[ ]+([\\.0-9]*)" pip_version "${pip_version}")
set(pip_version ${CMAKE_MATCH_1})

if(pip_version VERSION_GREATER_EQUAL 22.0)
set(wheel_build_command
${Python3_EXECUTABLE} -m pip wheel
--no-deps
--wheel-dir ${openvino_wheels_output_dir}
--verbose
--build-option --build-number=${WHEEL_BUILD}
--build-option --plat-name=${PLATFORM_TAG}
"${CMAKE_CURRENT_SOURCE_DIR}")
else()
set(wheel_build_command
${Python3_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/setup.py"
--quiet
--no-user-cfg
bdist_wheel
--dist-dir ${openvino_wheels_output_dir}
--build-number=${WHEEL_BUILD}
--plat-name=${PLATFORM_TAG})
endif()
# for --config-setting explanation see https://github.com/pypa/setuptools/issues/2491
set(wheel_build_command
${Python3_EXECUTABLE} -m build "${CMAKE_CURRENT_SOURCE_DIR}"
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, should we use pip wheel instead ? to avoid dependency on build

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, are you going to rework this call as well ?
https://github.com/openvinotoolkit/openvino/blob/master/src/bindings/python/CMakeLists.txt#L328

Copy link
Contributor Author

@p-wysocki p-wysocki Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, should we use pip wheel instead ? to avoid dependency on build

In the original code there was an if depending on pip version, and for older pips it would call setup.py directly. I tried unifying the functionality to call pip wheel for all pip versions, but it seems there's some incompatibility which results in very unfriendly errors which would require (in my opinion) a lot of changes, only once I figure out what needs to be changed.

Changing to using build (which is a pypa recommended replacement) works for all pip versions and removes the branching based on pip version.

We could work on making pip wheel work with old pip versions, but I believe it will be time consuming and difficult, or stay with pip wheel but drop support for older pips.

BTW, are you going to rework this call as well ?
https://github.com/openvinotoolkit/openvino/blob/master/src/bindings/python/CMakeLists.txt#L328

The changes regarding this call are in #27227.

Copy link
Contributor

Choose a reason for hiding this comment

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

we can enable pip wheel for newer pip versions, while old ones (via build) will not be used after some time and we can just force to use newer pip only (once Ubuntu 20.04 is dropped, for example)

--outdir ${openvino_wheels_output_dir}
--config-setting=--build-option=--build-number=${WHEEL_BUILD}
--config-setting=--build-option=--plat-name=${PLATFORM_TAG}
--config-setting=--quiet
--wheel)

add_custom_command(OUTPUT ${openvino_wheel_path}
COMMAND ${setup_py_env} ${wheel_build_command}
Expand Down
1 change: 1 addition & 0 deletions src/bindings/python/wheel/requirements-dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
-c ../constraints.txt
setuptools
wheel
build
patchelf; sys_platform == 'linux' and platform_machine == 'x86_64'
Loading