FEAT(installer): bundle c++ redistributables

[sqwishy]

After building each server and client msi, the installer script creates an exe bundle that includes a step to download and install the visual c++ redistributables.

re #5549

This commit needs the redistrituables installer downloaded to installer/VC_redist.x64.exe. It includes a change in cmake to copy the file from there to the build directory. The wixsharp installer building stuff needs VC_redist.x64.exe when building the bundle, even though the file isn't embedded in the resulting exe.

I presume you would rather configure cmake download VC_redist.x64.exe or something instead of adding it to version control, so I didn't commit it.

I left some comments in the C# code to try to explain what is going on with the installer and how the Wix stuff works. Let me know if you have any other questions.

The way the bundle works in this commit is that it hides the UI from the .msi installer and just displays is own UI in its place. So the license and logo are copied in to the bundle UI and it ends up resembling the .msi a bit. Here's a screenshot of what to expect.

Checks

• My commits follow the commit guidelines

[sqwishy]

Presently, the version required check is at least 14.0.0.0. The URL in the code installs version 14.42.34438.0 (there's a comment in the code about that). But you might want to adjust the version requirement as you see fit.

[Hartmnt]

Nice, thank you very much.

I still have a conceptual question: Why exactly does the VC installer have to be present at compile time and being downloaded when running the Mumble installer? Wouldn't the version used at build time be overwritten by the downloaded one anyway?

I presume you would rather configure cmake download VC_redist.x64.exe or something instead of adding it to version control, so I didn't commit it.

If we really need to have this at build time, we can make use/adapt this CMake function to download the VC_redist.x64.exe:

mumble/3rdparty/rnnoise-build/CMakeLists.txt

Lines 49 to 56 in f304e0a

	FetchContent_Declare(
	rnnoise_model
	URL "${rnnoise_model_url}"
	URL_HASH SHA256=${rnnoise_model_hash}
	DOWNLOAD_EXTRACT_TIMESTAMP 1
	)
	FetchContent_MakeAvailable(rnnoise_model)

Of course then it would make sense to declare the download url in CMake and write this variable to the .cs so we don't have to manage the same url twice.

[sqwishy]

Why exactly does the VC installer have to be present at compile time and being downloaded when running the Mumble installer? Wouldn't the version used at build time be overwritten by the downloaded one anyway?

That's a good question. The ExePackage has a child element called RemotePayload that has fields about the download file including a sha1 hash that it uses to validate the download when a user runs the installer. (The validation can be toggled off with SuppressSignatureVerification but it seems like diligent to leave it on.)

In the commit you commented on, I didn't specify the RemotePayload explicitly. As far as I can tell, Wix needs the source file to generate that RemotePayload with the information it needs. So it doesn't copy the VC_redist.x64.exe file into the installer, but it needs it there just to read information like the description, size, hash, and version. And because the redistributables aren't copied into the installer at build time, they must still be downloaded by the user at install time.

However, it looks like the RemotePayload could be declared in MumbleInstall.cs and then Wix wouldn't need to generate it and it would be unnecessary to have the VC_redist.x64.exe around when building the installer.

That means if, in the future, you change the DownloadUrl to get a different version of the redistributables, you will need to update the corresponding info in the RemotePayload in that file as well. Which would probably just be the sha1 hash, flie size, and version. But I suspect you won't need to change the redistributable you depend on very often, so including the RemotePayload explicitly might be preferable?

[Hartmnt]

In the commit you commented on, I didn't specify the RemotePayload explicitly. As far as I can tell, Wix needs the source file to generate that RemotePayload with the information it needs. So it doesn't copy the VC_redist.x64.exe file into the installer, but it needs it there just to read information like the description, size, hash, and version. And because the redistributables aren't copied into the installer at build time, they must still be downloaded by the user at install time.

However, it looks like the RemotePayload could be declared in MumbleInstall.cs and then Wix wouldn't need to generate it and it would be unnecessary to have the VC_redist.x64.exe around when building the installer.

That means if, in the future, you change the DownloadUrl to get a different version of the redistributables, you will need to update the corresponding info in the RemotePayload in that file as well. Which would probably just be the sha1 hash, flie size, and version. But I suspect you won't need to change the redistributable you depend on very often, so including the RemotePayload explicitly might be preferable?

I have read this a couple of times, not sure if I got this right. But it sounds to me like it would be possible to download VC_redist.x64.exe at build time using CMake. Compute the hash and other needed information using CMake. And finally inserting the results of those computations into MumbleInstaller.cs as a RemotePayload object (using pre-processor macros)?

[sqwishy]

I have read this a couple of times, not sure if I got this right. But it sounds to me like it would be possible to download VC_redist.x64.exe at build time using CMake. Compute the hash and other needed information using CMake. And finally inserting the results of those computations into MumbleInstaller.cs as a RemotePayload object (using pre-processor macros)?

I probably didn't explain it well. Right now in this branch, if you manually place VC_redist.x64.exe in the source tree or download it with cmake, wix will get the metadata on its own when it builds the installer.

The alternative is providing the metadata in the MumbleInstall.cs file as a RemotePayload object underneath the ExePackage for VC_redist.x64.exe. The correct metadata corresponds to the file at the DownloadUrl and will change as that URL does.

The first option has the advantage where, if you change the version of the distributables you depend on, you just change the DownloadUrl property rebuild the installers with a file downloaded from that URL -- wix will read metadata needed from that file. In the second option, you'd get the hash and filesize manually and update the source code with those values at the same time that you update the DownloadUrl.

The advantage of the second option is not having to worry about having the redistributables installer around at compile time, so it makes things a bit simpler. It kind of comes down to your preference. The second option is maybe a bit less convenient if you change what version of redistributables you depend on -- but my guess is that is very rare.

If you want, I can write a new commit in this branch to show you what I mean exactly. (That way I can actually test that the second option actually works like I think it will.) And you can squash/edit the changesets as you wish before merging.

[Hartmnt]

If you want, I can write a new commit in this branch to show you what I mean exactly. (That way I can actually test that the second option actually works like I think it will.) And you can squash/edit the changesets as you wish before merging.

Ok, I get it now, but I also think we are having two different conversations :)

Right now in this branch, if you [...] download it with cmake

My point is, it does not currently do that automatically. That is why the CI pipeline is failing.

We/You/I can change the CMakeFiles.txt in this branch to actually do the download. This would mean we would not specify the URL of the redistributable as string in DownloadUrl, but instead in a CMake variable, which we can change as need be. Then we use the CMake Variable containing the URL as pre-processor macro (wait can we use pre-processor macros in .cs files? help)

As a developer, having to download the redistributable and manually placing it in the directory when building is not an option. We need to be able to automatically build Mumble and the installer

[sqwishy]

As a developer, having to download the redistributable and manually placing it in the directory when building is not an option. We need to be able to automatically build Mumble and the installer

Yeah I agree. I might have misunderstood your question. I wasn't sure what your preference was between either having cmake download the redistributables or an option where the download wasn't required and instead the metadata was included in the source code.

If you prefer the first option, I can have a go at making the required changes to cmake. I'm not real sure if cscs.exe (the program that starts the toolchain for building the mumble installer) has macros like the -D option with C or C++ but it at least seems to be able to include files so cmake could write the version to a file in the build directory. I can probably figure something out.

[Hartmnt]

If you prefer the first option, I can have a go at making the required changes to cmake. I'm not real sure if cscs.exe (the program that starts the toolchain for building the mumble installer) has macros like the -D option with C or C++ but it at least seems to be able to include files so cmake could write the version to a file in the build directory. I can probably figure something out.

Yeah, that would be awesome. Downloading stuff at build time using CMake instead of doing it manually is fine. This will make the CI happy then I suppose :)

[sqwishy]

I've updated this branch so that cmake downloads the redistributable and passes the download URL as a command line argument to cscs when building the installer so that the URL is only defined in one place. Let me know if you have any other questions or would like other adjustments.

[Hartmnt]

@sqwishy Looks reasonable to me like this. However, the Windows ecosystem is a mystery to me.

@davidebeatrici Could you please test the installer on a Windows test machine?

@Krzmbrzl Any thoughts? I would like to merge this before the next 1.5 release