Refactor: Modify detection process for Inno Setup 6 #303
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Inno Setup 6 compiler detection by adding a fallback mechanism. Instead of immediately failing when the default installation directory doesn't exist, the code now attempts to find and use ISCC from the system PATH before throwing an error.
Key changes:
- Moves script file creation to the beginning of the method
- Adds conditional logic to check for the default installation path first
- Implements PATH-based fallback for ISCC execution when default path is not found
- Updates error message to reflect both possible failure scenarios
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw Exception( | ||
| '`Inno Setup 6` was not installed or ISCC is not in PATH.'); | ||
| } |
There was a problem hiding this comment.
The temporary file is not cleaned up when an exception is thrown. If the compiler is not found in PATH (line 28-30), the exception is thrown before file.deleteSync() is called (line 37). This will leave temporary script files on disk. Consider using a try-finally block to ensure cleanup, or moving file deletion earlier to handle all error paths.
packages/flutter_app_packager/lib/src/makers/exe/inno_setup/inno_setup_compiler.dart
Outdated
Show resolved
Hide resolved
| if (innoSetupDirectory.existsSync()) { | ||
| // Use ISCC from the default installation directory | ||
| processResult = await $( | ||
| p.join(innoSetupDirectory.path, 'ISCC.exe'), | ||
| [file.path], | ||
| ); | ||
| } else { | ||
| // Fall back to PATH | ||
| try { | ||
| processResult = await $('ISCC', [file.path]); | ||
| } on ProcessException { | ||
| throw Exception( | ||
| '`Inno Setup 6` was not installed or ISCC is not in PATH.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
The error handling logic is incomplete. When the default installation path exists but ISCC.exe is not found or fails to execute (e.g., due to missing file), a ProcessException will be thrown but won't be caught. This will result in an uncaught exception instead of the clear error message intended for users. The try-catch should wrap both branches, or the first branch should also have explicit error handling.
…no_setup_compiler.dart Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
This pull request improves the robustness of the
InnoSetupCompilerby adding a fallback mechanism for locating the Inno Setup compiler executable. Instead of failing if the default installation directory is not found, the code now attempts to useISCCfrom the systemPATHbefore throwing an error.Improvements to Inno Setup compiler execution:
compilemethod ininno_setup_compiler.dartto first try runningISCC.exefrom the default installation directory, and if not found, fall back to invokingISCCfrom the systemPATH. Throws a clear exception if neither is available.