Fix #4850: stop auto-creating ghost l10n bundles in wrong module#4867
Merged
Conversation
The CSS subprocess (CN1CSSCLI forked by CSSWatcher) inherits cwd =
`javase/` from the simulator. JavaSEPort.findLocalizationDirectory was
looking only at cwd and -- when nothing was found -- mkdirs()'d a
throwaway `javase/src/main/l10n/Bundle.properties`. Older CN1 versions
poisoned that ghost bundle with `@im=@im` via the AutoLocalizationBundle
wormhole. After users cleaned the *real* bundle in `common/src/main/l10n`
per liannacasper's workaround, every CN1CSSCLI respawn still loaded the
ghost bundle from `javase/`, crashed inside parseTextFieldInputMode, and
CSSWatcher.start() respawned the dead subprocess in an infinite loop --
hence "fix worked, simulator doesn't crash, but `CSS> ...` stack trace
keeps spamming the log" (Ngosti2000 + ThomasH99).
New rules for findLocalizationDirectory:
1. Check the current module first (cwd/src/main/{l10n,i18n}).
2. Walk up to the sibling `common/` module (matches CN1 maven layout
and CSSWatcher.addLocalizationCandidates which already does this).
3. Never auto-create the directory. If the developer hasn't set up
localization, the auto-bundle is a no-op -- project-level opt-in.
findDefaultLocalizationBundleFile no longer falls back to a non-existent
`Bundle.properties` path either. Combined with rule 3 above, no l10n
files on disk = enableAutoLocalizationBundle returns early at
`bundleFile == null`, and ThomasH99's "the auto-bundle fills up by
itself" complaint goes away for projects that didn't ask for it.
Both methods get static overloads taking an explicit projectDir so
unit tests can drive them without mutating user.dir. The instance
methods become thin wrappers around the static versions.
AutoLocalizationBundleTest gains three regression tests:
- verifyFindLocalizationDirectoryDoesNotAutoCreate: no l10n dir =>
returns null AND src/main/l10n is not created on disk.
- verifyFindLocalizationDirectoryWalksToCommonSibling: cwd=javase
with sibling common/src/main/l10n => returns common's dir; once
javase/src/main/l10n exists, local wins.
- verifyFindDefaultBundleReturnsNullWhenNoBundleFile: empty l10n
dir => returns null and Bundle.properties is not created.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 7 screenshots: 7 matched. |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
iOS screenshot updatesCompared 75 screenshots: 74 matched, 1 updated.
Benchmark Results
Build and Run Timing
|
Collaborator
Author
|
Compared 86 screenshots: 86 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
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
1 participant
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 the respawn loop reported on issue #4850 by ThomasH99 and Ngosti2000 -- the "simulator doesn't crash but the same
CSS> ...stack trace keeps spamming the log" state they hit even after applying liannacasper's manual workaround.Root cause (the part the previous fixes missed)
The
CN1CSSCLIsubprocess thatCSSWatcherforks inherits cwd =javase/from the simulator.JavaSEPort.findLocalizationDirectorylooked only at cwd and -- when nothing was found --mkdirs()'d a throwawayjavase/src/main/l10n/Bundle.properties. Older CN1 versions (< 7.0.237) poisoned that ghost bundle with@im=@imvia theAutoLocalizationBundlewormhole.After users cleaned the real bundle in
common/src/main/l10nper liannacasper's instructions, everyCN1CSSCLIrespawn still loaded the ghost bundle injavase/, crashed insideparseTextFieldInputMode("@im-@im"), andCSSWatcher.start()(Ports/JavaSE/.../CSSWatcher.java:376-379, 415-418) respawned the dead subprocess with no backoff. Hence the loopingCSS>stack trace seen in Ngosti2000's video.Fix
findLocalizationDirectorynow:cwd/src/main/{l10n,i18n}).common/module (matches the CN1 maven layout andCSSWatcher.addLocalizationCandidates, which already does this).findDefaultLocalizationBundleFileno longer returns a non-existentBundle.propertiespath as a fallback. Combined with rule 3, no l10n files on disk =enableAutoLocalizationBundlereturns early atbundleFile == nulland there's no surprise bundle materialising.Both methods get static overloads taking an explicit
projectDirso the regression tests can drive them without mutatinguser.dir. The instance methods become thin wrappers.Test plan
AutoLocalizationBundleTestgains three regression tests:verifyFindLocalizationDirectoryDoesNotAutoCreate-- no l10n dir => returns null ANDsrc/main/l10nis not created on disk.verifyFindLocalizationDirectoryWalksToCommonSibling-- cwd=javasewith siblingcommon/src/main/l10n=> returns common's dir; oncejavase/src/main/l10nexists, local wins.verifyFindDefaultBundleReturnsNullWhenNoBundleFile-- l10n dir present but empty => returns null andBundle.propertiesis not created.mvn -pl javase install -Plocal-dev-javasebuilds clean.archetype-smoke.ymljob already exercises the auto-bundle code path with the pref forced on -- this PR doesn't change that surface.Migration note for affected users
After upgrading,
javase/src/main/l10n(and the poisonedBundle.propertiesinside) is no longer touched, but the directory created by previous versions still exists. Users canrm -rf <module>/javase/src/main/l10nto clean up the ghost bundle. The5de1898d1self-heal already handles legacy poisoned files in real l10n dirs going forward.🤖 Generated with Claude Code