fix(windows): compile memset_shim to real .o via get_compiler() #121

sarman · 2026-06-19T20:23:03Z

sarman commented

2026-06-19 20:23:03 +00:00

Summary

compile("memset_shim") creates libmemset_shim.a but the previous code passed OUT_DIR/memset_shim.o to the linker — a file that never exists, causing the ld: cannot find error
Switched to cc::Build::new().get_compiler().to_command() to compile memset_s_shim.c directly to a .o file at a known OUT_DIR path
Positional object files are always included unconditionally by the linker, which also resolves the static-archive symbol-ordering problem (bare -l flags only pull symbols that are already undefined at that point in the link)

Test plan

Windows MinGW CI build passes (x86_64-pc-windows-gnu target)
memset_explicit symbol is resolved at link time (no undefined symbol error)
Linux and macOS builds unaffected (the block is guarded by target_os == "windows" && target_env == "gnu")

## Summary - `compile("memset_shim")` creates `libmemset_shim.a` but the previous code passed `OUT_DIR/memset_shim.o` to the linker — a file that never exists, causing the `ld: cannot find` error - Switched to `cc::Build::new().get_compiler().to_command()` to compile `memset_s_shim.c` directly to a `.o` file at a known `OUT_DIR` path - Positional object files are always included unconditionally by the linker, which also resolves the static-archive symbol-ordering problem (bare `-l` flags only pull symbols that are already undefined at that point in the link) ## Test plan - [ ] Windows MinGW CI build passes (`x86_64-pc-windows-gnu` target) - [ ] `memset_explicit` symbol is resolved at link time (no undefined symbol error) - [ ] Linux and macOS builds unaffected (the block is guarded by `target_os == "windows" && target_env == "gnu"`)

sarman self-assigned this 2026-06-19 20:23:03 +00:00

sarman added 1 commit 2026-06-19 20:23:04 +00:00

fix(windows): compile memset_shim to real .o via get_compiler() to fix MinGW link

Test / frontend-tests (pull_request) Successful in 1m46s

Details

Test / frontend-typecheck (pull_request) Successful in 1m58s

Details

PR Review Automation / review (pull_request) Successful in 4m5s

Details

Test / rust-fmt-check (pull_request) Successful in 13m11s

Details

Test / rust-clippy (pull_request) Successful in 14m23s

Details

Test / rust-tests (pull_request) Successful in 15m56s

Details

d0a985a53b

compile("memset_shim") produces libmemset_shim.a, not memset_shim.o, so the
previous cargo:rustc-link-arg pointed at a file that never exists. Switching to
get_compiler().to_command() compiles memset_s_shim.c directly to a .o file at
a known OUT_DIR path and passes it as a positional linker arg. A positional .o
is always included unconditionally, which also resolves the archive-extraction
ordering issue where -l flags only pull symbols that are already undefined at
that point in the link command.

sarman reviewed 2026-06-19 20:27:09 +00:00

sarman left a comment

Automated PR Review (qwen3.5-122b-think via liteLLM):\n\n

Summary

The PR addresses a Windows MinGW linking issue by compiling the memset shim directly to an object file instead of using compile(). However, there's a significant concern about cross-compilation configuration that could cause the build to fail on Windows targets.

Findings

[WARNING] src-tauri/build.rs:27 - Cross-compilation compiler may not be properly configured for MinGW
Evidence: let compiler = cc::Build::new().get_compiler();
The cc::Build struct is not configured with the target triple or compiler for MinGW cross-compilation. When building for x86_64-pc-windows-gnu, the compiler should be x86_64-w64-mingw32-gcc, but get_compiler() may return the host compiler instead. The cc crate respects environment variables like CC_x86_64_pc_windows_gnu, but only if Build::target() is properly set.

Fix: Configure the cc::Build with the target before getting the compiler:
```
let compiler = cc::Build::new()
    .target(target_os.as_str())
    .env("TARGET", "x86_64-pc-windows-gnu")
    .get_compiler();
```
Or better, use the cc crate's build method which handles cross-compilation automatically:
```
cc::Build::new()
    .file("memset_s_shim.c")
    .define("WIN32", None)
    .define("__WIN32__", None)
    .compile("memset_shim");
```
[SUGGESTION] src-tauri/build.rs:27 - Error handling could be more informative
Evidence: .expect("failed to invoke C compiler for memset_s_shim.c");
While not a blocker, adding more context to the error message (like the actual command or stderr output) would help debug cross-compilation issues.

Verdict: APPROVE WITH COMMENTS

The PR addresses the core issue correctly (using positional object files instead of static library), but the cross-compilation configuration should be verified. The cc crate typically handles target detection via environment variables, so this may work in practice, but explicit configuration would be more robust.

Automated PR Review (qwen3.5-122b-think via liteLLM):\n\n **Summary** The PR addresses a Windows MinGW linking issue by compiling the memset shim directly to an object file instead of using `compile()`. However, there's a significant concern about cross-compilation configuration that could cause the build to fail on Windows targets. **Findings** - [WARNING] src-tauri/build.rs:27 - Cross-compilation compiler may not be properly configured for MinGW Evidence: `let compiler = cc::Build::new().get_compiler();` The `cc::Build` struct is not configured with the target triple or compiler for MinGW cross-compilation. When building for `x86_64-pc-windows-gnu`, the compiler should be `x86_64-w64-mingw32-gcc`, but `get_compiler()` may return the host compiler instead. The `cc` crate respects environment variables like `CC_x86_64_pc_windows_gnu`, but only if `Build::target()` is properly set. Fix: Configure the `cc::Build` with the target before getting the compiler: ```rust let compiler = cc::Build::new() .target(target_os.as_str()) .env("TARGET", "x86_64-pc-windows-gnu") .get_compiler(); ``` Or better, use the `cc` crate's build method which handles cross-compilation automatically: ```rust cc::Build::new() .file("memset_s_shim.c") .define("WIN32", None) .define("__WIN32__", None) .compile("memset_shim"); ``` - [SUGGESTION] src-tauri/build.rs:27 - Error handling could be more informative Evidence: `.expect("failed to invoke C compiler for memset_s_shim.c");` While not a blocker, adding more context to the error message (like the actual command or stderr output) would help debug cross-compilation issues. **Verdict**: APPROVE WITH COMMENTS The PR addresses the core issue correctly (using positional object files instead of static library), but the cross-compilation configuration should be verified. The `cc` crate typically handles target detection via environment variables, so this may work in practice, but explicit configuration would be more robust.

sarman merged commit ec14409fd5 into beta

2026-06-19 20:42:29 +00:00

sarman deleted branch fix/cargo-config-sodium

2026-06-19 20:42:29 +00:00

sarman referenced this issue from a commit

2026-06-19 20:42:30 +00:00

Merge pull request 'fix(windows): compile memset_shim to real .o via get_compiler()' (#121) from fix/cargo-config-sodium into beta

Sign in to join this conversation.

No reviewers