Skip to content

add extra env variable to support winget installation#4398

Open
dtrawins wants to merge 12 commits into
mainfrom
winget_env
Open

add extra env variable to support winget installation#4398
dtrawins wants to merge 12 commits into
mainfrom
winget_env

Conversation

@dtrawins

@dtrawins dtrawins commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

🛠 Summary

WA needed for winget deployment - point to the genai and tokenizer libraries installed by winget

🧪 Checklist

  • Unit tests added.
  • The documentation updated.
  • Change follows security best practices.
    ``

Copilot AI review requested due to automatic review settings July 23, 2026 23:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Windows entrypoint to initialize OPENVINO_TOKENIZERS_PATH_GENAI automatically when OVMS is deployed as an MSIX/winget package, by discovering the OpenVINO Tokenizers dependency package and deriving the DLL path from its installed location.

Changes:

  • Added C++/WinRT package dependency inspection to locate the Tokenizers MSIX dependency.
  • Set OPENVINO_TOKENIZERS_PATH_GENAI based on the dependency’s InstalledLocation at process startup.
  • Introduced a new WinRT header include to support the dependency query.
Comments suppressed due to low confidence (2)

src/main_windows.cpp:118

  • This code uses C++/WinRT APIs (Package::Current()...) but the thread apartment is never initialized (no winrt::init_apartment() found). Without initialization, WinRT calls can fail and the env var will not be set even in packaged deployments.
    try {
        winrt::Windows::Foundation::Collections::IVectorView<winrt::Windows::ApplicationModel::Package> deps =
            winrt::Windows::ApplicationModel::Package::Current().Dependencies();
        for (const winrt::Windows::ApplicationModel::Package& dep : deps) {

src/main_windows.cpp:123

  • This unconditionally overwrites OPENVINO_TOKENIZERS_PATH_GENAI, which can clobber a user- or admin-provided setting. Prefer only setting it when it is not already defined, and consider logging/handling a failure return from SetEnvironmentVariableW.
                SetEnvironmentVariableW(L"OPENVINO_TOKENIZERS_PATH_GENAI", dllPath.c_str());
                DEBUG_LOG("OPENVINO_TOKENIZERS_PATH_GENAI initialized from package dependency.");
                return;

Comment thread src/main_windows.cpp Outdated
Comment thread src/main_windows.cpp
#include <windows.h>
#include <tchar.h>
#include <errors.h>
#include <winrt/Windows.ApplicationModel.h>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add #include <winrt/base.h>
It's valid comment

Comment thread src/main_windows.cpp Outdated
Comment on lines +112 to +120
constexpr wchar_t PACKAGE_NAME[] = L"Intel.OpenVINO.Tokenizers.2026.3.0.0";
constexpr wchar_t TOKENIZERS_RELATIVE_PATH[] = L"\\runtime\\bin\\intel64\\Release\\openvino_tokenizers.dll";

try {
winrt::Windows::Foundation::Collections::IVectorView<winrt::Windows::ApplicationModel::Package> deps =
winrt::Windows::ApplicationModel::Package::Current().Dependencies();
for (const winrt::Windows::ApplicationModel::Package& dep : deps) {
if (dep.Id().Name() == PACKAGE_NAME) {
const std::wstring dllPath = dep.InstalledLocation().Path().c_str() + std::wstring(TOKENIZERS_RELATIVE_PATH);
dtrawins and others added 5 commits July 24, 2026 10:44
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread src/main_windows.cpp

int main_windows(int argc, char** argv) {
DEBUG_LOG("Windows Main - Entry");
setOpenvinoTokenizersPathFromMsixDependencies();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
setOpenvinoTokenizersPathFromMsixDependencies();
winrt::init_apartment();
setOpenvinoTokenizersPathFromMsixDependencies();

Comment thread src/main_windows.cpp
}
}
} catch (const winrt::hresult_error&) {
// Not all deployments are packaged; keep default behavior when package metadata is unavailable.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Not all deployments are packaged; keep default behavior when package metadata is unavailable.
// Not all deployments are packaged; keep default behavior when package metadata is unavailable.
DEBUG_LOG("Failed to resolve Tokenizers dependency: 0x%08X %S",
e.code().value, e.message().c_str());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants