LT-22801: Keep patches from dropping shipped components - #1147
Open
johnml1135 wants to merge 6 commits into
Open
johnml1135 wants to merge 6 commits into
johnml1135 wants to merge 6 commits into
Conversation
Patches 9.3.11.2649 to 9.3.12.2718 on base 1452 shipped Avalonia.Themes.Fluent.dll. The Semi.Avalonia switch dropped it, so patches from 9.3.12.2752 on do not contain its component. Each patch supersedes the one before, so Windows Installer sees only base plus newest patch while machines still have Fluent's component registered. It logs SELMGR, treats feature Complete as advertised, copies nothing and returns 0, and the version users see still advances. Restore the component with a zero-byte RescuePatching placeholder at the same path, as already done for StructureMap.dll. LT-22807 tracks removing it before the next base. pyro only compares a patch with the base, so it cannot see a patch dropping a component an earlier patch added. Add two checks to the patch workflow, before signing: - Test-PatchComponentLedger.ps1 compares the new patch with every component published patches on the base have added: a committed seed ledger for base 1452, plus a ledger now published next to each .msp. It names the dropped file and the fix. - Test-PatchInstall.ps1 installs the published base and the latest patch, applies the new patch with MSIENFORCEUPGRADECOMPONENTRULES=1 (error 2771 instead of silent success), and checks that FieldWorks.exe on disk carries the new version. All 53 patches published on base 1452 were read into the seed. Fluent is the only component any of them dropped. Docs/workflows/patch-component-removal.md explains the failure, the fix, and the steps before cutting a new base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NUnit Tests 1 files ± 0 1 suites ±0 12m 43s ⏱️ +20s Results for commit 089eb90. ± Comparison against base commit 2f1d9db. This pull request removes 1 and adds 12 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1147 +/- ##
=======================================
Coverage 38.89% 38.89%
=======================================
Files 1518 1518
Lines 352262 352309 +47
Branches 40597 40601 +4
=======================================
+ Hits 137005 137033 +28
- Misses 185976 185997 +21
+ Partials 29281 29279 -2 🚀 New features to boost your workflow:
|
Use the existing RemovedSinceLastBase stand-ins for components dropped from either the base MSI or the immediately previous patch. Publish a four-column update-minus-base ledger beside each MSP and check the next patch against it. Remove the installer smoke test and make base builds fail with direct placeholder cleanup instructions.
Use S3 as the sole patch-ledger source. The first release on a patch line creates its ledger, and later patches require the ledger beside the immediately previous MSP.
Record APPFOLDER-relative paths so rescue entries remain correct for nested files. Keep ledger artifacts in S3 and narrow checks to file-backed components supported by RemovedSinceLastBase.
johnml1135
marked this pull request as ready for review
September 21, 2026 20:21
papeh
reviewed
Sep 21, 2026
papeh
left a comment
papeh
left a comment
Contributor
There was a problem hiding this comment.
Looks good overall so far. I'll take a look at the rest tomorrow.
One thought about the overall strategy: creating a ledger for each patch means that if we:
- build a nightly patch that introduces a new file, but
- remove that file before shipping,
then the shipping patch will have an empty file that was never installed on users' machines (except for the few users that get special access to nightly patches). Shipping this empty file would not be ideal, but it's also not a huge deal.
Keep scheduled base verification builds useful while blocking base releases that still contain patch stand-ins. Preserve the generated Master and Update MSIs for the component ledger check, and clarify ledger selection and cleanup guidance.
Contributor
Author
|
On the nightly-patch tradeoff: I agree it can leave a zero-byte stand-in that most users never needed. I am keeping that behavior because any S3-published patch can reach special-access users, and dropping its component recreates the silent-update failure for them. Restricting compatibility to promoted releases would require explicit promotion or channel metadata and is outside this minimal fix. The temporary-MSI support is isolated in genericinstaller #93, and hosted validation is running at https://github.com/sillsdev/FieldWorks/actions/runs/35722248879. |
papeh
reviewed
Sep 22, 2026
papeh
requested changes
Sep 22, 2026
papeh
left a comment
papeh
left a comment
Contributor
There was a problem hiding this comment.
Looks good overall. One bug. A few wording suggestions.
papeh
reviewed
Sep 22, 2026
papeh
reviewed
Sep 22, 2026
Rename Read-ComponentLedger -Path to -LedgerFiles, since it takes several files, and rename the artifact check script to Check-PatchComponentLedger.ps1 so it is not mistaken for a unit test. Make the missing-ledger branch of Select-PreviousPublishedPatch explicit, and make its error name the recovery paths that actually exist: a new base, or a ledger rebuilt by applying the published patch to its base MSI. Add ledger entries directly instead of copying them field by field. Drop the zero-byte output from the base-release warning and error; CI builds start clean, and the docs keep the note for dirty local builds. Ask for a cleanup issue only when none exists for the current base, and remove the base 1452 note from the installer AGENTS guidance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.
Start here: A patch built against base 1452 could install over the base yet do nothing over an earlier patch when that earlier patch introduced a component the new patch dropped. This change keeps Fluent's component identity with the existing zero-byte stand-in and publishes one small S3-only ledger so the next patch rejects the same removal before signing.
The patch check compares file-backed components under the MSI
APPFOLDERagainst both the base build and the previous patch ledger. It uses the existing zero-byte rescue path and the same concise remediation response for either source. The ledger is generated during the patch build, stored only in S3, and is never shipped in the repository or GitHub build-log artifact.Hasso's review is addressed: scheduled base verification warns about remaining stand-ins while a base release fails; comments and operator guidance explain the Windows Installer constraint without tying Fluent to a stale base number; and the selection tests now use interleaved patches from different bases. The patch build also preserves its temporary Master and Update MSIs long enough for the ledger check through genericinstaller #93.
Hosted validation is running on commit
09d288617c88eac43c183f4b6df76b9e9953110d: workflow run 35722248879. It uses the genericinstaller dependency branch from #93. The prior run exposed that genericinstaller deleted both temporary MSIs before the ledger step; the dependency PR keeps them only when the FieldWorks workflow requests it.Where to look:
scripts/Installer/PatchComponentLedger.psm1— ledger creation, S3 lookup, and relative MSI path handling.scripts/Installer/PatchComponentLedger.Tests.ps1— component paths and previous-patch selection coverage..github/workflows/patch-installer-cd.yml— S3-only ledger publication and temporary MSI retention.Build/Installer.legacy.targets— Fluent zero-byte stand-in and base-release cleanup gate.Docs/workflows/patch-component-removal.md— operator-facing behavior.Validation: full local build passed with 0 warnings and 0 errors; full test passed with 6,151 passed, 62 skipped, and 0 failed; focused ledger tests passed 22/22 under Windows PowerShell 5.1 and PowerShell 7; comment hygiene, token hygiene, whitespace, and gitlint checks passed.
Deliberate scope: the zero-byte mechanism covers application output files under
APPFOLDER; registry and data-root servicing remain outside this change. The first ledger-bearing patch bootstraps from the base build. LT-22807 cleanup remains separate.This change is