Conversation
The esaxx-rs transitive dependency (pulled in via tokenizers' default features) unconditionally links against the static CRT, causing linking conflicts on Windows. Disabling default features eliminates this native code dependency while retaining needed functionality via the onig feature.
Use forked llama-cpp-rs with Windows linking fixes. Also bumps cc crate to satisfy the fork's requirements.
Use a thin vendor/v8 shim that re-exports v8-goose (published on crates.io), which contains a fix for detecting crt-static via CARGO_CFG_TARGET_FEATURE instead of CARGO_FEATURE_CRT_STATIC. This avoids the slow git dependency approach (rusty_v8 submodule fetches) while correctly handling CRT linkage on Windows.
Both v8 and llama-cpp-rs statically link identical std::exception_ptr symbols from the C++ runtime. Use /FORCE:MULTIPLE to let the linker accept these duplicate definitions.
|
Nice one! I did not find we set CARGO_CFG_TARGET_FEATURE, so we use dynamic for both V8 and llama cpp v2? Just to make sure my understanding is correct. Also maybe we can add a step in CI to compile with windows runner so that next time we can have earlier feedback, could be a follow PR. |
We leave it at the default, which is dynamic CRT on Windows. This is unchanged from before this PR; this PR is just rationalising how all our dependencies handle CRT selection so that they all match, since you can't mix static and dynamic CRT. The way to change that to static, if we ever wanted to change it, would be to add "-Ctarget-feature=+crt-static" to RUSTFLAGS (in env var or in
Yes, I think this is a good idea - I will take a look at this. |
|
Interesting! BTW |
The code mode extension (allowing the agent to write code that calls tools, rather than tying tool calls together with inference) uses v8, and it's currently non-optional. I can take a look at making that a cargo feature if you are OK with building without code mode. |
|
Due to the nature of OBS which disallows downloading binaries during build time, we need to build v8 from source, which might take a long time. Having it optional is appreciated. But, let me see how long it takes to build v8 now. |
|
@kskarthik I opened #7567 to make it optional |
3 participants
Fixes several linking conflicts when building on Windows MSVC targets.
Changes
Disable default features on
tokenizers— Theesaxx-rstransitive dependency (via tokenizers' default features) has optional native code that unconditionally links against the static CRT, causing conflicts. Retains needed functionality via theonigfeature.Switch
llama-cpp-2to fork — Uses a git dependency on jh-block/llama-cpp-rs (goose-patchesbranch, changes here) which contains a fix to detect whether the static-crt target feature is enabled rather than assuming dynamic.Patch
v8crate viav8-goose— The upstreamv8crate detects CRT linkage usingCARGO_FEATURE_CRT_STATIC, which is testing a non-existent crate feature instead of correctly checking the target feature. The patched version correctly usesCARGO_CFG_TARGET_FEATUREinstead. Implemented as a thinvendor/v8shim that re-exportsv8-goose(published on crates.io), because a git dependency would pull in all the v8 submodules which are enormous.Allow duplicate symbols on Windows MSVC — Both
v8andllama-cpp-rsstatically link identicalstd::exception_ptrsymbols from the C++ runtime. Adds/FORCE:MULTIPLElinker flag for MSVC targets to accept these duplicate definitions.Testing
I've tested debug and release builds on my x86_64 Windows machine; they build successfully and function correctly.