Skip to content

Use python-build for package artifacts - #6433

Open
JanuszL wants to merge 4 commits into
NVIDIA:mainfrom
JanuszL:move-python-build-invocations
Open

Use python-build for package artifacts#6433
JanuszL wants to merge 4 commits into
NVIDIA:mainfrom
JanuszL:move-python-build-invocations

Conversation

@JanuszL

@JanuszL JanuszL commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Category:

Other

Description:

Modernize package artifact creation by replacing direct setup.py invocations
with python -m build. The change uses the standard build frontend while
retaining the existing non-isolated container-build behavior.

Additional information:

Affected modules and functionalities:

  • DALI Python wheel packaging in docker/build_helper.sh.
  • TensorFlow plugin source-distribution packaging.
  • Video plugin source-distribution packaging.

Key points relevant for the review:

  • Existing wheel options are forwarded as backend configuration settings.
  • --no-isolation preserves use of dependencies installed in the build image.

Tests:

  • Existing tests apply
    • build passes
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

Replace direct setup.py invocations with python -m build for DALI wheels and\nsource distributions. Preserve existing wheel configuration through backend\nconfig settings and avoid isolated dependency resolution in container builds.

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL

JanuszL commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

!build

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR modernizes DALI's package artifact creation by replacing direct setup.py invocations with python -m build across the main wheel build, the TF plugin sdist, and the video plugin sdist. The --no-isolation flag preserves the existing behavior of using dependencies already installed in the build image.

  • docker/Dockerfile.make_sdist_plugins correctly adds the build package so the video plugin sdist path works.
  • plugins/setup.py.in adds egg_info to the command skip-list so that the DALI import guard does not fire during PEP 517 metadata discovery — a necessary correctness fix for the new build frontend.
  • docker/Dockerfile_dali_tf was not updated to install build, leaving the TF plugin sdist path broken; make_dali_tf_sdist.sh now calls python -m build but the image only installs setuptools>=70.

Confidence Score: 4/5

Safe to merge for the video-plugin and main-wheel paths; the TF plugin sdist build will fail at runtime because Dockerfile_dali_tf does not install the build package.

Four of the five changed files are handled correctly. The omission is in Dockerfile_dali_tf: the PR explicitly adds build to Dockerfile.make_sdist_plugins for the video-plugin path, but the parallel TF-plugin image only installs setuptools>=70, so python -m build in make_dali_tf_sdist.sh will fail with No module named build.

docker/Dockerfile_dali_tf — needs build added to its pip install line to match what was done in Dockerfile.make_sdist_plugins.

Important Files Changed

Filename Overview
docker/Dockerfile_dali_tf Installs setuptools>=70 but omits the new build package dependency required by make_dali_tf_sdist.sh — TF plugin sdist build will fail.
docker/build_helper.sh Replaces python setup.py bdist_wheel with python -m build --wheel --no-isolation; options correctly split between --global-option and --build-option after previous review fix.
docker/Dockerfile.make_sdist_plugins Correctly adds the build package alongside existing setuptools>=70 and scikit-build to support python -m build in the video plugin path.
dali_tf_plugin/make_dali_tf_sdist.sh Switches TF plugin sdist creation from python setup.py sdist to python -m build --sdist --no-isolation; correct invocation, but depends on build being installed in the container image.
plugins/setup.py.in Adds egg_info to the skip-list alongside sdist so the DALI import/version guard does not run when build calls setup.py egg_info during PEP 517 metadata discovery.
plugins/video/CMakeLists.txt Replaces setup.py sdist with python -m build --sdist --no-isolation in the CMake post-build command for the video plugin.

Reviews (4): Last reviewed commit: "Allow plugin sdist metadata without DALI" | Re-trigger Greptile

Comment thread docker/build_helper.sh Outdated
Comment thread docker/build_helper.sh
@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59275705]: BUILD STARTED

Pass bdist_wheel options as backend build options and update the stale build comment.

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL

JanuszL commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

!build

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59277870]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59277870]: BUILD FAILED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL

JanuszL commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

!build

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59317998]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59317998]: BUILD FAILED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL

JanuszL commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

!build

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59363946]: BUILD STARTED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [59363946]: BUILD PASSED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants