LT-22801: Keep directory resolution out of patch transactions - #92
johnml1135 wants to merge 1 commit into
Conversation
LT-20524 scheduled the directory-resolution custom actions in the InstallExecuteSequence under "UILevel < 4", so that silent and basic-UI installs resolve APPFOLDER, DATAFOLDER and HARVESTDATAFOLDER the way a full-UI install does. A patch is applied with /qb, which is UILevel 3, so that condition caught every patch as well. Those actions are sequenced after AppSearch and FindRelatedProducts, which puts them before CostFinalize. Reassigning the directory properties there leaves the installer unable to confirm that the product's existing components are still installed locally, so every feature costs out Advertised and every component Action: Null. The patch then installs only components that are absent and replaces no file that is already there, while msiexec returns 0 and the registered ProductVersion advances -- so it looks like it worked. Add NOT PATCH to the nine conditions, and to the same nine actions in the InstallUISequence, which a patch reaches when it is applied with UI rather than through ProcRunner. A patch shows no directory-selection dialogs and resolves its directories from the installed product, so it has nothing to gain from these actions on either path. Found by bisecting five base builds. Base 1448, cut two days before LT-20524 landed, applies two patches in sequence correctly and its patch log never mentions these actions; base 1452, cut thirteen days after it, fails and its patch log mentions them fifteen times. Bases 1379, 1416 and 1439 all predate the change and all work. This needs a new base build to take effect. The sequence table lives in the base MSI, so an already installed base keeps the old conditions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing: this is not the LT-22801 fix. The directory actions are not the cause. The cause. Patches 9.3.11.2703 and 9.3.12.2718 add
Why this PR's premise does not hold.
What replaces it is a FieldWorks change: restore Fluent as a |
|
Replacement: sillsdev/FieldWorks#1147. Placeholder removal before the next base is tracked in LT-22807. |
Start here:
BaseInstallerBuild/Framework.wxs, the nine<Custom>elements inInstallExecuteSequence. One condition change is the whole fix.Status: the mechanism is unproven. Do not merge on my evidence alone. The bisect below is strong and reproducible; two attempts to reproduce the failure in a controlled rig both came back clean, so I could not demonstrate that this change fixes anything. Details in What we could not confirm.
What it does. LT-20524 scheduled the directory-resolution custom actions under
UILevel < 4, so silent and basic-UI installs resolveAPPFOLDER/DATAFOLDER/HARVESTDATAFOLDERthe way a full-UI install does. A patch is applied with/qb, which is UILevel 3 — so that condition catches every patch too. This addsNOT PATCH.Why that plausibly matters. These actions sit after
AppSearchandFindRelatedProducts, ahead ofCostFinalize. Reassigning directory properties there could leave the installer unable to confirm existing components are local — and in the production failure every feature costs outAdvertisedwith every componentAction: Null, so the patch installs only components that are absent, replaces nothing already on disk, and still returns 0 while the registered ProductVersion advances. Users see a stale build number: LT-22801.Where to look
NOT PATCHdepends on MSI settingPATCHduring/p— confirmed in a real log.InstallUISequence. A patch cannot dodge these actions by UI level:/passivestill ran at UILevel 3 in testing, so both sequences need it.Deliberately not here. Nothing repairs machines already on base 1452 — they need a fresh base regardless, matching the ticket's workaround. No change to ProcRunner or patch authoring.
Next: decide whether the bisect alone justifies taking this, or block it pending a CI base build carrying the fix. I would not merge it as proven.
The bisect — five base builds, reproducible
Each base bakes in whatever
genericinstallermasterheld at its cut date; both CD workflows check it out unpinned atref: master.5ea02f7op: FileCopyBase 1448 is the control: cut two days before
5ea02f7, with both its patches built after it (2604 on 07-13, 2622 on 07-23). Post-change patches on a pre-change base work.Each run installs the base, applies patch A, applies patch B, comparing the on-disk SHA-256 of
FieldWorks.exeat each step. The registry version advances even when nothing installs, so it cannot be used to judge success.5ea02f7is the only genericinstaller commit between 1448 and 1452 that can reach MSI authoring;cbf5918(07-13) touches only the bundle.Direct evidence — same patches, different base
Mentions of the LT-20524 actions in each patch-application log:
Feature state at
InstallValidate:The three files the failing patch installs are exactly the ones absent from the previous patch:
AIExportInstructions.md,GrammarAndTextsForAI.xml,Microsoft.Extensions.DependencyInjection.dll.Confirming the property the new condition tests:
What we could NOT confirm — two failed reproductions
This is the honest gap. Both attempts built an A/B pair of base MSIs differing only in these conditions, plus a locally built patch pair, and ran base -> patch A -> patch B against each.
Attempt 1. The rig's localization harvests were empty
ComponentGroupstubs, so 22 of 33 features held no components, sat atAction: Null, and could never flip to Advertised. 1,408 components against the real base's 2,887. Could not express the bug. A marker-detection bug in the harness also misreported a working patch as failed, since U2 files contain both markers.Attempt 2. Regenerated all 29 localization harvests with the real
HarvestOneL10nheat invocation. The rig then matched the real base exactly — 2,887 components, 1,485 localization, per-languageFeatureComponentsidentical row for row. Result:The fix demonstrably does what it is authored to do — C2 suppressed all nine actions where C1/C3 ran them. But the unfixed base did not fail, so the run says nothing about whether suppressing them fixes anything.
Unexplained: C1 reported
Local=13where the other cycles showed 32. Not Advertised, so not the bug, but it is the only asymmetry the unfixed base produced.The most likely missing ingredient: our patch B adds no new components, it only edits existing files. Production's patch 2754 carried 8 components patch A lacked, 3 with files — and those 3 were the only things that installed. Reconciling two transforms with differing component sets is plausibly where this goes wrong, and the rig never exercised it.
Other rig confounds: built with WiX 3.14 against CI's 3.11.2; localization payloads are placeholder-sized rather than real translated resources; base installed via
msiexec /irather than the Burn bootstrapper.Ruled out by measurement
REINSTALL=ALL REINSTALLMODE=ecmuson the silent/p. Applied on first application: identical 3 copies, unchanged binary, all 32 features Advertised. Withdrawn as PR LT-22801: Set REINSTALL and REINSTALLMODE for silent patch installs #90.-ggvs-agharvest divergence betweenbuildMsi.batandbuildPatch.bat. Real, dates to the 2017 initial commit — and present in all four working chains. Withdrawn as PR LT-22801: Harvest patches the same way as the base installer #91. Alsoheat -ggmints fresh random GUIDs per invocation, so aligning on it is impossible.5711bf6be, 2026-02-17). Base 1416 was cut seven weeks later and works.173675fand68d79ee. Base 1439 and its patches all carry them and work.Feature,MsiAssemblyorCondition; the Feature table is byte-identical before and after both patches, all 33 rows carrying DisallowAdvertise. All 109 shared components have identical GUIDs.__<name>65shadow component rows. Documented, benign MSI 3.x cost-linking bookkeeping, present on successful runs too./passivestill reports UILevel 3 for a patch, so the actions run regardless.How to validate this properly
base-installer-cd.ymlaccepts aworkflow_dispatchinput built for exactly this:Dispatching with
installer_ref: refs/pull/92/headproduces a production-built base carrying this fix — full localization, WiX 3.11.2, real harvest — removing every rig confound at once. Two patches then need building against it, which wants the base published as a release.Failing that, the local rig is reusable and documented; the next thing to try is a patch B that adds new components rather than only editing existing files.
🤖 Generated with Claude Code
This change is