Skip to content

Conversation

@AlexKlimaj
Copy link
Contributor

Attempt at fixing #13615

@HTRamsey HTRamsey requested a review from Copilot November 14, 2025 05:34
Copilot finished reviewing on behalf of HTRamsey November 14, 2025 05:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses issue #13615 by adding logic to handle cases where the Windows uninstaller executable is missing despite registry entries still existing. The installer now checks if the uninstaller file exists before attempting to run it, and cleans up orphaned registry keys if the file is not found.

Key Changes:

  • Added checkUninstaller section to verify uninstaller file existence before execution
  • Implemented quote removal logic to extract the file path from the UninstallString registry value
  • Added cleanupOrphanedRegistry section to delete orphaned registry keys when uninstaller is missing

Comment on lines +70 to +73
StrCpy $R1 $R0 "" 1 ; Skip first quote
StrLen $R2 $R1
IntOp $R2 $R2 - 1 ; Remove last quote
StrCpy $R1 $R1 $R2
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The quote removal logic assumes the UninstallString always has quotes, but doesn't verify this assumption. If the registry value lacks quotes (e.g., from a corrupted registry or different installer version), this code will incorrectly trim the first and last characters of the actual path.

Consider using a defensive approach that checks for quotes before removing them:

StrCpy $R1 $R0 1       ; Get first character
StrCmp $R1 '"' +1 +4   ; If it's a quote, remove quotes
StrCpy $R1 $R0 "" 1    ; Skip first quote
StrLen $R2 $R1
IntOp $R2 $R2 - 1      ; Remove last quote
StrCpy $R1 $R1 $R2
Goto +2
StrCpy $R1 $R0         ; No quotes, use path as-is
Suggested change
StrCpy $R1 $R0 "" 1 ; Skip first quote
StrLen $R2 $R1
IntOp $R2 $R2 - 1 ; Remove last quote
StrCpy $R1 $R1 $R2
StrCpy $R1 $R0 1 ; Get first character
StrCmp $R1 '"' +1 +4 ; If it's a quote, remove quotes
StrCpy $R1 $R0 "" 1 ; Skip first quote
StrLen $R2 $R1
IntOp $R2 $R2 - 1 ; Remove last quote
StrCpy $R1 $R1 $R2
Goto +2
StrCpy $R1 $R0 ; No quotes, use path as-is

Copilot uses AI. Check for mistakes.

cleanupOrphanedRegistry:
DetailPrint "Previous uninstaller not found, cleaning up orphaned registry keys..."
SetRegView 64
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

[nitpick] The SetRegView 64 call is redundant here as it was already set at line 64 before reading the registry. This line can be removed to avoid unnecessary duplication.

Suggested change
SetRegView 64

Copilot uses AI. Check for mistakes.
@HTRamsey
Copy link
Collaborator

Did this end up working out?

@AlexKlimaj
Copy link
Contributor Author

I do notice that the daily no longer checks for the 32 bit uninstaller.

This PR
image

Release
image

Daily with release installed with uninstaller renamed
image

This PR with uninstaller renamed
image

@HTRamsey HTRamsey merged commit be3e6a2 into mavlink:master Nov 15, 2025
11 checks passed
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.

2 participants