Fix libsql-ffi build on Windows #2005
Merged
+9
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 9, 2025
libsql-ffi build on Windows has been intermittently broken for a while due _build.rs_ calling `cp` which is not present on Windows. Timeline: - 2024-08-10: Issue tursodatabase#1657 was opened. - 2024-11-14: tursodatabase#1791 was merged. First fix using `#[cfg(windows)]`. - 2024-11-15: tursodatabase#1791 reverted in 9499c3c for breaking libsql-js on macOS. - 2024-12-04: 9499c3c was reverted since tursodatabase#1791 was fine and did not cause the macOS problem. Windows build works again. - 2025-03-17: da54c1b breaks Windows build. - 2025-03-21: 0.9.1 is released with broken Windows build. - 2025-03-27: This commit fixes the bug introduced in da54c1b. This commit fixes a bug introduced by da54c1b. `Command::status()?` was propogating the error so the fallback, `fs::copy`, was never reached. This is now fixed so libsql-ffi builds again on Windows. Closes tursodatabase#1657
|
I've rebased on main branch, rev 483a150. Please take a look if you have time :) |
avinassh
approved these changes
Apr 16, 2025
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
libsql-ffi/build.rs:79
- Switching from using the '?' operator to a match statement now correctly prevents early error propagation from the 'cp' command, ensuring the fallback is executed. Consider adding a brief comment to clarify this handling for future maintainers.
match Command::new("cp")
libsql-ffi/build.rs:88
- The conditional fallback using 'io::ErrorKind::InvalidInput' might be too narrow. Verify that this condition reliably indicates a directory copy, or consider broadening the error matching to better cover other possible error kinds for non-file sources.
Err(err) if err.kind() == io::ErrorKind::InvalidInput => copy_dir_all(from, to),
levydsa
approved these changes
Apr 17, 2025
|
Thanks for the patch @allan2! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
libsql-ffi build on Windows has been intermittently broken for a while due build.rs calling
cpwhich is not present on Windows.Timeline:
#[cfg(windows)].This commit fixes a bug introduced by da54c1b.
Command::status()?was propogating the error so the fallback,fs::copy, was never reached. This is now fixed so libsql-ffi builds again on Windows.Closes #1657