Skip to content

internal/discover: use explicit driver version for matching libraries #1578

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

Open
Ronitsabhaya75 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

internal/discover: use explicit driver version for matching libraries #1578

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1c13bd
internal/discover: use explicit driver version for matching libraries
Ronitsabhaya75 Jan 14, 2026
1ac460c
internal/discover: restore missing fields in graphicsDriverLibraries
Ronitsabhaya75 Jan 14, 2026
66e907b
internal/discover: use driverVersion directly in graphics library dis…
Ronitsabhaya75 Jan 14, 2026
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
26 changes: 15 additions & 11 deletions internal/discover/graphics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,21 @@ func newVulkanConfigsDiscover(logger logger.Interface, driver *root.Driver) Disc

type graphicsDriverLibraries struct {
Discover
logger logger.Interface
hookCreator HookCreator
Comment on lines -112 to -113
Copy link
Member

@elezar elezar Jan 14, 2026

Choose a reason for hiding this comment

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

Question: Why were these members removed?

Copy link
Author

@Ronitsabhaya75 Ronitsabhaya75 Jan 14, 2026

Choose a reason for hiding this comment

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

Question: Why were these members removed?

@elezar I accidentally removed those members lemme revert those. I'm really sorry for this mistake

// driverVersion is the version of the driver that is being used.
driverVersion string
logger logger.Interface
hookCreator HookCreator
Comment on lines +112 to +115
Copy link
Member

@elezar elezar Jan 16, 2026

Choose a reason for hiding this comment

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

Suggested change
// driverVersion is the version of the driver that is being used.
driverVersion string
logger logger.Interface
hookCreator HookCreator
logger logger.Interface
hookCreator HookCreator
// driverVersionSuffix is the version of the driver that is being used
// prefixed with a '.'
driverVersionSuffix string

}

var _ Discover = (*graphicsDriverLibraries)(nil)

func newGraphicsLibrariesDiscoverer(logger logger.Interface, driver *root.Driver, hookCreator HookCreator) (Discover, error) {
cudaVersionPattern, err := driver.Version()
driverVersion, err := driver.Version()
if err != nil {
return nil, fmt.Errorf("failed to get driver version: %w", err)
}
// We use the driver version as a pattern for matching libraries.
// This pattern is used to identify libraries that are part of the driver.
cudaLibRoot, err := driver.GetDriverLibDirectory()
if err != nil {
return nil, fmt.Errorf("failed to get libcuda.so parent directory: %w", err)
Expand All @@ -140,8 +144,8 @@ func newGraphicsLibrariesDiscoverer(logger logger.Interface, driver *root.Driver
// * libnvidia-allocator.so.RM_VERSION
// * libnvidia-vulkan-producer.so.RM_VERSION
// but need to be handled for the legacy case too.
"libnvidia-allocator.so." + cudaVersionPattern,
"libnvidia-vulkan-producer.so." + cudaVersionPattern,
"libnvidia-allocator.so." + driverVersion,
"libnvidia-vulkan-producer.so." + driverVersion,
},
)

Expand All @@ -156,14 +160,15 @@ func newGraphicsLibrariesDiscoverer(logger logger.Interface, driver *root.Driver
driver.Root,
[]string{
"nvidia_drv.so",
"libglxserver_nvidia.so." + cudaVersionPattern,
"libglxserver_nvidia.so." + driverVersion,
},
)

return &graphicsDriverLibraries{
Discover: Merge(libraries, xorgLibraries),
logger: logger,
hookCreator: hookCreator,
Discover: Merge(libraries, xorgLibraries),
logger: logger,
hookCreator: hookCreator,
driverVersion: driverVersion,
Comment on lines +168 to +171
Copy link
Member

@elezar elezar Jan 16, 2026

Choose a reason for hiding this comment

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

Suggested change
Discover: Merge(libraries, xorgLibraries),
logger: logger,
hookCreator: hookCreator,
driverVersion: driverVersion,
Discover: Merge(libraries, xorgLibraries),
logger: logger,
hookCreator: hookCreator,
driverVersionSuffix: "." + driverVersion,

}, nil
}

Expand Down Expand Up @@ -231,8 +236,7 @@ func (d graphicsDriverLibraries) Hooks() ([]Hook, error) {

// isDriverLibrary checks whether the specified filename is a specific driver library.
func (d graphicsDriverLibraries) isDriverLibrary(filename string, libraryName string) bool {
// TODO: Instead of `.*.*` we could use the driver version.
pattern := strings.TrimSuffix(libraryName, ".") + ".*.*"
pattern := strings.TrimSuffix(libraryName, ".") + "." + d.driverVersion
Copy link
Member

@elezar elezar Jan 16, 2026

Choose a reason for hiding this comment

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

Suggested change
pattern := strings.TrimSuffix(libraryName, ".") + "." + d.driverVersion
pattern := libraryName + d.driverVersionSuffix

Note that we never call this function with a libraryName ending in ., so we can remove the additional TrimSuffix to further simplify this.

Copy link
Author

@Ronitsabhaya75 Ronitsabhaya75 Jan 16, 2026

Choose a reason for hiding this comment

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

@elezar thank you for the review I'll sure work on the commits and once im free from work

match, _ := filepath.Match(pattern, filename)
return match
}
Expand Down