Skip to content

installer: use modern innosetup features - #593

Merged
dscho merged 1 commit into
git-for-windows:mainfrom
rimrul:modern-inno-features
Sep 11, 2026
Merged

dscho merged 1 commit into
git-for-windows:mainfrom
rimrul:modern-inno-features

Conversation

@rimrul

@rimrul rimrul commented Feb 15, 2025

Copy link
Copy Markdown
Member

Innosetup 6.4 introduced a New function ExecAndCaptureOutput that executes commands and captures there standard output and standard error streams. We've been using our own howmgrown solution called ExecWithCapture for this purpose, but it's a bit finicky. Switch to the official implementation.

@rimrul rimrul left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is entirely untested, I just made sure it builds.

Comment thread installer/install.iss
Comment thread installer/install.iss Outdated
Comment thread installer/install.iss Outdated
end;

// Split NUL-delimited key/value pairs, extract LF that denotes end of key
StdOut:=StringJoin(#10,Output.StdOut);

@rimrul rimrul Feb 15, 2025 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We probably want to iterate over Output.StdOut directly instead of joining and re-splitting this, but that was the easiest Q&D way to at least get it to compile again.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We do that now, but there's probably at least one off-by-one error in there.

@rimrul
rimrul force-pushed the modern-inno-features branch 2 times, most recently from 0c4df92 to 8d18d60 Compare February 22, 2025 10:44
@rimrul rimrul changed the title [WIP] installer: use ExecAndCaptureOutput [WIP] installer: use modern innosetup features Feb 22, 2025
Comment thread installer/install.iss Outdated
SourceDir={#SOURCE_DIR}
#if BITNESS=='64' || INSTALLER_FILENAME_SUFFIX=='arm64'
ArchitecturesInstallIn64BitMode=x64 arm64
ArchitecturesInstallIn64BitMode=x64compatible arm64

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With the new x64compatible, there should be no need to explicitly set arm64 anymore:

x64compatible
Matches systems capable of running x64 binaries. This includes systems running x64 Windows, and also Arm64-based Windows 11 systems, which have the ability to run x64 binaries via emulation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense to me!

@dscho dscho linked an issue Oct 22, 2025 that may be closed by this pull request
@dennisameling

Copy link
Copy Markdown
Member

@rimrul with all the recent updates to the installer, there's some conflicts on this PR now. Do you have bandwidth to rebase it onto the latest main branch? Happy to help if not.

@rimrul

rimrul commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Yeah, I should get around to it in the next few days.

@rimrul
rimrul force-pushed the modern-inno-features branch from 80488ef to c10f085 Compare September 8, 2026 08:40
@rimrul rimrul changed the title [WIP] installer: use modern innosetup features installer: use modern innosetup features Sep 8, 2026
@rimrul
rimrul marked this pull request as ready for review September 8, 2026 08:50
Comment thread installer/install.iss
if (BuiltinFSMonitorStopOption='') then begin
BuiltinFSMonitorStopOption:='(huh?)';
if not ExecWithCapture('"'+AppDir+'\cmd\git.exe" fsmonitor--daemon -h',Str,Str,ExitCode) or (ExitCode<>129) then begin
if not ExecAndCaptureOutput('"'+AppDir+'\cmd\git.exe"', 'fsmonitor--daemon -h', '', SW_SHOWNORMAL, ewWaitUntilTerminated, ExitCode, Output) or (ExitCode<>129) then begin

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see that the deleted code assigned StartupInfo.wShowWindow:=SW_HIDE;, so I was a bit concerned about the SW_SHOWNORMAL here. But the documentation of ExecAndCaptureOutput() says:

Console programs are always hidden and the ShowCmd parameter only affects GUI programs, so always using SW_SHOWNORMAL instead of SW_HIDE is recommended.

So all is good. If at all, I would document that in the commit message. But I could live with it in the present shape, too.

Innosetup 6.4 introduced a new function ExecAndCaptureOutput that
executes commands and captures there standard output and standard error
streams. We've been using our own howmgrown solution called
ExecWithCapture for this purpose, but it's a bit finicky. Switch to the
official implementation.

Note: While ExecAndCaptureOutput always executes console programs
hidden, it does still expect a value for the ShowCmd parameter and the
documentation recommends using SW_SHOWNORMAL. [1]

[1] https://jrsoftware.org/ishelp/index.php?topic=isxfunc_execandcaptureoutput

Signed-off-by: Matthias Aßhauer <mha1993@live.de>
@rimrul
rimrul force-pushed the modern-inno-features branch from c10f085 to bc59ce3 Compare September 10, 2026 11:00

@dscho dscho left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@dscho
dscho merged commit 86d5eaa into git-for-windows:main Sep 11, 2026
7 checks passed
@rimrul
rimrul deleted the modern-inno-features branch September 11, 2026 12:14
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.

installer: use new InnoSetup functionality

3 participants