Skip to content

Keep installed FieldWorks independent of dev registry paths (LT-22840) - #1183

Open
johnml1135 wants to merge 3 commits into
mainfrom
fix/LT-22840-dev-registry
Open

johnml1135 wants to merge 3 commits into
mainfrom
fix/LT-22840-dev-registry

Conversation

@johnml1135

@johnml1135 johnml1135 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Build/Repair-DeveloperRegistry.ps1 previews and, on request, removes source-tree overrides from the current user's registry.

The launch preflight is read-only, including with -Force. An opt-in repair can remove identified worktree code, data, ICU, encoding-converter root and silfw:// overrides. Test initialization no longer overwrites shared code and data paths, and ICU initialization skips registry paths that do not exist.

HKCU overrides remain deliberate: installed and Paratext readers retain their existing precedence, and a projects-folder change still persists. The repair keeps ProjectsDir and SilEncConverters40\DeveloperPluginDir; inspect -WhatIf before applying it. The build's registry targets remain available pending new-machine setup verification.

Where to look

  • Repair-DeveloperRegistry.Tests.ps1: selective cleanup, preview and preserved values.
  • InitializeFwRegistryHelperAttribute and FwDirectoryFinderTests: test setup does not rewrite shared directory values.
  • FwUtils.ChooseIcuDataDir and IcuDataDirChoiceTests: an existing user ICU directory wins; otherwise try the machine value.

Not here: no installed-reader source-tree filters, Paratext or Convert changes, or changes to ProjectsDirectory. The repair cannot identify a checkout whose FieldWorks.sln has moved or been removed.

Verification (Windows/x64, at a8e013bef):

  • .\build.ps1 -CommentHygiene -TokenHygiene: passed.
  • Repair-DeveloperRegistry.Tests.ps1: 23 checks passed in Windows PowerShell 5.1 and PowerShell 7.
  • FwUtilsTests: 489 of 493 passed. The 4 failures are DebugProcsTests.GetMessage_* (\n against \r\n). That file is unchanged from the base, and the same tests pass on the base in a CRLF checkout; the failing run used an LF checkout.
  • Not run: the full test.ps1, and a manual Project Location move and relaunch.

Next: review the cleanup scope.


Registry behavior

ICU_DATA still wins when set. Otherwise an existing HKCU ICU directory wins over an existing HKLM directory, followed by the existing development-tree fallback. A nonexistent per-user path is skipped in favor of an existing machine path. Explicit repair can also remove source-tree encoding-converter repository and silfw:// values; it preserves values outside identified worktrees.


This change is Reviewable

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   14m 19s ⏱️ -48s
6 958 tests +10  6 873 ✅ +10  85 💤 ±0  0 ❌ ±0 
6 967 runs  +10  6 882 ✅ +10  85 💤 ±0  0 ❌ ±0 

Results for commit a8e013b. ± Comparison against base commit 936ae91.

♻️ This comment has been updated with latest results.

@codecov-commenter

codecov-commenter commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.36%. Comparing base (d0d85cb) to head (a8e013b).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
Src/Common/FwUtils/FwUtils.cs 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1183      +/-   ##
==========================================
+ Coverage   39.88%   40.36%   +0.47%     
==========================================
  Files        1542     1550       +8     
  Lines      355361   355615     +254     
  Branches    41320    41477     +157     
==========================================
+ Hits       141753   143547    +1794     
+ Misses     184253   182758    -1495     
+ Partials    29355    29310      -45     
Files with missing lines Coverage Δ
Src/Common/FwUtils/FwUtils.cs 21.59% <77.77%> (+2.65%) ⬆️

... and 94 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@johnml1135

Copy link
Copy Markdown
Contributor Author

Update: removed the unused setRegistryValues/setKeysInHKCU targets, which wrote RootCodeDir/RootDataDir/ProjectsDir, ICU, EC and silfw:// keys to HKCU. A source-tree run keeps a changed projects folder in memory only and never writes HKCU; ProjectsDir is still shared with installed FLEx and is not filtered, so existing dev-tree project folders keep working. InitializeIcu prefers the running tree and ignores missing or source-tree registry ICU dirs. The Paratext plugin finder skips source-tree HKCU RootCodeDir/RootDataDir, and Convert.cs skips a source-tree v8 RootCodeDir. Repair-DeveloperRegistry.ps1 also clears the source-tree SilEncConverters40, EncodingConverterRepository and silfw:// keys, and keeps ProjectsDir. Both run orders are tested (FwUtilsTests 474, Paratext plugin tests 38, repair script 22 checks, all passing); installed 9.3.10 still needs the script once.

@johnml1135
johnml1135 force-pushed the fix/LT-22840-dev-registry branch from c9bf157 to 19d5858 Compare October 23, 2026 11:18

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The HKCU override is deliberate. Per-user RootCodeDir, RootDataDir and ProjectsDir values let a user redirect an installation without touching HKLM, and that capability was designed specifically for FieldWorks in grad lab courses and for user support. Developer machines use the same mechanism on purpose (see the note at FieldWorks.cs:3574), and installing a release on a developer machine has never been a supported setup. I don't want the installed readers changed to accommodate that setup.

The testing that found this doesn't need code changes. To check LT-22836 against 9.3.10, any of these keeps the installed build away from your dev keys:

  • Windows Sandbox or a VM, where HKCU starts clean
  • a second local Windows account, since HKCU is per user
  • reg export the FieldWorks\9 key, remove the values, test, then reg import
  • a dev build of the release/9.3.10 branch, which uses its own DistFiles since #1093

LT-22840 notes that the reader change can't fix the installed 9.3.10 anyway, so that machine needs the cleanup either way.

The reader changes also break cases the override serves:

  • ParatextLexiconPluginDirectoryFinder has no dev-tree detection, so a plugin loaded by Paratext now ignores an HKCU RootCodeDir that points at a source tree.
  • Native DirectoryFinder::FwRootDataDir in Util.cpp still reads HKCU first, so managed and native code would disagree on the data directory.
  • In a dev run, Project Location with "move" moves every project out of the HKCU ProjectsDir, but the new path lives only in memory. The next launch, dev or installed, finds an empty folder.

Please drop the reader filters and the ProjectsDirectory change, and keep the mkall.targets registry targets until someone runs the new-machine setup test that Hasso's note asks for. A smaller PR with the read-only Resolve-FieldWorksDevRegistry.ps1, the InitializeIcu fix for a missing ICU folder, and the test initializer change would be welcome. The repair script can go with it as an opt-in tool, but it should leave SilEncConverters40\DeveloperPluginDir alone.

This review was assisted by Claude Opus 5.5.

@johnml1135
johnml1135 force-pushed the fix/LT-22840-dev-registry branch from 19d5858 to a8e013b Compare October 30, 2026 11:55
@johnml1135

Copy link
Copy Markdown
Contributor Author

Shrunk as you asked. The branch is now three commits on the original base:

  • Test initialization no longer overwrites the shared code and data paths (14ec3d3).
  • InitializeIcu skips a user ICU directory that doesn't exist and falls through to an existing machine directory, then the existing development probe (1347967).
  • Resolve-FieldWorksDevRegistry.ps1 stays read-only, and the repair script is opt-in, preview first. It now leaves ProjectsDir and SilEncConverters40\DeveloperPluginDir alone, and its mocked test checks both survive (a8e013b).

Dropped: the installed-reader filters, the Paratext finder change and the ProjectsDirectory change, so HKCU precedence is as on main and managed and native readers agree again. The mkall.targets registry targets and Hasso's note are restored unchanged.

On Windows: the build passes; Repair-DeveloperRegistry.Tests.ps1 passes in PowerShell 5.1 and 7; FwUtilsTests passes apart from four DebugProcsTests.GetMessage_* failures, which come from an LF checkout of a file this PR doesn't touch (they pass on main in a CRLF checkout). I haven't run the full test.ps1 or a manual Project Location move.

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.

3 participants