Skip to content
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.

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

Inject additional libraries for full X11 functionality #548

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

Conversation

elezar
Copy link
Member

@elezar elezar commented Jun 17, 2024

This change updates X11 library detection to ensure that this works for a wider range of driver installations.

This includes the changes from #490

@elezar
Copy link
Member Author

elezar commented Jun 17, 2024

@ehfd I realized that I hadn't cleaned up the changes from #490 prior to merging them.

I therefore had to revert them and created this PR to include all the relevant changes.

@tux-rampage your commits in the #490 were not signed off and as such you're not included as an author here. Feel free to push some signed-off commits here.

@ehfd
Copy link
Contributor

ehfd commented Jun 17, 2024

Separately, I see no issues and approve the code.

@elezar elezar self-assigned this Jun 17, 2024
Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

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

small typo

internal/discover/graphics.go Outdated Show resolved Hide resolved
This change updates X11 library detection to ensure that this
works for a wider range of driver installations.

Signed-off-by: Seungmin Kim <[email protected]>
Signed-off-by: Evan Lezar <[email protected]>
@ehfd
Copy link
Contributor

ehfd commented Jun 18, 2024

I think @tux-rampage might not have access to the branch.

Signed-off-by: tux-rampage <[email protected]>

This is the required string.

@elezar
Copy link
Member Author

elezar commented Jun 19, 2024

I think @tux-rampage might not have access to the branch.

Signed-off-by: tux-rampage <[email protected]>

This is the required string.

Yes, I would suppose that they don't have access. @tux-rampage feel free to create a PR targetting this branch indicating that you sign-off your work. Or coinfirm that I can add the string myself.

@ehfd
Copy link
Contributor

ehfd commented Jun 22, 2024

@elezar On a separate note, what would be the best way to detect the kernel driver version from inside the container, other than /proc/driver/nvidia/version, which is not available with WSL?

@elezar
Copy link
Member Author

elezar commented Jun 24, 2024

@elezar On a separate note, what would be the best way to detect the kernel driver version from inside the container, other than /proc/driver/nvidia/version, which is not available with WSL?

You mean the "RM_VERSION"? Here one would probably have to rely on an NVML call.

@ehfd
Copy link
Contributor

ehfd commented Jun 24, 2024

@elezar I did a bit more exploring, and it seems like the RM_VERSION of WSL is similar, but almost always different to the https://download.nvidia.com/XFree86/Linux-x86_64/ driver download page. Having issues with provisioning the nvidia_drv.so and libglxserver_nvidia.so X server libraries inside WSL + Container toolkit.

I'll open a new issue about this.

@ehfd
Copy link
Contributor

ehfd commented Jun 24, 2024

Continuing in #563

@ehfd
Copy link
Contributor

ehfd commented Jun 24, 2024

@tux-rampage Could you possibly approve this PR and indicate that Signed-off-by: tux-rampage <[email protected]> is good? We're unable to merge this.

This adds a function to return a path as a path relative to
the specified driver root.

Signed-off-by: Evan Lezar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants