chore: make justfile fully compatible with Windows/PowerShell#100
chore: make justfile fully compatible with Windows/PowerShell#100MusabYK wants to merge 2 commits into
Conversation
j-kon
left a comment
j-kon
left a comment
There was a problem hiding this comment.
Thanks for taking this on, @MusabYK. This is a helpful improvement for Windows contributors, and I like that the scope stays focused on the Justfile.
I was not able to run just locally on my machine because it is not installed here, so one thing I would like to confirm before this is merged: have you tested the updated recipes on Windows/PowerShell, especially just list, just format, just clean, and just generate-bindings?
If those pass on Windows, this looks aligned with #97 and #98 to me.
| generate-bindings: | ||
| bash ./scripts/generate_bindings.sh | ||
| {{ if os() == "windows" { | ||
| "cd native; cargo build --profile dev; cargo run --profile dev --bin uniffi-bindgen -- generate --library target\\debug\\bdk_dart_ffi.dll --language dart --config uniffi.toml --out-dir ..\\lib\\" |
There was a problem hiding this comment.
Small behavior note: PowerShell ; always runs the next command, so if cd native or cargo build fails, the following command can still run. Could we keep this fail-fast like the original script behavior?
There was a problem hiding this comment.
Yeah, great point I'll look into fixing that.
| {{ if os() == "windows" { | ||
| "Start-Process https://github.com/bitcoindevkit/bdk-dart" | ||
| } else { | ||
| "open https://github.com/bitcoindevkit/bdk-dart" |
There was a problem hiding this comment.
Small cross-platform note: this still uses macOS open for every non-Windows OS. Since this PR is improving cross-platform Justfile support, should we use xdg-open on Linux and keep open only for macOS?
There was a problem hiding this comment.
Thats a good idea, i assumed the current justfile support Linux as well, I just test all the just commands on my WSL, they all passed except the just repo command.
I'll look into that as well. Thanks!
- Replace manual directory switching with native '[working-directory]' attribute.
- Add fail-fast error checking ('if (-not $?)') to Windows bindings generation.
- Standardize Windows file paths to use forward slashes.
|
Hi @j-kon I have applied the requested changes and refactored the
|
|
Thanks @MusabYK, I checked the latest update and the two review points look addressed now. The |
| [doc("Build native library and regenerate bindings.")] | ||
| generate-bindings: | ||
| bash ./scripts/generate_bindings.sh | ||
| {{ if os() == "windows" { |
There was a problem hiding this comment.
This multi-line interpolation has the same parser issue as the generate-bindings recipe: just --list --unsorted fails with error: Unterminated interpolation before any recipe can run.
3 participants
Description
This PR addresses #97 #98 and several platform-specific compatibility bugs in the justfile. Running certain recipes natively on Windows (via PowerShell) caused execution panics or syntax errors due to hardcoded Unix commands (rm -rf, open) and bash-specific operators (&&).
Changes Implemented
set unstable := true: Allow experimental features like [group(...)]set shell := ["sh", "-c"]: Default shell for Mac and Linuxset windows-shell := ["powershell.exe", "-Command"]Specific override ONLY for Windows usersgenerate-bindings: Fixed a crash where bash was explicitly invoked. Refactored it to conditionally cd into native and invoke the windows specific binding generation commands to correctly generate bindings forbdk_dart_ffi.dll.clean: Conditionally swapped Unix rm -rf for PowerShell's nativeRemove-Item -Recurse -Forcewhen executing on a windows host, preventing command parsing failures.repo: Conditionally replaced theopencommand with WindowsStart-Processfor launching the GitHub repository in the default browser.demo-analyze&demo-test: Conditionally replaced the chaining operator&&with the PowerShell's separator;for Windows environments.Investigation & findings
After fixing the Justfile to support Windows, I ran the
just cicommand, the project builds and the test suite starts successfully, but when thejust cireachesdemo-testseveral demo tests fail with the following error.Error:
PathAccessException: Deletion failed, path = 'C:\Users...\wallet_service_test_xxxxx'
(OS Error: The process cannot access the file because it is being used by another process, errno = 32)
dart:io FileSystemEntity.delete
package:bdk_demo/core/utils/wallet_storage_paths.dart 57:17 WalletStoragePaths.deleteFallbackWalletData
package:bdk_demo/services/wallet_service.dart 617:32 WalletService._reseedWalletToFallbackSqlite
Recommendation
The PathAccessException error should be addressed through a separate PR, once its fixed, the
just cishould pass on Windows just as it does on Linux and macOS.