Skip to content

LT-22801: Keep directory resolution out of patch transactions - #92

Closed
johnml1135 wants to merge 1 commit into
sillsdev:masterfrom
johnml1135:LT-22801-scope-dir-actions-to-installs
Closed

johnml1135 wants to merge 1 commit into
sillsdev:masterfrom
johnml1135:LT-22801-scope-dir-actions-to-installs

Conversation

@johnml1135

@johnml1135 johnml1135 commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Start here: BaseInstallerBuild/Framework.wxs, the nine <Custom> elements in InstallExecuteSequence. 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 resolve APPFOLDER/DATAFOLDER/HARVESTDATAFOLDER the way a full-UI install does. A patch is applied with /qb, which is UILevel 3 — so that condition catches every patch too. This adds NOT PATCH.

Why that plausibly matters. These actions sit after AppSearch and FindRelatedProducts, ahead of CostFinalize. Reassigning directory properties there could leave the installer unable to confirm existing components are local — and in the production failure every feature costs out Advertised with every component Action: 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 PATCH depends on MSI setting PATCH during /p — confirmed in a real log.
  • The same guard went on InstallUISequence. A patch cannot dodge these actions by UI level: /passive still ran at UILevel 3 in testing, so both sequences need it.
  • Takes effect only on a new base build; the sequence table lives in the base MSI.
  • LT-20524's silent-install fix must not regress. Installs are untouched by the new term.

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 genericinstaller master held at its cut date; both CD workflows check it out unpinned at ref: master.

base cut carries 5ea02f7 chain op: FileCopy
1379 2026-01-08 no works 1,385 → 1,424
1416 2026-04-07 no works 1,578 → 1,855
1439 2026-06-11 no works 1,707 → 1,797
1448 2026-07-08 no works 1,668 → 1,749
1452 2026-07-23 yes fails 1,898 → 3

Base 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.exe at each step. The registry version advances even when nothing installs, so it cannot be used to judge success.

5ea02f7 is 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:

base 1448 (works):   0
base 1452 (fails):  15

Feature state at InstallValidate:

patch on base 1448:  Feature: Indonesian; Installed: Local;     Action: Reinstall   -> 1,749 copies
patch on base 1452:  Feature: Indonesian; Installed: Advertise; Action: Reinstall   ->     3 copies

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:

PROPERTY CHANGE: Adding PATCH property. Its value is '...FieldWorks_9.3.12.2754_b1452_x64.msp'
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 ComponentGroup stubs, so 22 of 33 features held no components, sat at Action: 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 HarvestOneL10n heat invocation. The rig then matched the real base exactly — 2,887 components, 1,485 localization, per-language FeatureComponents identical row for row. Result:

C1 unfixed /qb      patch B: CAs ran=5  copies=32  Advertise=0  Local=13   patch APPLIED
C2 fixed   /qb      patch B: CAs ran=0  copies=32  Advertise=0  Local=32   patch APPLIED
C3 unfixed /passive patch B: CAs ran=5  copies=32  Advertise=0  Local=32   patch APPLIED

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=13 where 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 /i rather than the Burn bootstrapper.

Ruled out by measurement
  • REINSTALL=ALL REINSTALLMODE=ecmus on 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.
  • The -gg vs -ag harvest divergence between buildMsi.bat and buildPatch.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. Also heat -gg mints fresh random GUIDs per invocation, so aligning on it is impossible.
  • ProductVersion field collision. MSI compares three fields and 9.3.12.2718 -> 9.3.12.2754 is no change, but a genuine third-field change (9.3.11 -> 9.3.12) still fails.
  • The SDK-format project migration (FieldWorks 5711bf6be, 2026-02-17). Base 1416 was cut seven weeks later and works.
  • The Malayalam/Yoruba feature addition. Base 1416 already has 32 features and works.
  • genericinstaller's May pair 173675f and 68d79ee. Base 1439 and its patches all carry them and work.
  • Patch transform content. Neither patch touches Feature, MsiAssembly or Condition; the Feature table is byte-identical before and after both patches, all 33 rows carrying DisallowAdvertise. All 109 shared components have identical GUIDs.
  • The __<name>65 shadow component rows. Documented, benign MSI 3.x cost-linking bookkeeping, present on successful runs too.
  • Raising the UI level to dodge the actions. /passive still reports UILevel 3 for a patch, so the actions run regardless.
How to validate this properly

base-installer-cd.yml accepts a workflow_dispatch input built for exactly this:

installer_ref:
  description: 'Commit-ish for PatchableInstaller repository'
  default: 'master'

Dispatching with installer_ref: refs/pull/92/head produces 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 Reviewable

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>
@johnml1135

Copy link
Copy Markdown
Collaborator Author

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 Avalonia.Themes.Fluent.dll to feature Complete. Patch 9.3.12.2754, built after the Semi.Avalonia switch, does not contain that component at all. Each patch targets RTM and supersedes the previous one, so Windows Installer sees only base + 2754, while the machine still has Fluent's component registered. The failing log says so directly:

SELMGR: ComponentId '{B3A225EB-3642-5FE4-8ED4-B2DAE6F8AC9B}' is registered to feature 'Complete', but is not present in the Component table.  Removal of components from a feature is not supported!

Complete and its child features then cost out Advertise, so the reinstall copies nothing and still returns 0. The SELMGR line appears twice in the failing run and zero times in the succeeding first patch or in the working base-1448 chain.

Why this PR's premise does not hold.

  • APPFOLDER, DATAFOLDER and HARVESTDATAFOLDER resolve to identical values in the failing patch and the succeeding one on base 1452, so the LT-20524 actions misdirect nothing.
  • A rig base built without this change is table-identical to real base 1452 (InstallExecuteSequence, InstallUISequence, CustomAction, Property, Feature, and component/file counts), and its two-patch chain passed.
  • Across bases 1379, 1416, 1439 and 1448, every patch pair's component set is a superset of its predecessor's, and every one works. The two failing pairs on base 1452 each drop exactly the Fluent component.

What replaces it is a FieldWorks change: restore Fluent as a RescuePatching placeholder, plus CI checks that fail a patch which drops a component a published patch shipped. The NOT PATCH condition is harmless, but it fixes nothing, so it should not ship on this evidence.

@johnml1135 johnml1135 closed this Sep 21, 2026
@johnml1135

Copy link
Copy Markdown
Collaborator Author

Replacement: sillsdev/FieldWorks#1147. Placeholder removal before the next base is tracked in LT-22807.

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.

1 participant