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

Allow dlopen(…) to accept file://… so that values from import.meta.resolve(…) can be directly passed in again #9878

Closed
lgarron opened this issue Apr 3, 2024 · 2 comments · Fixed by #9883
Labels
bug Something isn't working

Comments

@lgarron
Copy link
Contributor

lgarron commented Apr 3, 2024

What version of Bun is running?

1.1.0

What platform is your computer?

Darwin 23.4.0 arm64 arm

What steps can reproduce the bug?

// The `await` can be removed in `bun` 1.1.0
dlopen(await import.meta.resolve(`./ffi.dylib`), {
  // …
})

What is the expected behavior?

The code works, as it did in bun 1.0.34.

What do you see instead?

Thanks to #5827 import.meta.resolve is now synchronous (which is fantastic for interop) and returns file:// URLs as strings to match node (certainly handy to have consistency!).

Unfortunately, dlopen currently fails when given a file:// scheme, so these changes broke some code. The workaround for this is not entirely obvious, as it requires re-parsing the resolved path as a URL:

// The `await` can be removed in `bun` 1.1.0
dlopen(new URL(await import.meta.resolve(`./ffi.dylib`)).pathname, {
  // …
})

It errors with:

ERR_DLOPEN_FAILED: Failed to open library. This is usually caused by a missing library or an invalid library path.

Since dlopen(…) isn't matching the functionality of an existing node API, I'd love to see it accept file URLs so that resolved path cans be passed to it intuitively and ergonomically. I think this is particularly valuable because:

  • This would make it super straightforward to write maximally portable code by writing the obvious, concise thing.
  • By contrast, the existing error message might make someone go double-check and triple-check for the existence of the relevant file, only to be left scratching their heads. ("The path is valid! bun is handing me a resolved path to a file that definitely exists!") It's a real red herring, and even with a lot of expertise it took me a few minutes to figure out what was going on.

Additional information

No response

@lgarron lgarron added the bug Something isn't working label Apr 3, 2024
Jarred-Sumner added a commit that referenced this issue Apr 3, 2024
Jarred-Sumner added a commit that referenced this issue Apr 3, 2024
Co-authored-by: Jarred Sumner <[email protected]>
@lgarron
Copy link
Contributor Author

lgarron commented Apr 5, 2024

Thanks for implementing this so quickly, this is really nice to have! 🤩

@lgarron
Copy link
Contributor Author

lgarron commented Apr 5, 2024

Actually, it looks like this isn't quite working yet?

git clone https://github.com/cubing/twsearch && cd twsearch
git checkout 6279f4abbee4ac8941d7cd5648c6811eb283cb0a
cargo install cargo-run-bin # Requires `cargo` to be installed already.
make build-rust-ffi
bun run "src/rs-ffi/test/js_test.ts"

bun v1.1.1 on macOS still errors with ERR_DLOPEN_FAILED: Failed to open library. This is usually caused by a missing library or an invalid library path.

Should I file a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant