Fix Inno Setup installer GUIDs (RFC #2519)#3058
Conversation
There was a problem hiding this comment.
PR Summary:
This PR fixes Inno Setup installer GUIDs to address RFC #2519. Previously, the build process generated new UUIDs for pyRevit and pyRevit CLI installers on every build, causing Windows to treat each version as a different application. This resulted in:
- Multiple entries in Add/Remove Programs
- Broken upgrade/uninstall behavior
The fix removes the dynamic UUID overwrites, allowing stable GUIDs defined in the .iss files to persist across builds.
Review Summary:
The PR correctly identifies and addresses the root cause by removing calls to _installer_set_uuid() for Inno Setup installers. However, there's a critical issue: the code still generates new random product codes and saves them to pyrevit-products.json, which is used for version detection and upgrade management. This creates a mismatch where the installed software has a stable GUID in the Windows registry, but the product database has a different GUID, breaking version detection and upgrade logic.
Knowledge utilized: Inno Setup AppId behavior (stable GUIDs required for Windows to recognize versions as the same app), pyRevit's product database usage for version tracking and IT admin tooling, and MSI vs Inno Setup product code requirements.
Recommendation: Address the critical comment before merging to ensure the product database correctly reflects the stable installer GUIDs.
Follow-up suggestions:
@devloai fix the critical issue in comment #1- Update the code to use stable GUIDs for the product database@devloai add constants for the stable GUIDs to configs.py- Implement the suggested configuration constants
| @@ -120,10 +120,6 @@ def set_product_data(_: Dict[str, str]): | |||
| pyrevit_pc = _get_new_product_code() | |||
| pyrevitcli_pc = _get_new_product_code() | |||
There was a problem hiding this comment.
Critical: Product codes mismatch will break version detection and upgrade logic
The change correctly removes the calls to _installer_set_uuid() to keep Inno Setup AppIds stable. However, pyrevit_pc and pyrevitcli_pc are still being generated as new UUIDs and saved to pyrevit-products.json via _update_product_data_file() calls on lines 137-138.
Why this is critical:
The pyrevit-products.json file is used for version detection and upgrade management (see web search results). IT admins query the Windows registry for the installed GUID and look it up in this file. The installer also checks for GUIDs in this file to find and uninstall previous versions.
Problem:
- Inno Setup
.issfiles now use stable GUIDs (never changing):- pyRevit:
f2a3da53-6f34-41d5-abbd-389ffa7f4d5f - pyRevit CLI:
9557b432-cf79-4ece-91cf-b8f996c88b47
- pyRevit:
- But
pyrevit-products.jsonreceives new random keys on every build - Result: Version detection fails because the registry GUID won't match the products.json GUID
- Result: Upgrades fail because the installer can't find the previous version's GUID in the database
Expected behavior:
For Inno Setup installers, the product codes saved to pyrevit-products.json should match the stable GUIDs hardcoded in the .iss files.
Suggested fix:
# Use stable GUIDs for Inno Setup installers, new codes for MSI
pyrevit_inno_pc = "f2a3da53-6f34-41d5-abbd-389ffa7f4d5f"
pyrevitcli_inno_pc = "9557b432-cf79-4ece-91cf-b8f996c88b47"
pyrevitcli_msi_pc = _get_new_product_code() # MSI requires new ProductCode per version
# update product info on MSI installer files (CLI only)
_msi_set_uuid(
pyrevitcli_msi_pc,
configs.PYREVIT_CLI_UPGRADE_CODE,
configs.PYREVIT_CLI_MSI_PROPS_FILES,
)
build_version = props.get_version()
# Save Inno Setup stable GUIDs to products database
_update_product_data_file(build_version, pyrevit_inno_pc)
_update_product_data_file(build_version, pyrevitcli_inno_pc, cli=True)Note: Only pyRevit CLI has MSI installers (PYREVIT_MSI_PROPS_FILES is empty). The MSI product code is separate from what goes in the products.json database, which should track the primary Inno Setup installer GUIDs.
actions
Quick Fix: Apply a small focused fix for this specific issue (takes < 1min).
Feedback: Rate this comment to help me improve future code reviews:
- 👍 Good - Helpful and accurate
- 👎 Poor - Wrong, unclear, or unhelpful
- Skip if you don't have any strong opinions either way.
| @@ -120,10 +120,6 @@ def set_product_data(_: Dict[str, str]): | |||
| pyrevit_pc = _get_new_product_code() | |||
| pyrevitcli_pc = _get_new_product_code() | |||
|
|
|||
There was a problem hiding this comment.
Minor: Misleading comment after removing Inno Setup UUID updates
The comment on line 123 (now line 127 in original) says "update product info on MSI installer files" but there's no longer a parallel comment for Inno Setup installers since those calls were removed.
For clarity, consider adding a brief comment explaining why Inno Setup UUIDs are NOT being updated:
pyrevit_pc = _get_new_product_code()
pyrevitcli_pc = _get_new_product_code()
# Inno Setup installers use stable GUIDs defined in .iss files (not updated here)
# update product info on MSI installer files
_msi_set_uuid(This makes it clear to future developers that the omission is intentional, not an oversight.
actions
Quick Fix: Apply a small focused fix for this specific issue (takes < 1min).
Feedback: Rate this comment to help me improve future code reviews:
- 👍 Good - Helpful and accurate
- 👎 Poor - Wrong, unclear, or unhelpful
- Skip if you don't have any strong opinions either way.
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1624-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1738-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1743-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1829-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1937-wip |
|
📦 New work-in-progress (wip) builds are available for 6.0.0.26032+1956-wip |
|
📦 New work-in-progress (wip) builds are available for 6.0.0.26032+2005-wip |
|
📦 New work-in-progress (wip) builds are available for 6.0.0.26032+2008-wip |
|
📦 New public release are available for 6.0.0.26032+2040 |
|
📦 New public release are available for 6.0.0.26032+2040 |
Issue
Addresses RFC #2519: "installers GUID must stay the same in order to detect versions of the same app."
Previously,
set_product_data(run viapyrevit set productsin CI) generated a new UUID for pyRevit and pyRevit CLI on every run and wrote it into the Inno Setup scripts (.issfiles) asMyAppUUID. As a result:Change
File:
dev/_release.pyIn
set_product_data, the two calls that overwrote the Inno Setup AppId were removed:_installer_set_uuid(pyrevit_pc, configs.PYREVIT_INSTALLER_FILES)_installer_set_uuid(pyrevitcli_pc, configs.PYREVIT_CLI_INSTALLER_FILES)The
.issfiles already define stable GUIDs; they are no longer overwritten on each build:f2a3da53-6f34-41d5-abbd-389ffa7f4d5f9557b432-cf79-4ece-91cf-b8f996c88b47Unchanged:
_msi_set_uuid) is unchanged_update_product_data_file) are unchanged_installer_set_uuidhelper remains in the codebase for possible future useResult