Skip to content

BUILD: improved configure status reporting#1239

Merged
janjust merged 3 commits intoopenucx:masterfrom
ikryukov:nvml_tlcuda_fix
Jan 6, 2026
Merged

BUILD: improved configure status reporting#1239
janjust merged 3 commits intoopenucx:masterfrom
ikryukov:nvml_tlcuda_fix

Conversation

@ikryukov
Copy link
Copy Markdown
Collaborator

What

Improved configuration process and detection
Example: vLLM image ships without nvml and it was hard to debug why the CUDA is not working + NVLS were enabled in that case

Why ?

  • Clearer feedback - users immediately understand why TL CUDA is disabled
  • Faster builds when NVML is missing - skips unnecessary NVCC architecture generation

@ikryukov ikryukov changed the title BUILD: improved status reporting BUILD: improved configure status reporting Dec 15, 2025
@ikryukov ikryukov self-assigned this Dec 15, 2025
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Dec 15, 2025

Greptile Summary

improved configuration status reporting to provide clearer feedback when NVML is missing and TL CUDA cannot be enabled

Key improvements:

  • provides early warning when NVML headers/library missing, explaining why TL CUDA disabled
  • skips expensive NVCC architecture generation when TL CUDA unavailable (optimization for environments like vLLM without NVML)
  • adds detailed status messages throughout configuration process

Critical issue:

  • src/components/tl/cuda/configure.m4 contains invalid AS_IF syntax at lines 21-25 that will cause autoconf parsing errors

Confidence Score: 1/5

  • this PR contains a critical syntax error that will break the build configuration
  • the AS_IF macro usage at lines 21-25 in src/components/tl/cuda/configure.m4 has invalid m4 syntax - attempting to chain multiple test conditions as separate arguments rather than using nested AS_IF for elif logic. this will cause autoconf to fail or produce incorrect configure scripts. the rest of the changes are good improvements for status reporting
  • pay close attention to src/components/tl/cuda/configure.m4 - contains syntax error that must be fixed

Important Files Changed

Filename Overview
config/m4/cuda.m4 improved NVML detection feedback and conditionally skips expensive NVCC gencode generation when TL CUDA unavailable
src/components/tl/cuda/configure.m4 added detailed status messages but contains critical AS_IF syntax error that will break autoconf

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@ikryukov
Copy link
Copy Markdown
Collaborator Author

/build

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jan 5, 2026

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

Signed-off-by: Ilya Kryukov <ikryukov@nvidia.com>
Signed-off-by: Ilya Kryukov <ikryukov@nvidia.com>
Signed-off-by: Ilya Kryukov <ikryukov@nvidia.com>
@Sergei-Lebedev
Copy link
Copy Markdown
Contributor

/build

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. src/components/tl/cuda/configure.m4, line 21-25 (link)

    syntax: incorrect AS_IF syntax - autoconf will not parse this correctly

    AS_IF takes 3 arguments: condition, then-action, else-action. this code attempts to chain multiple test conditions which is invalid m4 syntax. needs nested AS_IF for elif logic

    also added quotes around variables to match codebase style in config/m4/cuda.m4

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@janjust janjust merged commit 429a404 into openucx:master Jan 6, 2026
11 checks 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