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

all: instrument library loading #1328

Merged
merged 1 commit into from
Jan 1, 2025
Merged

Conversation

TristanCrawford
Copy link
Contributor

Potential fix for #1327

TL;DR This instruments the calls to DynLib.open to log exactly what libraries are missing and returns a consistent LibraryNotFound error.

image


As for the issue I was running into earlier: The open call in src/sysgpu/vulkan.zig was not catching and returning in the same manner as the other library loads, which is why I was getting that non-descript FileNotFound error.

One caveat is that some libraries that aren't really required will still log an error, but it's definitely better than the ambiguous errors from before imo.

There may be a better name and/or a better place to put the loadLibrary function, so I'm open to shifting some things around if y'all see fit. Cheers!


  • By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

@TristanCrawford TristanCrawford force-pushed the main branch 2 times, most recently from 40dfe88 to 91f825f Compare December 31, 2024 22:38
src/core/util.zig Outdated Show resolved Hide resolved
src/core/util.zig Outdated Show resolved Hide resolved
@emidoots
Copy link
Member

Two small comments, looks good otherwise! Thanks for improving this!

@emidoots emidoots merged commit 41ddd22 into hexops:main Jan 1, 2025
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.

2 participants