[Mirror] Fix Windows desktop OS build version reporting#5
Open
bmehta001 wants to merge 13 commits into
Open
Conversation
Use servicing-aware registry values so desktop telemetry reports the current Windows build instead of stale BuildLabEx data after OS updates. Files changed: - lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp - tests/unittests/PalTests.cpp Fixes microsoft#1407 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor the Windows OS full-version composition into a testable helper so registry and Rtl build source precedence can be covered without depending on the host machine state. The new PAL tests cover CurrentBuildNumber precedence, CurrentBuild fallback, Rtl build fallback, and missing UBR formatting so future key or fallback regressions are caught before merge. Files changed: - lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp - tests/unittests/PalTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a deterministic PAL test proving that UBR value 0 is still included when the source reports it as present. This keeps zero-value handling separate from the missing-UBR fallback. Files changed: - tests/unittests/PalTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per Windows-team guidance on issue microsoft#1407: the RtlGetVersion() API is the authoritative source for the running OS build number, and the registry CurrentBuildNumber / CurrentBuild values are only useful for offline reads of another Windows installation. For the running OS the API and registry are always in sync, so prefer the API. UBR is registry-only and is always safe to combine with the kernel-provided build number. Drop the registry CurrentBuildNumber / CurrentBuild precedence chain and the now-unused getCurrentVersionStringValue helper. Format the OS full version directly from rtlOsvi.dwBuildNumber plus the optional UBR. Update PalTests to cover the simplified formatter (UBR present, UBR missing, zero UBR present), replacing the obsolete source-precedence tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
force-pushed
the
bhamehta/fix-windows-os-version
branch
from
5042ec7 to
78e6db4
Compare
trancified
approved these changes
May 27, 2026
trancified
left a comment
trancified
left a comment
There was a problem hiding this comment.
Good, using the RTLGetVerson API for everything except UBR.
* --atl * pr
… install (microsoft#1451) * ci: add least-privilege permissions to remaining workflows Adds top-level `permissions: { contents: read }` to the six workflow files on `main` that lacked an explicit GITHUB_TOKEN scope: - .github/workflows/build-android.yml - .github/workflows/build-ios-mac.yml - .github/workflows/build-posix-latest.yml - .github/workflows/build-ubuntu-2204.yml - .github/workflows/build-windows-vs2022.yaml - .github/workflows/test-win-latest.yml All six only check out source and run builds or tests; none post PR comments, deploy artifacts via the token, or otherwise need write scopes. `actions/upload-artifact@v4` (build-android.yml) uses its own per-run SAS URL and does not require GITHUB_TOKEN scopes. Carry-forward from PR microsoft#1450, which hardened spellcheck.yml the same way, and mirrors the analogous modules-repo commit decc96501. Each file gets an explanatory comment naming the CodeQL rule it satisfies (actions/missing-workflow-permissions) if Actions analysis is later enabled in this repo's code-scanning config. No behavior change. Already-permissioned workflows (codeql-analysis.yml, deploy-docs-pages.yml) are not touched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci(spellcheck): pin misspell + add least-privilege permissions Three hardening fixes for .github/workflows/spellcheck.yml, ported from the same audit applied to the sister modules repo: 1. Add top-level 'permissions: contents: read'. Without an explicit block, the GITHUB_TOKEN inherits the repo default (typically read+write across most APIs), which is far more than misspell needs. misspell only walks .md/.txt/lib/tests files and prints typos to stdout; it never comments on PRs, posts statuses, or writes packages. This also satisfies CodeQL rule 'actions/missing-workflow-permissions' if Actions analysis is enabled here later (it isn't in the current codeql-analysis.yml matrix, but no reason to wait for that). 2. Replace the unpinned bootstrap install with a pinned tarball download. The prior 'curl https://git.io/misspell | sh' pattern executed a shell script from the master branch of an external repo via a git.io redirect — a supply-chain risk that also made CI non-reproducible. Now we download a specific release artifact (v0.3.4) directly from GitHub releases. 3. Verify the tarball SHA256 against the published checksum 'afd95caf1eecc72ff382791e00b3b11523a20b0579d95e2295c1c043688743d5' (linux_64bit). Any tampering or unexpected upstream change will fail CI rather than silently execute. Scope of misspell unchanged: still scans .md, .txt, examples/**, all of lib/ (excluding json.hpp), and tests/**. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Reduce C++ CodeQL build scope Build a single representative x64 Release native configuration instead of the full Windows matrix so CodeQL extracts a smaller database and avoids standard-runner memory exhaustion. Files changed: - .github/workflows/codeql-analysis.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move CodeQL C++ build into script Keep the reduced CodeQL C++ target list in a dedicated batch file instead of embedding it directly in the workflow, as requested in review. Files changed: - .github/workflows/codeql-analysis.yml - build-codeql-cpp.bat Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes microsoft#1407
Please include this in the release notes for the next tagged build.
Before this PR,
DeviceInfo.OsBuild/ext.os.veron Windows desktop could be aBuildLabEx-style string (e.g.26100.1.amd64fre.ge_release.240331-1435). After this PR it is always a numericmajor.minor.build[.ubr]value (e.g.10.0.26100.4061).BuildLabExform (split on., substring extraction for branch / flavor / date) will need to be updated.If you maintain downstream consumers of
DeviceInfo.OsBuild/ext.os.ver, please audit those queries before the next release containing this change rolls out.Why this source
Confirmed with the Windows team:
So we prefer
RtlGetVersion()(kernel-authoritative for the running OS) and addUBRfromHKLM\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\UBR. The registryCurrentBuildNumber/CurrentBuildprecedence chain is not needed here — those are useful for offline-reading another Windows installation's hive, not the live running OS.Validation
msbuild Solutions\MSTelemetrySDK.sln /t:Tests\UnitTests /p:Configuration=Release /p:Platform=x64 /p:PlatformToolset=v145UnitTests.exe --gtest_filter=PalTests.*→ 10 tests passed, including the three newWindowsOsFullVersion*casesUnitTests.exe(full run) → 463 tests passed