Skip to content

Add support to create MSI installer for Windows build #1400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 2, 2018

Conversation

karim
Copy link
Member

@karim karim commented May 31, 2018

Initial effort to migrate Windows build system from NSIS to MSI (Windows Installer).

Initial effort to migrate Windows build system from NSIS to MSI (Windows Installer).
@karim karim self-assigned this May 31, 2018
@karim karim requested a review from justinclift May 31, 2018 12:05
@karim
Copy link
Member Author

karim commented May 31, 2018

Files

build.cmd will create both 32-bit (build.cmd Win32) and 64-bit (build.cmd Win64) installers. Just add it to your windows build batch file after compiling the application.

product.wxs contains all the logic to create the installer that build.cmd will use.

variables.wxi contains all the changeable variables, the libraries locations, etc. It is included by product.wxs.

license.rtf is the same as LICENSE but in a different format, since Windows Installer only accept .rtf files for the license dialog.

@karim karim mentioned this pull request May 31, 2018
Copy link
Member

@justinclift justinclift left a comment

Choose a reason for hiding this comment

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

Thanks Karim. I'll try this out in a new Windows VM in a few hours (need to install stuff first), to gain some familiarity with the needed bits. 😄

<?define AppContact="justin@postgresql.org" ?>
<?define AppReadme="https://github.com/sqlitebrowser/sqlitebrowser" ?>
<?define AppHelpLink="https://github.com/sqlitebrowser/sqlitebrowser/wiki" ?>
<?define AppSupportLink="https://github.com/sqlitebrowser/sqlitebrowser/issues/new/choose" ?>
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to have this just go to the main issues page, instead of the .../new/choose one. Having people go to the existing issues section would more easily let them search the existing issues first. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

True. I will edit it. 😃

@justinclift
Copy link
Member

With the license.rtf file, should the -plugins one also be converted to an .rtf?

@karim
Copy link
Member Author

karim commented May 31, 2018

This is for the setup dialog window only. It accepts .rtf (Microsoft Word) files only so I had to convert it. Both files (LICENSE and LICENSE-PLUGINS) are copied to licenses folder like the old installer anyway.

msi

@justinclift
Copy link
Member

justinclift commented May 31, 2018

Setting up a new Win 7 dev VM for experimenting in atm. Probably get the setup of the VM finished tonight, and be in place to try this stuff out sometime tomorrow. 😄

@karim
Copy link
Member Author

karim commented May 31, 2018

Hmm.. why a new VM? If you already got WiX Toolset installed (in the current build system), just run build.cmd Win32/Win64 somewhere in winbuild.bat to create the new installers. Also keep generating the NSIS ones till we finish testing so we can have something as a fallback.

@justinclift
Copy link
Member

justinclift commented May 31, 2018

Hmm.. why a new VM?

Two reasons. 😄

  1. I generally treat that build VM as a production system.
  • So, I don't screw with it very much, Just In Case.
  • When trying out new/unknown software (eg experimenting) I tend to do stuff in a reasonably isolated environment that I can snapshot, wind back to previous states quickly, redo things over, etc.
  1. The build VM is on a very old, very slow Core 2 duo system. Even though it's running on an SSD it's pathetically slow to use interactively. Not great for experimenting on either. 😉

Although it takes a bit of effort to set up (almost done now), I prefer to create a VM (KVM this time) on my local desktop, backed by some decent speed (NVMe) storage from my local NAS.

Much faster to then muck around with (eg snapshots, experimenting, whatever) than running on that crappy old Core 2 duo box. 😄

@karim
Copy link
Member Author

karim commented May 31, 2018

Fair enough. 😃

@justinclift
Copy link
Member

Trying this out in the new VM now. It's spitting out this warning, though an .msi is generated:

C:\git_repos\sqlitebrowser\installer\windows\product.wxs(44) : warning LGHT1104: Merge module 'VCRedist' has an installer version of 405 which is greater than the product's installer version of 301. Merging a module with a higher installer version than the product it is being merged into can result in invalid values i n the resulting msi. You must set the Package/@InstallerVersion attribute to 405 or greater to merge this merge module into your product.

Trying out the .msi in the same VM it was built in, it seems functional. DB4S installs ok, loads ok, and uninstalls ok. 😄

Initial searching online for that error code turns up this:

This seems useful/relevant too, though I don't quite grok it yet:

@justinclift
Copy link
Member

Uploaded that initial .msi here, in case other people want to try it out:

@karim
Copy link
Member Author

karim commented Jun 1, 2018

That's funny cause I used the merge module on my system that comes with Visual Studio 2015 and it only required version 3.1. 😃

Initially, I wasn't using Package/InstallerVersion attribute. It defaults to Windows Installer v2.0 (200), and after adding VCRedist it wanted v3.1 (301) so I changed.

According to https://en.wikipedia.org/wiki/Windows_Installer#Versions version 4.5 should be included with Windows Vista and up, and can be downloaded for earlier Windows versions (via windows update).

The first link you posted explains it well. They just wanted to drop support for Windows XP by increasing the version number. But it looks like they changed it back with Visual Studio 2015.

The second link is the one I used to add the VCRedist merge module.

It is your call. We can increase the version to 4.5 to silence the warning, use light.exe switch to silence it, or simply ignore it. 😸

@justinclift
Copy link
Member

justinclift commented Jun 1, 2018

Testing that .msi on a fresh Win7 install (without updates applied), it works. DB4S also launches and seems functional.

Weirdly, DB4S pops up an warning dialog with: "Error The network is not accessible." when it starts though. It's probably something to do with our automatic version check, and doesn't look to negatively effect things. The warning seems strange though, as the network is definitely available.

Turned off Windows Firewall... same message. Weird.

@karim
Copy link
Member Author

karim commented Jun 1, 2018

Trying out the .msi in the same VM it was built in, it seems functional. DB4S installs ok, loads ok, and uninstalls ok.

You probably have Visual C++ Redistributable installed with Visual Studio. Can you try it on a VM with no Visual C++ Redistributable installed at all?

@justinclift
Copy link
Member

Ahhh, as long as that .msi warning thing about the version doesn't mean our users will be negatively impacted, we can probably ignore it.

We're still generating WinXP compaible builds too aren't we?

@justinclift
Copy link
Member

Can you try it on a VM with no Visual C++ Redistributable installed at all?

Yep, did that right after (next comment). 😄

@justinclift
Copy link
Member

Oh, we should probably get the DB4S logo into the installer too. 😄

@justinclift
Copy link
Member

Yep, if I disable "Automatic updates" in our preferences, that network warning thing doesn't happen on startup. We can probably ignore it, at least for now. 😄

@karim
Copy link
Member Author

karim commented Jun 1, 2018

Yep, did that right after (next comment).

Yeah. 😄

We're still generating WinXP compaible builds too aren't we?

That depends on Qt. If we are still building with Qt 5.7 and not using any new APIs, the same version should work.

Oh, we should probably get the DB4S logo into the installer too.

Where? can you take a screenshot?

Yep, if I disable "Automatic updates" in our preferences, that network warning thing doesn't happen on startup. We can probably ignore it, at least for now.

Later on we can add a checkbox in a custom dialog during installation to disable automatic checking for update, cause some reported that it was crashing for them, so they can be able to at least start the app.

But we really should find out the reason for this error.

@justinclift
Copy link
Member

Asking the user if they want automatic update checking enabled is a good idea.

I'm running the standard Windows Update thing on the Win7 VM, to see if that fixes the network warning. I'd rolled back the VM to just after the OS was installed, so I think it's effectively Win7 SP1 and nothing else. If the warning goes away, then it's probably something we don't have to change anything for. Instead, if anyone reports the same problem we can let them know their system probably needs updating. 😄

I'll get the logo screenshot done after that.

@karim
Copy link
Member Author

karim commented Jun 1, 2018

One question regarding SQLCipher. Can we have both (SQLite & SQLCipher) linked together in the app, and at runtime decide which one to load? Or we must build each one separately?

@justinclift
Copy link
Member

I think it's a one-or-the-other thing, so we need to keep on making both.

@justinclift
Copy link
Member

Ok, even with Windows updates applied that network error message is still happening. Weird.

@justinclift
Copy link
Member

justinclift commented Jun 2, 2018

Actually... it probably wouldn't hurt to put the .xcf files in the repo too, so other people can have a go if they feel like it as well. 😄

@karim
Copy link
Member Author

karim commented Jun 2, 2018

Done. 😄

What's next now?

@justinclift
Copy link
Member

Cool. 😄

Is there an .xcf for the banner.bmp too?

@karim
Copy link
Member Author

karim commented Jun 2, 2018

Unfortunately no. This one I done with Windows Paint. But I can make one for it. Give me some minutes.

@justinclift
Copy link
Member

Ooops, forgot you said that. Um, if you're ok to add in an equivalent banner.xcf that'd be optimal. 😄

From my point of view, this PR looks ready to merge (when the banner.xf is added). 😄

I'll add our nightly build batch file (as per our gist) just this is merged, so we have all of the windows building stuff in one place.

Note - The windows batch file we run nightly might be slightly different from the gist... I have a feeling I changed some of the options for an issue a while ago, and never got around to updating the gist. Will need to check first. 😉

@karim
Copy link
Member Author

karim commented Jun 2, 2018

I'll add our nightly build batch file

Yes, I meant to talk to you about this but I always forget. 😃 Maybe you should make build-scripts repo and put it there, or tools repo, or just add it to this one so it is easier to edit.

Don't merge just yet. There are a few things we need to discuss. The things I remember now..

  • Add SQLCipher
  • Detect the old NSIS and uninstall first
  • Decide if we should drop dual install of 32 and 64 bits (once we ship a product with GUID it is hard to change later, since the same one is needed to support install/uninstall/upgrade)

@justinclift
Copy link
Member

Good points. I'm reading back through the comment in #1401 to get a better understanding of the bits.

  • SQLCipher: Hoping this isn't too hard. It's just a differently named output file it needs to generate.

  • Detect old NSIS + uninstall first. Not a 100% definite requirement, as we could just put a lot of "NOTE - Uninstall old version first!" type of notices on all the things. 😉 But if it's not hard to do accurately, then sure lets do it. 😄

  • Parallel 32 and 64-bit installation: Probably better to use same GUID for both the 32 and 64 bit installations, as that way our end users are unlikely to get into a situation where they have multiple installs on the system.

    • Power users may need catering to differently, but we can cross that bridge later. 😄

@justinclift
Copy link
Member

Maybe you should make build-scripts repo and put it there, or tools repo, or just add it to this one so it is easier to edit.

I'll just dump it into the same installer/windows directory that build.cmd is going into. 😄

@karim
Copy link
Member Author

karim commented Jun 2, 2018

SQLCipher: Hoping this isn't too hard. It's just a differently named output file it needs to generate.

True. We just need to bundle a different DLL when SQLCipher flag is set.

Detect old NSIS + uninstall first. Not a 100% definite requirement

Also true. Hopefully no one complains about MSI not working cause they didn't uninstall first.

Parallel 32 and 64-bit installation: Probably better to use same GUID for both the 32 and 64 bit installations

Yes, I prefer that too. Now we get only 1 product (like before). We will have to delete one of the GUID (UpgradeCode) in variables.wxi file. Which one we should keep? Or we generate a new one? I use https://www.guidgen.com/ whenever I need a GUID.

@justinclift
Copy link
Member

We will have to delete one of the GUID (UpgradeCode) in variables.wxi file. Which one we should keep?

I doubt it matters, as only ourselves and a few people testing the 64-bit one have probably used either. Um, use the 64-bit one's maybe, just to make it easy on ourselves? 😁

@karim
Copy link
Member Author

karim commented Jun 2, 2018

I will get that done soon. I have to finish few things first.

@justinclift
Copy link
Member

Just added the windows nightly build script, as is currently on the build VM. The only difference from the gist is a change in username and upload path due to the recent server migration. 😄

@justinclift
Copy link
Member

justinclift commented Jun 2, 2018

Hmmm. My adding the winbuild.bat directly into this PR probably wasn't the best move. That'll make squashing the commits pretty tricky.

I've just wound back the commit I added winbuild.bat in. I'll add it after this PR is merged, like I originally said. 😄

@karim
Copy link
Member Author

karim commented Jun 2, 2018

You can merge it now and I will get SQLCipher at a later commit.

@justinclift justinclift merged commit c8eddc4 into sqlitebrowser:master Jun 2, 2018
@justinclift
Copy link
Member

Awesome @karim, done. 😄

@ferenczy
Copy link

ferenczy commented Aug 6, 2019

Just curious, what was the reason for migrating from the NSIS installer to MSI, please?

@chrisjlocke
Copy link
Member

"Migrating to Windows Installer will give us a greater control over the installation and will give the user the best possible experience. "

Think it was also slightly easier from a development perspective.

@karim
Copy link
Member Author

karim commented Aug 6, 2019

Adding to what Chris said above, we also had an issue (#1399) with NSIS installer.

We need to bundle Visual C++ Redist with the installer. With NSIS, the VCRedist.exe was included along with it (an installer inside an installer) and we run it at the end.

VCRedist.exe installer used to hang on some computers (with broken Windows Update service). Eventually, it fails to install, even though the application is installed correctly.

Also, it doesn't uninstall the application cleanly as you have now two entries in Add/Remove Programs; one for DB4S and another for Visual C++ Redist. When you uninstall, only DB4S was removed.

Does that answer your question on why we moved away from NSIS to MSI? Or maybe you are asking a general question about the advantage of MSI over NSIS?

@ferenczy
Copy link

ferenczy commented Sep 5, 2019

No, you have provided enough information, thank you. I was just curious about the reasons since I usually saw people migrating from MSI as opposed to to MSI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants