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

Add support to create MSI installer for Windows build #1400

Merged
merged 9 commits into from Jun 2, 2018

Conversation

Projects
None yet
2 participants
@karim
Copy link
Member

karim commented May 31, 2018

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

Add support to create MSI installer for Windows build
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

@karim

This comment has been minimized.

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 referenced this pull request May 31, 2018

Open

New Windows Installer #1401

@justinclift
Copy link
Member

justinclift left a comment

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" ?>

This comment has been minimized.

@justinclift

justinclift May 31, 2018

Member

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. 😄

This comment has been minimized.

@karim

karim May 31, 2018

Author Member

True. I will edit it. 😃

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented May 31, 2018

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

@karim

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member Author

karim commented May 31, 2018

Fair enough. 😃

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

@karim

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

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

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 1, 2018

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

@karim

This comment has been minimized.

Copy link
Member Author

karim commented Jun 2, 2018

Maybe white is better?

db4s2

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

Yeah. My preference is the white background. 😄

With the large size logo, can it be moved to the top half?

@karim

This comment has been minimized.

Copy link
Member Author

karim commented Jun 2, 2018

db4s3

If you are familiar with GIMP I can send you the .xcf file so you can also try. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

Nah, that last one (logo at the top) is good enough. 😄

@justinclift

This comment has been minimized.

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 added some commits Jun 2, 2018

@karim

This comment has been minimized.

Copy link
Member Author

karim commented Jun 2, 2018

Done. 😄

What's next now?

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

Cool. 😄

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

@karim

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

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

This comment has been minimized.

Copy link
Member Author

karim commented Jun 2, 2018

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

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

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

This comment has been minimized.

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. 😄

@justinclift justinclift force-pushed the karim:windows-installer branch from 8d08698 to 53c261f Jun 2, 2018

@karim

This comment has been minimized.

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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Jun 2, 2018

Awesome @karim, done. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment