Skip to content

Add persistent output locale override setting - #6310

Merged
JohnMcPMS merged 17 commits into
microsoft:masterfrom
Trenly:trenly/fix-422-output-locale-override
Sep 1, 2026
Merged

Add persistent output locale override setting#6310
JohnMcPMS merged 17 commits into
microsoft:masterfrom
Trenly:trenly/fix-422-output-locale-override

Conversation

@Trenly

@Trenly Kaleb Luedtke (Trenly) commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Add a persistent output.locale user setting that allows overriding winget interface string resolution by BCP47 tag.

Current implementation details:

  • Reads .output.locale at CLI startup and applies the override only when the setting is non-empty.
  • Applies the override through Resource::SetLanguageOverride, which sets the MRT Language qualifier for winget resource loading.
  • Treats override failures as non-fatal (logs warning and continues without the override).
  • Uses wil::srwlock in the shared resource loader for synchronized read/write access when resolving strings and updating locale override.
  • Does not apply this setting in server initialization.

This PR also updates settings schema/documentation and local package resource fallback handling for localized winget.resw files.

🔗 References

Resolves #422

🤖 Copilot Assistance

This PR includes changes authored with GitHub Copilot assistance.

🔍 Validation

  • Added/updated unit tests for OutputLocale setting parsing/normalization behavior.
  • Added E2E coverage verifying help output changes when output.locale changes.

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

* Add output.locale setting and schema support
* Apply runtime language qualifier override for resource resolution
* Include localized resw fallbacks in dev package and update docs/release notes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Trenly
Kaleb Luedtke (Trenly) changed the base branch from release-v1.29 to master June 21, 2026 04:51
@microsoft-github-policy-service microsoft-github-policy-service Bot added Issue-Feature This is a feature request for the Windows Package Manager client. Area-Settings Issue related to WinGet Settings labels Jun 21, 2026
Comment thread src/AppInstallerCLIPackage/AppInstallerCLIPackage.wapproj
@Trenly
Kaleb Luedtke (Trenly) marked this pull request as ready for review June 21, 2026 05:10
@Trenly
Kaleb Luedtke (Trenly) requested a review from a team as a code owner June 21, 2026 05:10
Comment thread src/AppInstallerCLICore/Core.cpp Outdated
Comment thread src/AppInstallerCLICore/Core.cpp Outdated
Comment thread src/AppInstallerCLICore/Core.cpp Outdated
Comment thread src/AppInstallerCommonCore/UserSettings.cpp Outdated
Comment thread src/AppInstallerCLITests/UserSettings.cpp
Comment thread doc/windows/package-manager/winget/settings.md
Comment thread src/AppInstallerCLICore/Core.cpp Outdated
Comment thread src/AppInstallerCommonCore/Public/winget/UserSettings.h Outdated
Comment thread src/AppInstallerCLICore/Core.cpp Outdated
@microsoft-github-policy-service microsoft-github-policy-service Bot added Needs-Author-Feedback Issue needs attention from issue or PR author Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Jun 30, 2026

@florelis Flor Chacón (florelis) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but I'd want to hold off on merging until we have a plan for what to do with App Installer.

Comment thread src/AppInstallerCommonCore/UserSettings.cpp Outdated
@florelis

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@Trenly

This comment was marked as outdated.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@florelis

Copy link
Copy Markdown
Member

Looks like we will need to reset the override in App Installer. Internal PR number for that: 16173490
I won't merge this until that other change is in place.

@florelis

Copy link
Copy Markdown
Member

Question from internal discussion:
Instead of overriding the primary language like this, would it be possible to just add the locale qualifier when creating the resource loader? That would avoid setting the locale package-wide, and we wouldn't need to make any changes to App Installer.


bool SetLanguageOverride(std::string_view localeTag)
{
return Loader::Instance().SetLanguageOverride(Utility::ConvertToUTF16(localeTag));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Assuming this is done before any localized strings are fetched, this is going to create a loader with default options, and then immediately overwrite it with a new one that uses the locale. We could make it so that we only needed to create it once, but that's probably too small a gain to care.

@florelis

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@Trenly

Copy link
Copy Markdown
Contributor Author

Grrr, pesky build failures

@Trenly

Copy link
Copy Markdown
Contributor Author

Flor Chacón (@florelis) - I've merged the latest changes from master to see if that resolves the ADO build issue despite it building locally without them. I've verified it builds locally after the merge:

image

@florelis

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

ApplyOutputLocaleOverride() unconditionally triggered Loader::Instance()
at startup for every command, including dscv3 --manifest which runs
winget.exe unpackaged during the WinGetGenerateDSCv3Manifests build step.

The Loader() constructor had been refactored to call CreateLoader({}),
which calls ResourceContext::ResetGlobalQualifierValues() before
ResourceLoader(). This broke the safety guard that the original code
relied on: when unpackaged and resource.pri is not found, ResourceLoader()
throws a catchable hresult_error, preventing execution from reaching
ResourceLoader::GetForViewIndependentUse which fast-fails with an
uncatchable crash (0xC0000409 STATUS_STACK_BUFFER_OVERRUN) when not
under a debugger.

Two fixes:
- Restore the original constructor initialization path (without calling
  CreateLoader) so the ResourceLoader() probe is not interfered with
- Guard ApplyOutputLocaleOverride to skip loader initialization entirely
  when no locale override is configured (the common case)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Trenly

Kaleb Luedtke (Trenly) commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Root Cause Analysis

Note

This comment was authored by GitHub Copilot.

Root cause: ApplyOutputLocaleOverride() unconditionally accessed Loader::Instance() at startup for every command. The WinGetGenerateDSCv3Manifests build step runs winget.exe dscv3 --manifest as an unpackaged process (before MSIX packaging), and the refactored Loader() constructor now calls CreateLoader({}) which calls ResourceContext::ResetGlobalQualifierValues() before the ResourceLoader() probe.

The original code relied on ResourceLoader() throwing a catchable hresult_error when resource.pri is not found (unpackaged context) — this prevented execution from ever reaching ResourceLoader::GetForViewIndependentUse, which fast-fails with an uncatchable crash (0xC0000409) when no debugger is attached. Calling ResetGlobalQualifierValues first changed the WinRT resource state and broke this safety guard.

Fix

Two changes applied in commit 25c246a:

  1. Resources.cpp: Restored the original constructor initialization path — no ResetGlobalQualifierValues call, preserving the ResourceLoader() probe guard
  2. Core.cpp: Guard ApplyOutputLocaleOverride() to return early when the locale setting is empty, avoiding Loader::Instance() initialization at startup for commands that don't need UI output (like dscv3 --manifest)

Comment thread src/AppInstallerSharedLib/Resources.cpp Outdated
Comment thread src/AppInstallerCLICore/Core.cpp
Kaleb Luedtke (Trenly) and others added 2 commits August 5, 2026 21:37
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JohnMcPMS

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@JohnMcPMS
JohnMcPMS merged commit 9e7f797 into microsoft:master Sep 1, 2026
9 checks passed
@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Attention Issue needs attention from Microsoft label Sep 1, 2026
@Trenly
Kaleb Luedtke (Trenly) deleted the trenly/fix-422-output-locale-override branch September 2, 2026 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Settings Issue related to WinGet Settings Issue-Feature This is a feature request for the Windows Package Manager client.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow the user to override winget's locale setting

4 participants