packaging(installer): make openads-setup.iss compile (brace comment + deprecated arch)#96
Conversation
…ated arch)
The review-feedback commit left the script unable to compile with ISCC
(CI does not build the installer, so it went unnoticed):
- The data-directory comment was a `{ ... }` brace comment that contained
the literal `{autopf}` constant. Inno Setup brace comments do not nest, so
the `}` of `{autopf}` closed the comment early and the rest parsed as code
("Identifier expected", line 87). Rewritten as `//` line comments without
curly braces.
- ArchitecturesInstallIn64BitMode=x64 is deprecated in Inno Setup 6 (warns,
substitutes x64os). Use the recommended x64compatible.
Verified: ISCC now compiles a clean openads-<ver>-setup.exe with zero
warnings/errors (staging stub).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Windows installer configuration (openads-setup.iss) by changing ArchitecturesInstallIn64BitMode to x64compatible and replacing brace comments with line comments to prevent parsing issues in Inno Setup. The reviewer suggested also adding ArchitecturesAllowed=x64compatible to explicitly prevent the installer from running on 32-bit Windows systems, as only 64-bit binaries are distributed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ; Service registration + Program Files install need elevation. | ||
| PrivilegesRequired=admin | ||
| ArchitecturesInstallIn64BitMode=x64 | ||
| ArchitecturesInstallIn64BitMode=x64compatible |
There was a problem hiding this comment.
Since OpenADS only distributes 64-bit binaries (such as openads_serverd.exe and ace64.dll), the installer should be restricted from running on 32-bit Windows systems to prevent broken installations. Consider adding ArchitecturesAllowed=x64compatible to enforce this.
ArchitecturesAllowed=x64compatible
ArchitecturesInstallIn64BitMode=x64compatible
1 participant
The Windows installer script merged in #94 does not compile with Inno Setup's
ISCC (CI does not build the installer, so it went unnoticed):
Error on line 87: Identifier expected.— the data-directory comment is a{ ... }brace comment that contains the literal{autopf}constant. InnoSetup brace comments do not nest, so the
}of{autopf}closes the commentearly and the remainder parses as code. Rewritten as
//line commentswithout curly braces.
ArchitecturesInstallIn64BitMode=x64is deprecated in Inno Setup 6 (warns,silently substitutes
x64os). Use the recommendedx64compatible.Verified locally:
ISCC.exe /DAppVer=1.4.0 /DSrcDir=<staging> openads-setup.issnow produces
openads-<ver>-setup.exewith zero warnings and zero errors.No behavior change to the wizard, file list, ini writer or service handling.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com