Skip to content

Conversation

divyegala
Copy link
Member

No description provided.

@divyegala divyegala requested a review from a team as a code owner September 6, 2025 04:18
enable_check_generated_files: false
ignored_pr_jobs: "telemetry-summarize"
conda-cpp-build:
needs: checks
Copy link
Member

Choose a reason for hiding this comment

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

Why has this been removed? wheel-build-libcuvs still has it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to run CI, I couldn't get the style checker to pass because pre-commit for me locally does not raise any CMake styling issue.

- libcurand
- libcusolver
- libcusparse
- libnvjitlink-dev
Copy link
Member

Choose a reason for hiding this comment

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

Why does this have the -dev suffix? The others above in this section don't.

- libcusparse-dev
run:
- ${{ pin_compatible("cuda-version", upper_bound="x", lower_bound="x") }}
- libnvjitlink-dev
Copy link
Member

Choose a reason for hiding this comment

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

Why does this have the -dev suffix? Headers shouldn't be needed at runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point, we probably only need libnvjitlink at run.

get_filename_component(obj_name ${obj} NAME_WE)
get_filename_component(obj_dir ${obj} DIRECTORY)

if(obj_ext MATCHES ".fatbin")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(obj_ext MATCHES ".fatbin")
if(obj_ext MATCHES "\\.fatbin$")

# =============================================================================

set(file_contents)
foreach(obj ${OBJECTS})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
foreach(obj ${OBJECTS})
foreach(obj IN LISTS OBJECTS)

- output_types: conda
packages:
- libcuvs-tests==25.10.*,>=0.0.0a0
depends_on_libnvjitlink-dev:
Copy link
Member

Choose a reason for hiding this comment

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

We typically don't put hyphens in dependency set names:

Suggested change
depends_on_libnvjitlink-dev:
depends_on_libnvjitlink_dev:

@divyegala
Copy link
Member Author

@KyleFromNVIDIA thanks Kyle for your review! Unfortunately, this PR is still in draft state and needs a lot of work and benchmarking to be done. However, I will attempt to answer your review during the dev process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request non-breaking Introduces a non-breaking change
Development

Successfully merging this pull request may close these issues.

2 participants