Skip to content

[REVIEW] nvImageCodec v0.7.0 infra only #997

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
Open
cdinea wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[REVIEW] nvImageCodec v0.7.0 infra only #997

cdinea wants to merge 11 commits into from

Conversation

@cdinea
Copy link
Contributor

@cdinea cdinea commented Dec 17, 2025 •
edited
Loading

  • Pinned nvimgcodec/nvimagecodec deps to 0.7.0 in deps/conda/pyproject

  • Made cuslide2 plugin a MODULE + add linkable core lib for tests

  • Avoided global include_directories; use target_include_directories

@cdinea
Pin nvimgcodec to 0.7.0 and fix cuslide2 plugin CMake
dff2e2b 
- Pin nvimgcodec/nvimagecodec deps to 0.7.0 in deps/conda/pyproject

- Make cuslide2 plugin a MODULE + add linkable core lib for tests

- Avoid global include_directories; use target_include_directories
@cdinea cdinea requested review from a team as code owners December 17, 2025 02:04
@copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 17, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cdinea cdinea added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Dec 17, 2025
@cdinea
Copy link
Contributor Author

cdinea commented Dec 17, 2025

/ok to test dff2e2b

@cdinea cdinea changed the title [WIP] nvImgCodec v0.7.0 infra only [WIP] nvImageCodec v0.7.0 infra only Dec 17, 2025
@cdinea
Copy link
Contributor Author

cdinea commented Dec 17, 2025

/ok to test 3897ea7

@cdinea
Fix cuslide2 include propagation for nvimgcodec headers
a57cbb3 
Make nvimgcodec dynlink include path PUBLIC on cuslide2_core so the MODULE plugin target can compile cuslide.cpp (includes headers that include <nvimgcodec.h>).
@cdinea
Copy link
Contributor Author

cdinea commented Dec 17, 2025

/ok to test a57cbb3

@cdinea
Fix nvimgcodec pip pin for 0.7.0
71e9bf4 
PyPI provides nvidia-nvimgcodec-cu13 as 0.7.0.11
@cdinea
Copy link
Contributor Author

cdinea commented Dec 17, 2025

/ok to test 71e9bf4

@cdinea cdinea changed the title [WIP] nvImageCodec v0.7.0 infra only [REVIEW] nvImageCodec v0.7.0 infra only Dec 17, 2025
@cdinea
Copy link
Contributor Author

cdinea commented Dec 17, 2025

@jakirkham @KyleFromNVIDIA @gigony @grlee77 please review this PR that migrates to nvImageCodec v0.7.0 and adds the infra changes that @KyleFromNVIDIA requested on the second PR for nvImageCodec v0.6.0

@cdinea cdinea self-assigned this Dec 17, 2025
@cdinea cdinea mentioned this pull request Dec 30, 2025
Open
7 tasks
@jakirkham
Copy link
Member

jakirkham commented Jan 6, 2026

/ok to test

@copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 6, 2026

/ok to test

@jakirkham, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

@jakirkham
Copy link
Member

jakirkham commented Jan 6, 2026

/ok to test 7cdb055

@jakirkham
Copy link
Member

jakirkham commented Jan 6, 2026 •
edited
Loading

Tried to resolve the conflicts here with a merge commit

AFAICT they are fixed however GitHub's UI is still referring to files no longer in the PR. Not sure what is up with that. Hoping GitHub will catch up a bit Edit: Looks like GitHub's UI is now reflecting the resolution

@jakirkham
Copy link
Member

jakirkham commented Jan 6, 2026

/ok to test f836348

@jakirkham
Copy link
Member

jakirkham commented Jan 6, 2026

Pushed another commit to fix the license year checker (as it is a new year now). Looks like that fixed up the style checker issues

@jakirkham
Copy link
Member

jakirkham commented Jan 6, 2026

Looks like one CI job errored

From a quick look not seeing a test failure or other attributable cause. All other jobs appeared to have passed

Restarted the failing CI job

@cdinea
Copy link
Contributor Author

cdinea commented Jan 6, 2026

Pushed another commit to fix the license year checker (as it is a new year now). Looks like that fixed up the style checker issues

thank you @jakirkham

@cdinea
Copy link
Contributor Author

cdinea commented Jan 6, 2026

Looks like one CI job errored

From a quick look not seeing a test failure or other attributable cause. All other jobs appeared to have passed

Restarted the failing CI job

thank you for the help @jakirkham - it looks like all checks passed

grlee77
grlee77 previously requested changes Jan 6, 2026
View reviewed changes
Copy link
Contributor

@grlee77 grlee77 left a comment

Choose a reason for hiding this comment

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

Thanks @cdinea, a couple of minor comments/questions

@cdinea cdinea mentioned this pull request Jan 6, 2026
Closed
@cdinea
Copy link
Contributor Author

cdinea commented Jan 7, 2026

/ok to test 5fa974c

@jakirkham
Copy link
Member

jakirkham commented Jan 7, 2026

/ok to test 3224085

@jakirkham jakirkham dismissed grlee77’s stale review January 7, 2026 02:10

Feedback addressed

@jakirkham jakirkham requested a review from grlee77 January 7, 2026 02:10
@cdinea
Copy link
Contributor Author

cdinea commented Jan 7, 2026

/ok to test 2d137c3

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

Reviewers

@KyleFromNVIDIA KyleFromNVIDIA Awaiting requested review from KyleFromNVIDIA KyleFromNVIDIA is a code owner automatically assigned from rapidsai/packaging-codeowners

@grlee77 grlee77 Awaiting requested review from grlee77

At least 1 approving review is required to merge this pull request.

Assignees

@cdinea cdinea

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cdinea @jakirkham @grlee77