From 0b95cb83d07f1e5fe11b05a64bfc0b57bf46ddbf Mon Sep 17 00:00:00 2001 From: elderorb Date: Sun, 16 Aug 2026 11:19:58 +0200 Subject: [PATCH 1/2] windows: keep the reviewed MSVC/Vulkan portability fixes only Keep the reviewed Windows/MSVC portability work and drop the now-obsolete branch bulk. Preserve the shared-library packaging/link repair, the header-free shared-library smoke test, the portable temp-path and sparse file helpers used by the MiniMax-H3 / KV-offload / C API tests, and the internal platform_compat shim under src/vllm/support. Route tests through the existing internal src include path, keep the env seam on the existing tests/support/test_env.h path, and update laguna-gen to use the minimal internal helper for the native Windows env override. Issue: #503 Identity: ENG-RELEASE-WINDOWS FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:GPT-5 [Codex] --- CMakeLists.txt | 7 +- docs/USAGE.md | 7 ++ examples/CMakeLists.txt | 1 + examples/laguna_gen/main.cpp | 5 +- .../model_loader/safetensors_reader.cpp | 7 +- src/vllm/support/platform_compat.h | 63 ++++++++++++++++ tests/CMakeLists.txt | 3 + tests/capi/test_capi.cpp | 18 +++-- tests/capi/test_dlopen.cpp | 71 +++++++++++++++---- tests/vllm/models/test_minimax_h3.cpp | 18 +++-- .../models/test_minimax_h3_video_fold.cpp | 23 +++--- tests/vllm/test_safetensors.cpp | 4 +- tests/vllm/v1/test_kv_offload_connector.cpp | 6 +- tests/vllm/v1/test_kv_offload_tiering.cpp | 5 +- 14 files changed, 186 insertions(+), 52 deletions(-) create mode 100644 src/vllm/support/platform_compat.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 0cefa2ddc..0d080eb56 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2316,6 +2316,7 @@ add_library(vllm_shared SHARED "${_vllm_shared_stub}") add_library(vllm::shared ALIAS vllm_shared) set_target_properties(vllm_shared PROPERTIES OUTPUT_NAME vllm + ARCHIVE_OUTPUT_NAME vllm_shared VERSION ${PROJECT_VERSION} SOVERSION ${PROJECT_VERSION_MAJOR} CXX_VISIBILITY_PRESET hidden @@ -2328,7 +2329,11 @@ target_include_directories(vllm_shared PUBLIC $) # Force-link the whole `vllm` archive (the C ABI + engine + the CPU-backend # static registrar) and inherit its PUBLIC deps (CUDA::cudart, Threads, ...). -target_link_libraries(vllm_shared PRIVATE vllm) +# On Windows the packaged shared target also needs the vendored BLAKE3 archive +# explicitly on its own link line; relying on the static archive's usage +# requirements is not sufficient once the C ABI DLL is assembled via +# /WHOLEARCHIVE. +target_link_libraries(vllm_shared PRIVATE vllm blake3_vendored) # Export only the C ABI: `vllm_*` stays global, everything else is localized. # UNLIKE the force-link guard above, `UNIX AND NOT APPLE` is CORRECT here: a # linker version script is a GNU-ld/ELF feature with no ld64 spelling (ld64 uses diff --git a/docs/USAGE.md b/docs/USAGE.md index 0c7c4035c..72c525e86 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -2691,6 +2691,13 @@ declarations in that header) suitable for `dlopen` / FFI / LocalAI integration. This line read `19` and `36` until 2026-08-17; both numbers were last true several ABI additions ago, and neither is derived by any gate. +On native Windows/MSVC, the shared-library packaging lane keeps the runtime DLL +name at `vllm` and gives the import/static archive the distinct name +`vllm_shared`, so one build tree can hold the shared C ABI package and the +static `vllm` archive without a filename collision. The same ABI smoke test +therefore resolves the exported symbols through `LoadLibraryA` / +`GetProcAddress` on Windows and `dlopen` / `dlsym` on POSIX. + ```c #include "vllm.h" diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 6ac37b929..35192ab32 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -20,6 +20,7 @@ vllm_cpp_set_warnings(deepseek-v4-gen) # driver (W5 RUNNABLE vehicle — 3-shard keep-quant load + stateless recompute). add_executable(laguna-gen laguna_gen/main.cpp) target_link_libraries(laguna-gen PRIVATE vllm::vllm) +target_include_directories(laguna-gen PRIVATE ${CMAKE_SOURCE_DIR}/src) vllm_cpp_set_warnings(laguna-gen) # kimi-linear-gen: the Kimi-Linear-48B-A3B golden token battery, a THIN CLIENT diff --git a/examples/laguna_gen/main.cpp b/examples/laguna_gen/main.cpp index 60dc71ae2..17ecd0314 100644 --- a/examples/laguna_gen/main.cpp +++ b/examples/laguna_gen/main.cpp @@ -32,6 +32,7 @@ #include "vllm/model_executor/model_loader/gguf_reader.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" #include "vllm/model_executor/models/laguna.h" +#include "vllm/support/platform_compat.h" #include "vllm/tokenizer/tokenizer.h" #include "vllm/transformers_utils/hf_config.h" #include "vt/backend.h" // vt::GetBackend / CreateQueue (--gpu: GEMMs on the GB10) @@ -201,7 +202,9 @@ int main(int argc, char** argv) { // s/tok (2×; 2.56 → 5.0 tok/s), coherent + near-tie. `setenv(...,0)` respects an // explicit `VT_NVFP4_FP4_NATIVE=0` override. Scoped to this Laguna driver (the // 27B/35B use the separate DirectD cutlass path, untouched). - setenv("VT_NVFP4_FP4_NATIVE", "1", 0); + if (std::getenv("VT_NVFP4_FP4_NATIVE") == nullptr) { + (void)vllm::support::SetEnvVar("VT_NVFP4_FP4_NATIVE", "1"); + } const std::string config_path = (fs::path(model) / "config.json").string(); const std::string tok_path = (fs::path(model) / "tokenizer.json").string(); std::fprintf(stderr, "[gen] NVFP4 safetensors dir %s\n", model.c_str()); diff --git a/src/vllm/model_executor/model_loader/safetensors_reader.cpp b/src/vllm/model_executor/model_loader/safetensors_reader.cpp index 9f4373723..539dc406a 100644 --- a/src/vllm/model_executor/model_loader/safetensors_reader.cpp +++ b/src/vllm/model_executor/model_loader/safetensors_reader.cpp @@ -22,6 +22,8 @@ #include +#include "vllm/support/platform_compat.h" + namespace vllm { namespace { @@ -280,10 +282,7 @@ namespace { #if !defined(_WIN32) long HostPageSize() { - static const long page = [] { - const long p = ::sysconf(_SC_PAGESIZE); - return p > 0 ? p : 4096; - }(); + static const long page = support::HostPageSize(); return page; } #endif diff --git a/src/vllm/support/platform_compat.h b/src/vllm/support/platform_compat.h new file mode 100644 index 000000000..340897fcd --- /dev/null +++ b/src/vllm/support/platform_compat.h @@ -0,0 +1,63 @@ +#pragma once + +#include +#include +#include + +#if defined(_WIN32) +#ifndef NOMINMAX +#define NOMINMAX +#endif +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#include +#else +#include +#endif + +namespace vllm::support { + +#if defined(_WIN32) + +inline long HostPageSize() { + SYSTEM_INFO system_info{}; + GetSystemInfo(&system_info); + return system_info.dwPageSize > 0 + ? static_cast(system_info.dwPageSize) + : 4096L; +} + +inline int CurrentProcessId() { return static_cast(::GetCurrentProcessId()); } + +inline int FileDescriptorFromFile(std::FILE* file) { return _fileno(file); } + +inline bool TruncateFile(int fd, std::uint64_t size) { return _chsize_s(fd, size) == 0; } + +inline bool SetEnvVar(const char* name, const char* value) { + return _putenv_s(name, value) == 0; +} + +#else + +inline long HostPageSize() { + const long page_size = ::sysconf(_SC_PAGESIZE); + return page_size > 0 ? page_size : 4096L; +} + +inline int CurrentProcessId() { return ::getpid(); } + +inline int FileDescriptorFromFile(std::FILE* file) { return ::fileno(file); } + +inline bool TruncateFile(int fd, std::uint64_t size) { + return ::ftruncate(fd, static_cast(size)) == 0; +} + +inline bool SetEnvVar(const char* name, const char* value) { + return ::setenv(name, value, 1) == 0; +} + +#endif + +} // namespace vllm::support diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 21214ac89..74a85211d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,9 @@ function(vllm_cpp_add_test name) # under tests/support/ as "support/.h". No header name under tests/ # collides with one under include/ or src/, so nothing else resolves here. target_include_directories(${name} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + # Some test-only portability helpers live under src/vllm/support/ on purpose: + # they are internal build shims, not installed public headers. + target_include_directories(${name} PRIVATE ${CMAKE_SOURCE_DIR}/src) if(TARGET vllm::sanitize_test_shared) target_link_libraries(${name} PRIVATE vllm::sanitize_test_shared vllm_test_main) else() diff --git a/tests/capi/test_capi.cpp b/tests/capi/test_capi.cpp index c031bd552..d9926675d 100644 --- a/tests/capi/test_capi.cpp +++ b/tests/capi/test_capi.cpp @@ -24,15 +24,15 @@ #include #include -#include - #include #include "capi/engine_handle.h" +#include "support/test_env.h" #include "vllm/config/device.h" #include "vllm/config/multimodal.h" #include "vllm/entrypoints/model_loader.h" #include "vllm/platforms/interface.h" +#include "vllm/support/platform_compat.h" #include "vllm/entrypoints/openai/serving_utils.h" #include "vllm/model_executor/models/qwen3_5_weights.h" #include "vllm/tokenizer/bpe.h" @@ -414,7 +414,7 @@ TEST_CASE("capi: vllm_complete_tokens matches the string-prompt completion (ABI // reports six zero-initialized buffer entries must not satisfy ABI v12. const int32_t expected_ids[6] = {22, 12, 14, 9, 13, 2}; for (int i = 0; i < 6; ++i) { - INFO("generated token index ", i); + CAPTURE(i); CHECK(out_tokens[i] == expected_ids[i]); } REQUIRE(via_tok.text != nullptr); @@ -1222,7 +1222,7 @@ TEST_CASE("capi: enable_jump_forward defaults to 0 and validates (ABI v10)") { TEST_CASE("capi: enable_jump_forward=on reaches the engine; default is inert (ABI v10)") { // Resolution reads VT_ENABLE_JUMP_FORWARD as an override; clear it so this // test asserts the FIELD's effect, not an ambient env override. - ::unsetenv("VT_ENABLE_JUMP_FORWARD"); + vllm_test::UnsetEnv("VT_ENABLE_JUMP_FORWARD"); const HfConfig c = MakeConfig(); // Default (nullopt): jump-forward resolves OFF — byte-identical to before v10. @@ -1697,8 +1697,12 @@ struct VideoFoldWorkspace { std::string root, fixture; VideoFoldWorkspace() { static int counter = 0; - root = "/tmp/vllm_capi_video_" + std::to_string(::getpid()) + "_" + - std::to_string(counter++); + root = + (std::filesystem::temp_directory_path() / + ("vllm_capi_video_" + + std::to_string(vllm::support::CurrentProcessId()) + "_" + + std::to_string(counter++))) + .string(); std::filesystem::create_directories(root); fixture = root + "/fixture"; minimax_h3_fold::WriteFoldFixture(fixture); @@ -1812,7 +1816,7 @@ TEST_CASE("capi v12: vllm_video_generate reproduces the pre-fold goldens") { for (int f = 0; f < 8; ++f) { char name[64]; std::snprintf(name, sizeof(name), "/frame_%06d.ppm", f); - INFO("frame ", f); + CAPTURE(f); CHECK(ReadAllBytes(out_dir + name) == ReadAllBytes(golden_dir + name)); } CHECK(ReadAllBytes(out_dir + "/audio.wav") == diff --git a/tests/capi/test_dlopen.cpp b/tests/capi/test_dlopen.cpp index 584f3563d..de595cbf9 100644 --- a/tests/capi/test_dlopen.cpp +++ b/tests/capi/test_dlopen.cpp @@ -16,16 +16,61 @@ #include +#if defined(_WIN32) +#include +#else #include +#endif #include #ifndef VLLM_SHARED_LIB_PATH -#error "VLLM_SHARED_LIB_PATH must be defined (path to the built libvllm.so)" +#error "VLLM_SHARED_LIB_PATH must be defined (path to the built shared library)" #endif namespace { +#if defined(_WIN32) +using SharedLibraryHandle = HMODULE; + +std::string LastSharedLibraryError() { + const DWORD error = GetLastError(); + return error == 0 ? std::string() : ("GetLastError=" + std::to_string(error)); +} + +SharedLibraryHandle OpenSharedLibrary(const char* path) { + return LoadLibraryA(path); +} + +void* LoadSymbol(SharedLibraryHandle handle, const char* name) { + return reinterpret_cast(GetProcAddress(handle, name)); +} + +bool CloseSharedLibrary(SharedLibraryHandle handle) { + return FreeLibrary(handle) != 0; +} +#else +using SharedLibraryHandle = void*; + +std::string LastSharedLibraryError() { + const char* error = dlerror(); + return error != nullptr ? std::string(error) : std::string(); +} + +SharedLibraryHandle OpenSharedLibrary(const char* path) { + return dlopen(path, RTLD_NOW | RTLD_LOCAL); +} + +void* LoadSymbol(SharedLibraryHandle handle, const char* name) { + return dlsym(handle, name); +} + +bool CloseSharedLibrary(SharedLibraryHandle handle) { + return dlclose(handle) == 0; +} +#endif + + // Function-pointer types for the ABI symbols we dlsym. These mirror the // declarations in vllm.h; a header-less consumer would type them by hand. using fn_version = const char* (*)(void); @@ -58,27 +103,23 @@ using fn_string_free = void (*)(char*); using fn_completion_free = void (*)(vllm_completion*); using fn_last_error = const char* (*)(void); -// Resolve `name` from `handle`; the returned pointer must be non-null (fails the -// test otherwise). Uses a union-free reinterpret through void* (POSIX-sanctioned -// for dlsym function pointers). template -Fn Sym(void* handle, const char* name) { - void* p = dlsym(handle, name); - INFO("dlsym(", name, ")"); - REQUIRE(p != nullptr); - return reinterpret_cast(p); +Fn Sym(SharedLibraryHandle handle, const char* name) { + void* symbol = LoadSymbol(handle, name); + INFO("resolve(", name, ")"); + REQUIRE(symbol != nullptr); + return reinterpret_cast(symbol); } } // namespace // ─── the packaging DoD: dlopen + dlsym every ABI symbol, drive header-free ──── -TEST_CASE("dlopen: libvllm.so resolves the whole C ABI by name and drives it") { - // (1) dlopen the built shared library (RTLD_NOW forces eager symbol binding — - // an unresolved symbol would fail here, proving the .so is self-contained). - void* lib = dlopen(VLLM_SHARED_LIB_PATH, RTLD_NOW | RTLD_LOCAL); - INFO("dlopen error: ", (dlerror() != nullptr ? dlerror() : "")); +TEST_CASE("shared library resolves the whole C ABI by name and drives it") { + SharedLibraryHandle lib = OpenSharedLibrary(VLLM_SHARED_LIB_PATH); + INFO("shared library load error: ", LastSharedLibraryError()); REQUIRE(lib != nullptr); + // (2) dlsym EVERY stable C ABI symbol by name — all must be non-null. auto p_version = Sym(lib, "vllm_version"); auto p_abi = Sym(lib, "vllm_abi_version"); @@ -143,5 +184,5 @@ TEST_CASE("dlopen: libvllm.so resolves the whole C ABI by name and drives it") { // p_engine_free on null is a no-op (exercises the free pointer safely). p_engine_free(nullptr); - CHECK(dlclose(lib) == 0); + CHECK(CloseSharedLibrary(lib)); } diff --git a/tests/vllm/models/test_minimax_h3.cpp b/tests/vllm/models/test_minimax_h3.cpp index 570fb44b3..349219e59 100644 --- a/tests/vllm/models/test_minimax_h3.cpp +++ b/tests/vllm/models/test_minimax_h3.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -26,8 +27,6 @@ #include #include #include -#include -#include #include #include #include @@ -50,6 +49,7 @@ #include "support/max_abs_diff.h" #include "vllm/model_executor/model_loader/gguf_dequant.h" #include "vllm/model_executor/model_loader/gguf_reader.h" +#include "vllm/support/platform_compat.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" #include "vllm/multimodal/qwen3vl_processor.h" #include "../gguf_builder.h" @@ -83,6 +83,8 @@ using vllm::ParseMiniMaxH3DitParams; namespace { +namespace fs = std::filesystem; + // --------------------------------------------------------------------------- // H3Rand — the exact mirror of the generator's deterministic stream // (scripts/gen-minimax-h3-goldens.py :: h3_rand). A per-tensor FNV-1a seed plus a @@ -560,7 +562,7 @@ std::map WriteMiniMaxH3ShardedDit( const std::set& omit_payload = {}) { REQUIRE(num_shards > 0); REQUIRE(entries.size() >= num_shards); - ::mkdir(dir.c_str(), 0755); + fs::create_directories(dir); std::map weight_map; std::vector> per_shard(num_shards); @@ -593,7 +595,7 @@ std::map WriteMiniMaxH3ShardedDit( uint64_t WriteMiniMaxH3SparseShardedRelease(const std::vector& specs, const std::string& dir, size_t num_shards) { REQUIRE(num_shards > 0); - ::mkdir(dir.c_str(), 0755); + fs::create_directories(dir); std::vector> per_shard(num_shards); std::map weight_map; for (size_t i = 0; i < specs.size(); ++i) { @@ -635,7 +637,10 @@ uint64_t WriteMiniMaxH3SparseShardedRelease(const std::vector(sizeof(n) + header.size() + offset)) == 0); + const auto declared_size = + static_cast(sizeof(n) + header.size() + offset); + REQUIRE(vllm::support::TruncateFile( + vllm::support::FileDescriptorFromFile(fh), declared_size)); std::fclose(fh); declared += offset; } @@ -656,7 +661,8 @@ void RemoveShardedDit(const std::string& dir, size_t num_shards) { std::remove((dir + "/" + ShardFileName(s, num_shards)).c_str()); } std::remove((dir + "/model.safetensors.index.json").c_str()); - ::rmdir(dir.c_str()); + std::error_code ec; + fs::remove(dir, ec); } } // namespace diff --git a/tests/vllm/models/test_minimax_h3_video_fold.cpp b/tests/vllm/models/test_minimax_h3_video_fold.cpp index fada8d9fc..902eed6fd 100644 --- a/tests/vllm/models/test_minimax_h3_video_fold.cpp +++ b/tests/vllm/models/test_minimax_h3_video_fold.cpp @@ -29,22 +29,23 @@ #include #include #include +#include #include #include #include -#include -#include - #include "vllm/entrypoints/openai/video_api.h" #include "vllm/model_executor/model_loader/gguf_reader.h" #include "vllm/model_executor/models/minimax_h3.h" #include "vllm/platforms/interface.h" // CurrentPlatform() — the seam device 1 resolves through +#include "vllm/support/platform_compat.h" #include "minimax_h3_video_fold_fixture.h" #include "vt/backend.h" namespace { +namespace fs = std::filesystem; + std::string ReadAll(const std::string& path) { std::ifstream in(path, std::ios::binary); REQUIRE_MESSAGE(in.good(), "cannot open ", path); @@ -58,18 +59,20 @@ struct FoldWorkspace { std::string root; FoldWorkspace() { static int counter = 0; - root = "/tmp/vllm_h3_video_fold_" + std::to_string(::getpid()) + "_" + - std::to_string(counter++); - ::mkdir(root.c_str(), 0755); + root = (fs::temp_directory_path() / + ("vllm_h3_video_fold_" + + std::to_string(vllm::support::CurrentProcessId()) + "_" + + std::to_string(counter++))) + .string(); + fs::create_directories(root); fixture = root + "/fixture"; minimax_h3_fold::WriteFoldFixture(fixture); } ~FoldWorkspace() { - // Best-effort cleanup; a leftover /tmp dir on abort is diagnosable, not + // Best-effort cleanup; a leftover temp dir on abort is diagnosable, not // harmful. - const std::string cmd = "rm -rf '" + root + "'"; - const int rc = std::system(cmd.c_str()); - (void)rc; + std::error_code ec; + fs::remove_all(root, ec); } std::string fixture; }; diff --git a/tests/vllm/test_safetensors.cpp b/tests/vllm/test_safetensors.cpp index 2de1df835..d9dfadff8 100644 --- a/tests/vllm/test_safetensors.cpp +++ b/tests/vllm/test_safetensors.cpp @@ -24,6 +24,7 @@ #include "vllm/model_executor/model_loader/read_only_file_mapping.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" +#include "vllm/support/platform_compat.h" namespace { @@ -585,8 +586,7 @@ TEST_CASE("safetensors temp fixtures isolate simultaneous caller names") { namespace { size_t HostPageSize() { - const long p = ::sysconf(_SC_PAGESIZE); - return p > 0 ? static_cast(p) : 4096; + return vllm::support::HostPageSize(); } // Resident set (KiB) of the /proc/self/smaps VMA that contains `addr` — the diff --git a/tests/vllm/v1/test_kv_offload_connector.cpp b/tests/vllm/v1/test_kv_offload_connector.cpp index 3790a13a6..1dee40703 100644 --- a/tests/vllm/v1/test_kv_offload_connector.cpp +++ b/tests/vllm/v1/test_kv_offload_connector.cpp @@ -23,10 +23,9 @@ #include #include -#include - #include "vllm/config/kv_transfer.h" #include "vllm/config/scheduler.h" +#include "vllm/support/platform_compat.h" #include "vllm/v1/core/sched/scheduler.h" #include "vllm/v1/kv_cache_interface.h" #include "vllm/v1/kv_offload/cache_identity.h" @@ -121,7 +120,8 @@ class TempDir { explicit TempDir(const std::string& tag) { static int c = 0; path_ = std::filesystem::temp_directory_path() / - ("vllmcpp_kvconn_" + tag + "_" + std::to_string(::getpid()) + "_" + + ("vllmcpp_kvconn_" + tag + "_" + + std::to_string(vllm::support::CurrentProcessId()) + "_" + std::to_string(c++)); std::filesystem::create_directories(path_); } diff --git a/tests/vllm/v1/test_kv_offload_tiering.cpp b/tests/vllm/v1/test_kv_offload_tiering.cpp index 60aa55155..c87d04c00 100644 --- a/tests/vllm/v1/test_kv_offload_tiering.cpp +++ b/tests/vllm/v1/test_kv_offload_tiering.cpp @@ -23,8 +23,7 @@ #include #include -#include - +#include "vllm/support/platform_compat.h" #include "vllm/v1/core/kv_cache_utils.h" #include "vllm/v1/kv_offload/base.h" #include "vllm/v1/kv_offload/cache_identity.h" @@ -42,7 +41,7 @@ class TempDir { explicit TempDir(const std::string& tag) { static int counter = 0; path_ = std::filesystem::temp_directory_path() / - ("vllmcpp_kvtier_" + tag + "_" + std::to_string(::getpid()) + "_" + + ("vllmcpp_kvtier_" + tag + "_" + std::to_string(vllm::support::CurrentProcessId()) + "_" + std::to_string(counter++)); std::filesystem::create_directories(path_); } From eb5ab03ab8e9bbbd61ad606cd7a63a2f83aa7d77 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 17 Aug 2026 20:44:33 +0000 Subject: [PATCH 2/2] fix(windows): keep the portability shim off the example ABI and out of every test target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review repairs on top of the reviewed MSVC subset. No behavior change on any path that runs today. `examples/laguna_gen` is reverted to `setenv(..., 0)` and loses the private `-I ${CMAKE_SOURCE_DIR}/src`. The explicit `getenv(...) == nullptr` guard it replaced that with was semantically identical, but reaching `src/vllm/support/platform_compat.h` made laguna_gen the first example to include a header that exists ONLY under `src/` — every prior breach on its `scripts/example-abi-allowlist.txt` row is an `include/` header, and the allowlist calls a private `-I` into `src/` "the deepest breach" where it records one for `quant_gemm_bench`. `scripts/check-surface-coverage.py` parses `#include` lines and is blind to `-I` flags, so no gate would have caught it. `laguna-gen` is not among the seven targets `scripts/build-windows-release.ps1:372-382` builds, so this bought nothing on Windows. `vllm_cpp_add_test` no longer grants `${CMAKE_SOURCE_DIR}/src` to every test. `tests/CMakeLists.txt` already carries 123 explicit per-target grants; a blanket one in the helper converts that deliberate opt-in into a repo-wide default. The five suites that actually reach `platform_compat.h` get it by name instead (`test_capi` already had one). No header name under `tests/` collides with one under `src/`, so this was not breaking anything — it was simply wider than the change needs. `SetEnvVar`'s empty-value divergence is documented rather than silently carried: `_putenv_s(name, "")` REMOVES the variable where POSIX `setenv(name, "", 1)` defines it empty. No caller passes an empty value, so normalising would be unexercised code; the comment records the contract and points at the same rule `tests/support/test_env.h` already states for the test-side seam. EXCEPTION, argued rather than waived: `documentation-checkpoint` refuses this commit because `examples/CMakeLists.txt` is in `USER_USAGE_FILES`, so any edit to it owes `docs/USAGE.md`. This commit REVERTS an example back to what `main` already has -- the net diff under `examples/` against `origin/main` is empty -- so there is no user-facing fact for `docs/USAGE.md` to gain, and writing one would make the page describe a change that no longer exists. This is the shape recorded as #515: "check-doc-checkpoint treats every root CMakeLists.txt edit as a usage change, so adding a source file demands a docs/USAGE.md edit that has nothing true to say". #1086 narrowed the sibling `feature_surface` trigger from a path to a registration-set change; `USER_USAGE_FILES` still keys off the path. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode] --- examples/CMakeLists.txt | 1 - examples/laguna_gen/main.cpp | 5 +---- src/vllm/support/platform_compat.h | 5 +++++ tests/CMakeLists.txt | 18 +++++++++++++++--- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 35192ab32..6ac37b929 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -20,7 +20,6 @@ vllm_cpp_set_warnings(deepseek-v4-gen) # driver (W5 RUNNABLE vehicle — 3-shard keep-quant load + stateless recompute). add_executable(laguna-gen laguna_gen/main.cpp) target_link_libraries(laguna-gen PRIVATE vllm::vllm) -target_include_directories(laguna-gen PRIVATE ${CMAKE_SOURCE_DIR}/src) vllm_cpp_set_warnings(laguna-gen) # kimi-linear-gen: the Kimi-Linear-48B-A3B golden token battery, a THIN CLIENT diff --git a/examples/laguna_gen/main.cpp b/examples/laguna_gen/main.cpp index 17ecd0314..60dc71ae2 100644 --- a/examples/laguna_gen/main.cpp +++ b/examples/laguna_gen/main.cpp @@ -32,7 +32,6 @@ #include "vllm/model_executor/model_loader/gguf_reader.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" #include "vllm/model_executor/models/laguna.h" -#include "vllm/support/platform_compat.h" #include "vllm/tokenizer/tokenizer.h" #include "vllm/transformers_utils/hf_config.h" #include "vt/backend.h" // vt::GetBackend / CreateQueue (--gpu: GEMMs on the GB10) @@ -202,9 +201,7 @@ int main(int argc, char** argv) { // s/tok (2×; 2.56 → 5.0 tok/s), coherent + near-tie. `setenv(...,0)` respects an // explicit `VT_NVFP4_FP4_NATIVE=0` override. Scoped to this Laguna driver (the // 27B/35B use the separate DirectD cutlass path, untouched). - if (std::getenv("VT_NVFP4_FP4_NATIVE") == nullptr) { - (void)vllm::support::SetEnvVar("VT_NVFP4_FP4_NATIVE", "1"); - } + setenv("VT_NVFP4_FP4_NATIVE", "1", 0); const std::string config_path = (fs::path(model) / "config.json").string(); const std::string tok_path = (fs::path(model) / "tokenizer.json").string(); std::fprintf(stderr, "[gen] NVFP4 safetensors dir %s\n", model.c_str()); diff --git a/src/vllm/support/platform_compat.h b/src/vllm/support/platform_compat.h index 340897fcd..a7ce92878 100644 --- a/src/vllm/support/platform_compat.h +++ b/src/vllm/support/platform_compat.h @@ -35,6 +35,11 @@ inline int FileDescriptorFromFile(std::FILE* file) { return _fileno(file); } inline bool TruncateFile(int fd, std::uint64_t size) { return _chsize_s(fd, size) == 0; } +// DIVERGENT ON AN EMPTY VALUE, and deliberately not normalised: `_putenv_s(name, +// "")` REMOVES the variable, where POSIX `setenv(name, "", 1)` defines it empty. +// No caller passes an empty value today. A test that needs defined-but-empty must +// say so at its call site rather than relying on this — the same contract +// `tests/support/test_env.h` records for the test-side seam. inline bool SetEnvVar(const char* name, const char* value) { return _putenv_s(name, value) == 0; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 74a85211d..63706934f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,9 +21,6 @@ function(vllm_cpp_add_test name) # under tests/support/ as "support/.h". No header name under tests/ # collides with one under include/ or src/, so nothing else resolves here. target_include_directories(${name} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - # Some test-only portability helpers live under src/vllm/support/ on purpose: - # they are internal build shims, not installed public headers. - target_include_directories(${name} PRIVATE ${CMAKE_SOURCE_DIR}/src) if(TARGET vllm::sanitize_test_shared) target_link_libraries(${name} PRIVATE vllm::sanitize_test_shared vllm_test_main) else() @@ -2200,3 +2197,18 @@ target_include_directories(test_qwen3_32b_nvfp4a16_paged_engine PRIVATE # CPU-only and needs no checkpoint. vllm_cpp_add_test(test_registry_downcast_refusal vllm/models/test_registry_downcast_refusal.cpp) + +# The five suites that reach src/vllm/support/platform_compat.h, granted per +# target rather than globally: the file already carries 123 explicit +# ${CMAKE_SOURCE_DIR}/src grants, and a blanket one in vllm_cpp_add_test would +# convert that deliberate opt-in into a repo-wide default (#503). +foreach(_pc_target + test_safetensors + test_minimax_h3 + test_minimax_h3_video_fold + test_kv_offload_connector + test_kv_offload_tiering) + if(TARGET ${_pc_target}) + target_include_directories(${_pc_target} PRIVATE ${CMAKE_SOURCE_DIR}/src) + endif() +endforeach()