Skip to content

feat(windows): guard delphi_msbuild against silent Delphi CE failures#16191

Open
MattGyverLee wants to merge 14 commits into
keymanapp:masterfrom
MattGyverLee:chore/windows/delphi-ce-guard
Open

feat(windows): guard delphi_msbuild against silent Delphi CE failures#16191
MattGyverLee wants to merge 14 commits into
keymanapp:masterfrom
MattGyverLee:chore/windows/delphi-ce-guard

Conversation

@MattGyverLee

@MattGyverLee MattGyverLee commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Third and smallest of the Delphi 12 CE workflow PRs. Detects Delphi Community Edition via a dcc32.exe probe and aborts delphi_msbuild with a clear message when the developer is on CE but hasn't set KEYMAN_DELPHI_CE=1. Turns the single most confusing failure mode in the CE workflow into a loud, obvious error.

The problem

If a contributor is on Delphi 12 CE and forgets to export KEYMAN_DELPHI_CE=1 in their shell (easy to do — SETX only affects new shells), delphi_msbuild falls through to msbuild.exe. On CE, msbuild reports:

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.23

...and produces no output. Downstream cp, mv, mt.exe, sentrytool_delphiprep steps then fail with "file not found" errors pointing at paths Delphi never wrote to. Diagnosing this from the symptom takes considerable time — you chase the missing file assuming Delphi's build worked, when really Delphi never ran.

Discovered while running the CE workflow walkthrough for #16044; every "Delphi silently succeeded but no output" issue I hit traced back to a lost KEYMAN_DELPHI_CE env var.

What the change does

Adds a one-time dcc32.exe probe in resources/build/win/environment.inc.sh:

  • CE's dcc32.exe is a 23KB stub that prints This version of the product does not support command line compiling. and exits non-zero.
  • Pro/Enterprise's dcc32.exe is the real compiler and accepts flags.

_probe_delphi_ce runs dcc32.exe once, captures stderr, and sets _KEYMAN_DELPHI_IS_CE=1 if the marker string is present. Result cached per shell — subsequent delphi_msbuild calls don't re-spawn dcc32.

delphi_msbuild then checks: if we're not in CE mode (KEYMAN_DELPHI_CE != 1) but the probe detected CE, builder_die with a message pointing at docs/build/windows-delphi-ce.md.

CI impact

None. CI runs on Delphi 10.3 Pro. The probe hits Pro's real dcc32.exe, doesn't see the CE marker, sets _KEYMAN_DELPHI_IS_CE="", and delphi_msbuild proceeds to msbuild normally. The added overhead is one dcc32 spawn per build.sh invocation — trivial.

Relates-to / Depends-on

Relates-to: #4599
Depends-on: #16044 (that PR introduces the KEYMAN_DELPHI_CE=1 env var and the CE prompt branch this guard sits alongside)

Once #16044 lands, this can merge without conflicts (based on that branch).

User Testing

User testing not required: build-environment change with no runtime behaviour modification. CI exercises the Pro-path.

MattGyverLee and others added 14 commits July 2, 2026 13:42
@MattGyverLee
chore(windows): Delphi 11/12 source compatibility
ebc6d2a 
Lets the existing tree compile under Delphi 11 (VER350) and Delphi 12
(VER360) without breaking Delphi 10.3 (VER330). CI is unchanged: with
KEYMAN_DELPHI_VERSION unset, every build script keeps defaulting to
Delphi 10.3 (BDS 20.0) exactly as before.

This is a bridge to lower contributor friction on keymanapp#4599 (deprecate
Delphi). The 10.3 build server can also be upgraded to a paid Delphi
11/12 license on top of these patches if/when that's wanted.

Build-script knob:
* resources/build/win/configure_environment.inc.sh,
  resources/build/win/delphi_environment.inc.sh: DELPHI_VERSION now
  reads ${KEYMAN_DELPHI_VERSION:-20.0}.
* resources/builder.inc.sh: builder_describe_platform's delphi tool
  detection respects the same KEYMAN_DELPHI_VERSION default, so
  machines with only Delphi 12 installed no longer have win,delphi-
  gated targets silently skipped.

Keyman-owned source compat (additive VER350/VER360 arms; VER330 paths
untouched):
* common/windows/delphi/tools/devtools/SourceRootPath.pas: teach
  DelphiMajorVersion about BDS 22.0 / 23.0 install dirs.
* common/windows/delphi/web/Keyman.System.HttpServer.Base.pas: extend
  the IFNDEF VER330 tripwire to accept VER340/350/360.
* common/windows/delphi/components/FixedTrackbar.pas: same tripwire
  extension.
* common/windows/delphi/general/CleartypeDrawCharacter.pas:
  EnumFontFamiliesEx integer-return comparison on VER340/350/360 (was
  VER340-only).
* common/windows/delphi/general/JsonUtil.pas: pass [] options arg to
  TJSONAncestor.ToChars on VER350/360 (Delphi 11+ added the parameter).

Vendored third-party patches (each hunk annotated with a "Keyman local
patch" comment plus refresh strategy):
* developer/src/ext/jedi/jcl/jcl/source/common/JclSynch.pas: wrap
  Boolean args to CreateEvent / OpenEvent / CreateWaitableTimer /
  OpenWaitableTimer / OpenSemaphore / CreateMutex / OpenMutex in
  explicit BOOL() casts (Delphi 12 tightened implicit Boolean->BOOL at
  call sites); switch JclWin32.CreateMutex -> Winapi.Windows.CreateMutex
  to match the file's existing pattern for the other Winapi calls.
* developer/src/ext/jedi/jvcl/jvcl/run/JvComponent.pas: on VER350+,
  unconditionally call DoCreate (Embarcadero removed the
  OldCreateOrder property in Delphi 11; modern behavior is equivalent
  to OldCreateOrder=True).
* developer/src/ext/mbcolor/mxs.inc: add VER350/VER360 blocks defining
  DELPHI_5_UP through DELPHI_10_UP. Without these, HTMLColors.pas
  silently drops Variants from its uses clause and breaks with
  "Undeclared identifier: 'null'".

.gitignore: add patterns for per-arch version*.res and meson
wraplock files.

Relates-to: keymanapp#4599
@MattGyverLee @claude
docs(windows): surface KEYMAN_DELPHI_VERSION in windows.md
a57500f 
Pro-tier Delphi 11/12 users had no way to discover the env-var knob
introduced by this PR - it was documented only in the PR body and in
the CE-workflow doc from keymanapp#16044. Add a short paragraph to the Delphi
requirements section pointing at the two Studio path values and
reaffirming that the unset default preserves CI behavior.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
revert: drop unnecessary JclSynch.pas Boolean/BOOL cast patches
b611e81 
Per @mcdurdin's review: he built with unmodified JclSynch.pas and
verified the Delphi 12 compile succeeds without the BOOL casts.
The patches were defensive against a compile failure I never
actually observed on this file — I assumed Delphi 12's tightened
implicit-Boolean handling required them without empirically
checking. Removing.

If a real compile failure on JclSynch shows up later, address it
then with a specific error to fix rather than a preemptive patch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
revert: drop JvComponent.pas OldCreateOrder patch pending verification
d7b3784 
Per @mcdurdin's review: the OldCreateOrder-was-removed premise
disagrees with the Athens documentation he linked
(https://docwiki.embarcadero.com/Libraries/Athens/en/Vcl.Forms.TForm.OldCreateOrder).
Given JclSynch.pas turned out to be an unnecessary defensive patch,
this one deserves the same skepticism. Reverting until a specific
Delphi 12 compile failure on JVCL surfaces that motivates a
targeted fix.

The remaining vendored ext patch (mbcolor/mxs.inc) stays because it
produces a concrete E2003 Undeclared identifier: 'null' at
HTMLColors.pas:290 on Delphi 12 — reproducible, not defensive.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
address remaining review points: tripwire acknowledgements, scope
9d53e77 
- FixedTrackbar.pas: extend the existing 'Tested on' comment block
  to note VER350/VER360 IFNDEF arms are added without full
  verification against Vcl.Grids.pas — same self-documentation
  pattern @mcdurdin used when he added the 10.3 attestation line
  in 2019.
- HttpServer.Base.pas: extend the pre-existing "may need to check"
  comment to name the newly-silenced tripwire arms and record that
  Indy's URL handling on 10.4/11/12 has not been re-verified. The
  workaround stays applied conservatively rather than removed
  optimistically.
- Drop `.wraplock` .gitignore rule — Meson subproject artifact,
  no connection to Delphi 11/12 source compat. Scope drift.
- windows.md: trim the "so CI is unaffected" tail from the
  KEYMAN_DELPHI_VERSION paragraph. CI default is a separate
  concern from the env-var default.

Motivated by the same "no unverified defensive extensions" concern
that led me to revert the JclSynch and JvComponent patches earlier.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
docs(windows): sync emsdk install version with the pinned minimum
736afd9 
resources/build/minimum-versions.inc.sh pins KEYMAN_MIN_VERSION_EMSCRIPTEN
at 3.1.64, but the windows.md walkthrough still told contributors to
`emsdk install 3.1.58`. That version now fails to compile core/src/wasm.cpp
because a recent Core commit (9909d7d "expose km_core_state_options_update
to WASM") uses stricter emscripten pointer-binding APIs. Bumping the docs
to match the pin.

Discovered while running the CE build walkthrough on this branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
fix(devtools): tolerate Delphi 12 EnvOptions.proj Null attrs and Win64x
9ef5278 
Two Delphi 12 dev-env issues in devtools' -ai / TIncludePaths.Reset when
processing %AppData%\Embarcadero\BDS\23.0\EnvOptions.proj:

1. Delphi 12 emits an empty <PropertyGroup/> at the top of the file with
   no Condition attribute. The pre-existing VarIsNull guard didn't
   short-circuit reliably under Delphi 12's variant handling, so a Null
   Attributes['Condition'] hit Pos() and raised EVariantTypeCastError.
   Replace with VarToStrDef(..., '') for a defensive Variant->string
   coercion that handles Null and Empty.

2. The Win32/Win64 match was Pos('Win32', ...) — a substring match that
   also matches 'Win64x' (a Delphi 12 target now present in the file).
   Building against the Win64x PropertyGroup then hit an empty
   <DelphiBrowsingPath/> and again produced Null. Match with the
   surrounding literal single quotes ('''Win32''' / '''Win64''') so
   'Win64x' no longer matches, and additionally wrap the NodeValue
   reads in VarToStrDef so empty child elements degrade cleanly.

Applied to both AddPathToProjectXML (the -ai/-ip path) and Reset (the
-ri path) — same code shape in both.

Found while running the CE build workflow against Delphi 12 CE — the
tripwire fired at cef4delphi's `devtools -ai` step and blocked the
engine build entirely until this fix landed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
fix(global): let CustomisationStorage compile under Delphi 12 dcc64
d69d9b7 
TCustFileList.AddCustFile used a nested expression the Delphi 12 Win64
compiler rejects with E2010 'Incompatible types: TCustFile and TObject':

    Result := Items[inherited Add(FObjectClass.Create)];

dcc32 (Win32) accepted the same source, so the file has been building
fine for CI's default 10.3 flow. On Delphi 12 dcc64, the inline chain
loses the widening from TCustFile (returned by FObjectClass.Create) to
TObject (Add's parameter). Splitting the object creation into a local
TObject variable lets the widening happen explicitly and both compilers
accept the code without any IFDEF.

Found while building the Win64 half of kmcomapi.dproj as part of the
CE workflow walkthrough — the Win32 half compiled cleanly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee
feat(windows): Delphi CE interactive build workflow
ecc0aa8 
Lets developers on the free Delphi Community Edition tier build the
still-Delphi parts of the tree, even though CE 11+ blocks the dcc32
CLI that build.sh normally drives. None of the compiler / runtime
stack moves; this is purely build-environment work plus docs.

CI is unchanged. KEYMAN_DELPHI_CE is unset by default, so build.sh
keeps invoking msbuild.exe exactly as before.

Depends on the Delphi 11/12 source-compat work in a sibling PR — that
PR adds the VER350/VER360 IFDEF arms and KEYMAN_DELPHI_VERSION knob
needed for the IDE to actually compile.

Build-script knob:
* resources/build/win/environment.inc.sh: delphi_msbuild() now checks
  KEYMAN_DELPHI_CE=1. When set, it prints an IDE-build prompt naming
  the .dproj and waits for Enter instead of running msbuild.exe. The
  pre-build (rc.exe) and post-build (binary copy, codegen) steps in
  build.sh run normally around the prompt, so the developer only has
  to drive the Delphi compile by hand.

Docs:
* docs/build/windows.md: one-line pointer to windows-delphi-ce.md from
  the existing "Delphi 10.3 CE no longer available" warning, so future
  CE users find the workaround instead of dead-ending.
* docs/build/windows-delphi-ce.md: ~940-line guide -- prerequisites,
  registry library-path setup, source patches required on the sibling
  branch, the canonical build order (including the tsysinfox64 ->
  .bin -> tsysinfo_x64.res chicken-and-egg), install-and-overlay
  workflow, debugging, and a troubleshooting catalog. Written against
  Delphi 12 Athens CE specifically (currently the only free tier);
  Delphi 11 CE works with the same approach modulo a couple of paths.

engine.groupproj fix (independent of the Delphi work but unblocks IDE
"Build All", which CE users rely on):
* windows/src/engine/engine.groupproj: drop the inst\insthelper
  reference. The path doesn't resolve -- insthelper.dproj lives at
  windows/src/engine/insthelper, not .../inst/insthelper -- and IDE
  "Build All" was failing on it. build.sh-based builds were never
  affected because they don't walk the groupproj.

.gitignore: add patterns for the per-script log files that the CE
workflow writes (configure.log, core-build.log, etc.).

Relates-to: keymanapp#4599
@MattGyverLee @claude
docs: rework windows-delphi-ce.md per review feedback
d36fc00 
Address @mcdurdin's "redundant and incorrect information" review.
Drop 836 lines. Trim scope to what this PR actually introduces
(KEYMAN_DELPHI_CE=1 interactive prompt) plus the CE-specific
prerequisites the standard windows.md doesn't cover:

- Remove content that belongs in PR keymanapp#16043 (the source-compat
  patches for JCL, JVCL, mbcolor, SourceRootPath, HttpServer,
  FixedTrackbar, CleartypeDraw, JsonUtil). Point readers there.
- Delete the pre-PR engine.groupproj / insthelper workaround
  section - this PR removes that reference, so the workaround
  no longer applies to trees on this branch.
- Drop internal memory-system references ([[double-bracket-wiki]])
  that would render as literal text on GitHub.
- Fix the VER340->'21.0' factual error (it's Delphi 10.4's real
  Studio path, not a CE-license rev bump).
- Collapse duplicated content: uiAccess/kmshell-path explanation,
  install-Keyman-19 step, revert-before-PR warning, .res file
  handling, engine.groupproj discussion.
- Fix broken cross-references to "section 5.2 step (a)/(b)" and
  "BSD block" that had no matching structure.
- Consolidate build order and shell workflow into one section.
- Convert the a-f cross-project dependency graph into a table for
  scanability.

Doc now ~270 lines vs. 939. Scope matches the PR: what
KEYMAN_DELPHI_CE=1 does, plus the CE-only setup (Library Search
Paths, Keyman 19 install requirement, dependency ordering under
IDE prompts).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
docs: address @mcdurdin's remaining review comments
83a61b8 
Working through the inline comments that the initial rework didn't
already resolve:

- Fix TKeymanPaths.KeymanDesktopInstallPath() explanation: the path
  is read from a registry key the installer populates, not
  hardcoded. Reword the prereq accordingly and the corresponding
  troubleshooting entry.
- Soften "Keyman 19 required" to "strongly recommended". It is
  possible to debug without an official install; the install just
  simplifies the setup by giving you a working system to overlay
  onto. Also fix the URL (keyman.com/windows, not /desktop).
- Switch env-var setup to SETX to match the style used throughout
  windows.md.
- Replace the manual PowerShell manifest.in patch with
  `./build.sh debug-manifest`, which swaps in the pre-existing
  debug-manifest.in template that already exists for this purpose.
- Fix dependency (e) failure description: the runtime failure
  appears when launching Keyman, not on enable-keyboard.
- Trim the debugging section: drop the incorrect claim about symbol
  behavior and note that C++/Delphi cross-stepping is out of scope
  (windbg territory).
- Drop the canonical numbered build order - the script prompts the
  developer in the right order already, the list was redundant.
- Drop §8 "reverting before a PR" - covered by the §5 revert note
  and §3 backup restore.
- Remove the local-workflow entries from .gitignore
  (delphi-library-paths.backup*.reg, local-delphi-12-patches.patch,
  the six *.log entries). These files aren't generated by any repo
  script - they were leftovers from my local build workflow.

Doc now ~245 lines vs. 272 in the first rework, ~939 originally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
docs: correct DELPHIINCLUDES mirroring claim, tighten cross-refs
9603b1b 
- §3 registry recipe: only the first six paths mirror DELPHIINCLUDES
  in delphi_flags.inc.sh (verified: that variable defines exactly
  those six). The remaining eleven JCL/JVCL/jedi paths are IDE-only
  and have no equivalent in the CLI build path; my earlier claim
  that all seventeen mirror DELPHIINCLUDES was wrong.
- Replace two "see PR keymanapp#16043" references with pointers to
  windows.md's Delphi requirements section, which will remain the
  canonical entry point after 16043 merges (PR-number cross-refs
  age badly).
- §6 debugging: add one-line acknowledgment that the content isn't
  CE-specific but has no other current home, per review feedback
  that the section drifts from the doc's stated scope.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
docs: harvest CE workflow gaps discovered during real build walkthrough
b6b9b7e 
Ran the full CE workflow end-to-end against Delphi 12 CE and hit six
distinct failure modes not previously covered in the doc. Adding each
as either a §1/§2/§4 setup step or a §7 troubleshooting entry so the
next contributor doesn't rediscover them.

Setup steps added to §1 Prerequisites:
- Test-signing certificates one-time setup via
  common/windows/delphi/tools/certificates/build.sh certificates.
  Skipping this makes every wrap-signcode call fail with
  'SignTool Error: File not found: keymantest-sha1.pfx'.

Prominent CAUTION added to §2 Environment variables:
- If KEYMAN_DELPHI_CE isn't set in the current shell, delphi_msbuild
  falls through to msbuild.exe, which on CE produces a fake
  'Build succeeded... Time Elapsed 00:00:0X.XX' with no output. The
  downstream cp/mv/mt.exe/sentrytool then fail on paths where Delphi
  never actually wrote anything. This is by far the most confusing
  failure mode in the whole workflow. Document the verification
  command and require it in every fresh shell.

§4 build order fixes:
- Row (a) was incomplete: MessageIdentifierConsts.pas is not
  produced by devtools/build.sh, it's produced by
  windows/src/global/delphi/build.sh, which also produces
  keyman_components.bpl. Rewrote the row to name the actual
  producer script and split the setup-locale codegen into row (a').
- Row (c): named tsysinfo/build.sh as the orchestrator (the .dproj
  and .groupproj don't do the tsysinfox64->exe->res chain; the
  build.sh do_build does).
- New sub-section: multi-platform Delphi packages (kmcomapi builds
  once for Win32 and once for Win64 - toggle Delphi's Platform
  dropdown between the two consecutive CE prompts).
- New sub-section: full-clean bin/obj when sentrytool_delphiprep
  complains about residual debug sections.

§7 troubleshooting additions covering:
- 'Build succeeded... Time Elapsed' + downstream missing-file (the
  env-var-lost signature)
- 'SignTool Error: keymantest-sha1.pfx' (certificates missing)
- 'This executable has a debug section. Not able to update' (partial
  prior build)
- 'mv: cannot stat bin/Win64/Debug/...dll' (Platform toggle missed)
- 'EVariantTypeCastError' and 'E2010 TCustFile and TObject' pointing
  to the two Delphi-12-specific source-compat patches now in keymanapp#16043.

Doc grew 253 -> 359 lines. All additions are grounded in real
failures observed in the walkthrough, not speculation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@MattGyverLee @claude
feat(windows): guard delphi_msbuild against silent CE build failures
52ba14e 
If a developer is on Delphi Community Edition but forgets to set
KEYMAN_DELPHI_CE=1, `delphi_msbuild` falls through to msbuild.exe,
which — on CE — reports "Build succeeded" and a small Time Elapsed
line but produces no output. Downstream cp/mv/mt.exe/sentrytool
then fail on paths where Delphi never actually wrote anything, and
diagnosing the root cause takes ~30 minutes of confused debugging.

Detect CE via a one-time dcc32.exe probe (dcc32 on CE prints
"This version of the product does not support command line compiling"
and exits non-zero; on Pro it accepts args). If CE is detected and
KEYMAN_DELPHI_CE isn't set, `builder_die` with a clear message
pointing at docs/build/windows-delphi-ce.md.

Result is cached in shell-local vars so subsequent delphi_msbuild
calls don't re-spawn dcc32.

CI unchanged: CI runs against Delphi 10.3 Pro where the probe
returns non-CE and delphi_msbuild proceeds normally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-project-automation github-project-automation Bot moved this to Todo in Keyman Jul 3, 2026
@keymanapp-test-bot keymanapp-test-bot Bot added has-user-test user-test-missing User tests have not yet been defined for the PR labels Jul 3, 2026
@keymanapp-test-bot

keymanapp-test-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

User Test Results

Test specification and instructions

ERROR: user tests have not yet been defined

@keymanapp-test-bot keymanapp-test-bot Bot added this to the A19S32 milestone Jul 3, 2026
@keyman-server

keyman-server commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

This pull request is from an external repo and will not automatically be built. The build must still be passed before it can be merged. Ask one of the team members to make a manual build of this PR.

@MattGyverLee MattGyverLee mentioned this pull request Jul 3, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

has-user-test user-test-missing User tests have not yet been defined for the PR

Projects

Keyman
Status: Todo

Milestone

A19S32

Development

Successfully merging this pull request may close these issues.

2 participants

@MattGyverLee @keyman-server