Skip to content

fix(libsql-sys): support non-Unix when rusqlite feature is disabled#1971

Merged
penberg merged 1 commit intotursodatabase:mainfrom
CBenoit:fix/libsql-sys/non-crossplatform-code
Mar 20, 2025
Merged

fix(libsql-sys): support non-Unix when rusqlite feature is disabled#1971
penberg merged 1 commit intotursodatabase:mainfrom
CBenoit:fix/libsql-sys/non-crossplatform-code

Conversation

@CBenoit
Copy link
Copy Markdown
Contributor

@CBenoit CBenoit commented Feb 27, 2025

Currently the codepath used when rusqlite is disabled, is Unix-platform-dependent.

For instance, here is the error we compiling from Windows:

error[E0433]: failed to resolve: could not find `unix` in `os`
   --> C:\Users\runneradmin\.cargo\git\checkouts\libsql-311658d335deb3b1\9b04f1d\libsql-sys\src\connection.rs:276:26
    |
276 |             use std::os::unix::ffi::OsStrExt;
    |                          ^^^^ could not find `unix` in `os`
    |
note: found an item that was configured out
   --> /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14\library\std\src\os\mod.rs:27:9
note: the item is gated here
   --> /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14\library\std\src\os\mod.rs:19:1
note: found an item that was configured out
   --> /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14\library\std\src\os\mod.rs:65:9
note: the item is gated here
   --> /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14\library\std\src\os\mod.rs:64:1

error[E0599]: no method named `as_bytes` found for reference `&OsStr` in the current scope
   --> C:\Users\runneradmin\.cargo\git\checkouts\libsql-311658d335deb3b1\9b04f1d\libsql-sys\src\connection.rs:277:73
    |
277 |             let path = std::ffi::CString::new(path.as_ref().as_os_str().as_bytes())
    |                                                                         ^^^^^^^^
    |
help: there is a method `as_encoded_bytes` with a similar name
    |
277 |             let path = std::ffi::CString::new(path.as_ref().as_os_str().as_encoded_bytes())
    |                                                                         ~~~~~~~~~~~~~~~~

Some errors have detailed explanations: E0433, E0599.
For more information about an error, try `rustc --explain E0433`.
error: could not compile `libsql-sys` (lib) due to 2 previous errors

This patch is adding a platform-independent branch.

The database path is expected to be UTF-8 by the libsql native library. However, in the real world, all Unix paths are not necessarily UTF-8. For this reason, I understand why the Unix codepath is attempting a direct conversion from OsString to CString.
However, it’s not possible to do something similar on other platforms. For these platforms, we are enforcing correct UTF-8 using to_str. At least, it should work reasonably well in most cases.

@CBenoit CBenoit force-pushed the fix/libsql-sys/non-crossplatform-code branch from 54fbc95 to a822963 Compare February 27, 2025 11:26
@CBenoit
Copy link
Copy Markdown
Contributor Author

CBenoit commented Mar 18, 2025

Hello @penberg
I’m not sure how my change is impacting the "Go bindings tests" job.

@penberg
Copy link
Copy Markdown
Collaborator

penberg commented Mar 19, 2025

@CBenoit Please rebase on top of main, you're just getting hit by unrelated bug that is fixed in main.

Currently the codepath used when rusqlite is disabled, is
Unix-platform-dependent. This patch is adding a platform-independent
branch. The database path is expected to be UTF-8 by the libsql native
library. However, in the real world, all Unix paths are not necessarily
UTF-8. For this reason, I understand why the Unix codepath is attempting
a direct conversion from OsString to CString. However, it’s not possible
to do something similar on other platforms. For these platforms, we
are enforcing correct UTF-8 using to_str. At least, it should work
reasonably well in most cases.
@CBenoit CBenoit force-pushed the fix/libsql-sys/non-crossplatform-code branch from 22a1565 to 8ea257a Compare March 20, 2025 00:25
@CBenoit
Copy link
Copy Markdown
Contributor Author

CBenoit commented Mar 20, 2025

@penberg I see! Done!

@penberg penberg added this pull request to the merge queue Mar 20, 2025
Merged via the queue into tursodatabase:main with commit 687d23e Mar 20, 2025
19 checks passed
@CBenoit CBenoit deleted the fix/libsql-sys/non-crossplatform-code branch March 20, 2025 12:37
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