Skip to content

Add WiX installer support#1522

Closed
InBetweenNames wants to merge 2 commits into
arrayfire:masterfrom
InBetweenNames:devel
Closed

Add WiX installer support#1522
InBetweenNames wants to merge 2 commits into
arrayfire:masterfrom
InBetweenNames:devel

Conversation

@InBetweenNames
Copy link
Copy Markdown

On Windows, the WiX installer is nice since it lets you easily add and modify environment variables, enabling you to create convenient packages for deployment. This patch adds support for CMake's WiX generator in order to do that.

Older CUDA versions may be used, so this will add the default CUDA
install's nvvm binary path for easier deployment.
@pavanky
Copy link
Copy Markdown
Member

pavanky commented Aug 1, 2016

@shehzan10 can you look into this

@shehzan10
Copy link
Copy Markdown
Member

I am not well aware of the WiX system and will read up a bit about it.

The disadvantage is that creating a secondary installer will be different from the installer we produce using NSIS. Hence it will have a different set of examples etc which will not be documented.

We will also need to have the WiX option similar to RPM where is it only enabled by a build option. Otherwise make package will always produce it.

Is this something we want done for 3.4.0? Based on that I'll change my priority to test the PR.

Comment thread wix/WIXPatch.wxs
<Environment Id="ENVPATH" Action="set"
Name="PATH" Permanent="no" System="yes" Part="last" Value="[INSTALL_ROOT]lib"/>
<Environment Id="NVPATH" Action="set"
Name="PATH" Permanent="no" System="yes" Part="last" Value="[%CUDA_PATH]\nvvm\bin"/>
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.

This (adding NVVM to path) is something we have discussed at length internally about and have consistently decided not to have in our installer. The single most important reason being that NVIDIA does not add it to Path themselves.

Other reasons include

  • Making the PATH too long.
  • Most people will not remove such a path manually if they do not know what added it.

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.

May be an alternative would be to have the installer copy nvvm.dll (if it exists) into arrayfire/lib ?

@InBetweenNames
Copy link
Copy Markdown
Author

Yeah WiX needs an RTF license file unfortunately, I don't believe the markdown will work, but I could test it. I did try using the NSIS installer, but I found the one generated from cpack didn't create the AF_PATH variable which the official installers do. This WiX installer does do that, fortunately. I could amend this PR so that it doesn't add nvvm to the PATH, however, or at least have an option to add nvvm to the PATH. On my deployed systems, it's nice to have the nvvm automatically added to the PATH, and it's nice that the WiX uninstaller automatically removes it from the PATH as well.

@shehzan10
Copy link
Copy Markdown
Member

We do not use CPack to generate the Windows installer. We have a different set up for that. We generate Visual Studio files, sign the DLLs, add MKL/other dependencies etc which is better for the windows environment.
Essentially, we have designed the windows installer to work literally out of the box.

I am certainly not opposed to the WiX installer. I think we can make it a lot better if we don't target 3.4.0. That will give us more time to test it out.

@InBetweenNames
Copy link
Copy Markdown
Author

That explains it then :) should I amend this PR to make those changes?

@shehzan10
Copy link
Copy Markdown
Member

Let me read/test it out a little and then we can talk in detail about what direction to take with this.

@shehzan10 shehzan10 added this to the timeless milestone Aug 1, 2016
@shehzan10 shehzan10 self-assigned this Aug 1, 2016

SET(CPACK_WIX_LICENSE_RTF "${PROJECT_SOURCE_DIR}/LICENSE.rtf")
SET(CPACK_WIX_PATCH_FILE "${PROJECT_SOURCE_DIR}/wix/WIXPatch.wxs")
SET(CPACK_WIX_UPGRADE_GUID "FF9E2D77-CDC7-4D24-8B7B-99D66EDEE862")
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.

What does this number signify ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's a unique identifier for the installer, used for when users upgrade to a newer version. The existing installation can be detected by this. This way multiple entries in Programs and Features aren't created, and the existing installation can be upgraded in place.

For more information, see: https://cmake.org/cmake/help/v3.6/module/CPackWIX.html

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.

@InBetweenNames I was wondering if this is randomly generated. If so does it need to change with every version ?

Can you also add some comments on when this string needs to be preserved and when it needs to be regenerated ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is indeed randomly generated, and if you wanted to have multiple ArrayFire installations side by side, then this number could change. Otherwise, leaving this number intact allows for future versions of ArrayFire to replace existing versions on the system.

@shehzan10
Copy link
Copy Markdown
Member

@InBetweenNames If you join our gitter we can have a more flowing conversation there. You can send me a private message over there.

@pavanky pavanky modified the milestone: timeless Jul 26, 2017
@pavanky pavanky changed the base branch from devel to master August 23, 2017 15:23
@9prady9
Copy link
Copy Markdown
Member

9prady9 commented Jan 23, 2018

@InBetweenNames Once #2048 has been merged into master, can you please rebase your changes of master.

@9prady9
Copy link
Copy Markdown
Member

9prady9 commented Feb 25, 2018

@InBetweenNames We are handling NSIS64 and IFW generators via #2048 which will be used to produce our installer binaries going forward. We will add WIX support in the furture should it provide an definitive advantage over the current ones. However, you are always welcome to send your changes as well (please base your changes of our latest master once #2048 is merged). Thank you.

@9prady9 9prady9 closed this Feb 25, 2018
@InBetweenNames
Copy link
Copy Markdown
Author

InBetweenNames commented Feb 26, 2018 via email

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants