Fix windows cache_dir path #4377
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Windows-specific path handling issues by normalizing backslashes to forward slashes for the CACHE_DIR plugin configuration when exporting graphs, and adds a unit test to prevent regressions. It also updates the Windows environment setup script to better propagate environment variables to the caller.
Changes:
- Normalize Windows path separators for
CACHE_DIRinGraphExport::createPluginString()(both when provided viamanualStringand viapluginConfig.cacheDir). - Add a Windows-only unit test validating that
CACHE_DIRis serialized without backslashes. - Update
windows_setupvars.batcontrol flow to propagate selected environment variables back to the calling shell and handle setup failures.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| windows_setupvars.bat | Adjusts error handling and propagates selected environment variables to the caller session. |
| src/test/graph_export_test.cpp | Adds a Windows-only unit test ensuring CACHE_DIR is normalized to forward slashes in the generated plugin config JSON. |
| src/graph_export/graph_export.cpp | Introduces path-separator normalization helper and applies it to CACHE_DIR handling in plugin config generation/parsing. |
| static std::string normalizeWindowsPathSeparators(const std::string& path) { | ||
| #ifdef _WIN32 | ||
| std::string normalized = path; | ||
| std::replace(normalized.begin(), normalized.end(), '\\', '/'); | ||
| return normalized; | ||
| #else | ||
| return path; | ||
| #endif | ||
| } |
There was a problem hiding this comment.
Don't we have some utils function for that already? Looks like a fairly common logic.
| // plugin_config is injected into pbtxt and later parsed as JSON again. | ||
| // Normalize Windows path separators to avoid backslash escape ambiguity. | ||
| auto cacheDirIt = d.FindMember("CACHE_DIR"); | ||
| if (cacheDirIt != d.MemberEnd() && cacheDirIt->value.IsString()) { | ||
| auto normalizedCacheDir = normalizeWindowsPathSeparators(cacheDirIt->value.GetString()); | ||
| cacheDirIt->value.SetString(normalizedCacheDir.c_str(), static_cast<rapidjson::SizeType>(normalizedCacheDir.size()), d.GetAllocator()); | ||
| } |
There was a problem hiding this comment.
Why are we doing it here and not through pluginConfig.cacheDir.has_value() like for kvCachePrecision below?
|
The --cache_dir would propagate to classic models when using config.json if I recall correctly. Fix in graph_export would resolve the issue with OVMS started with single GenAI model. It won't work with --config_path i guess? |
🛠 Summary
JIRA CVS-187603
Normalize path and add test.
🧪 Checklist
``