Installer should use public start menu (and desktop) for links

[nico-dr]

if you are logged in as non administrator user (hopefull the most are) and you install Jamulus on a Win 10 PC, you will be asked for an administrator account and the installer will run under this (different) user with the result that the current logged in user will not find the usefull links to start Jamulus.

[ann0see]

Same issue here. (Thought of this a while ago). I was also a bit confused that it didn't show up in my start menu. Should be an easy fix.

https://github.com/corrados/jamulus/blob/26e7c2c24fa64297da5ee015838ab30a2b4061f1/windows/installer.nsi#L116

And

https://github.com/corrados/jamulus/blob/26e7c2c24fa64297da5ee015838ab30a2b4061f1/windows/installer.nsi#L180

Should be changed probably?

See: http://sredna.users.sourceforge.net/public/NSIS/2.46/Manual.html#setshellvarcontext

[pljones]

The installer needs a good rewrite.

It should allow the individual user to decide how to install it.

I want to have an "all users" install. Some people might not want that, though.

I don't want desktop shortcuts. Some people might want them, though.

I only want the 64bit version installed. And I want the default install location to be correct (not the 32bit location).

But some people may still need the 32bit version, as well, even when running on 64bit.

I want to override what the install locations are.

Most of all, I want the installer to remember my choices.

[ann0see]

The installer needs a good rewrite.

Probably yes.

Nevertheless my draft pr could be a fast fix for this issue. Can someone try my edits?

I got really annoyed by this bug and if you don't find Jamulus on your desktop it's a very bad UX. Experienced users can always delete desktop icons, inexperienced users would be annoyed by the current behavior ("No, I can't see Jamulus!!! It's not installed!!!" "Yes it is! Go to Program Files>..." "How do I do that?"...)

[corrados]

The installer needs a good rewrite.

Can someone try out the current implementation on https://github.com/corrados/jamulus/tree/improve_win_ci? I am pretty sure that this solves the issues. Unfortunately, I did not succeed in running it.

[ann0see]

I'm now trying it.

You need to set

Set-ExecutionPolicy -ExecutionPolicy RemoteSigned -Scope CurrentUser

and there seem to be multiple other issues

Edit:

Could get it to compile now. There are some path issues in the power shell script. Also the link to sourceforge is broken (it probably downloads HTML)

My diff:

--- a/windows/deploy_windows.ps1
+++ b/windows/deploy_windows.ps1
@@ -1,10 +1,10 @@
 param(
     # Replace default path with system Qt installation folder if necessary
-    [string] $QtInstallPath = "C:\Qt\5.12.3"
+    [string] $QtInstallPath = "C:\Qt\5.9.9"
 )

 # Global constants
-$RootPath = "$PWD"
+$RootPath = "$PWD\..\"
 $BuildPath = "$RootPath\build"
 $DeployPath = "$RootPath\deploy"
 $WindowsPath ="$RootPath\windows"
@@ -53,10 +53,15 @@ Function Install-Dependency
     if (Test-Path -Path "$WindowsPath\$Destination") { return }

     $TempFileName = [System.IO.Path]::GetTempFileName() + ".zip"
+    $TempDir = [System.IO.Path]::GetTempPath()
+
     Invoke-WebRequest -Uri $Uri -OutFile $TempFileName
-    Expand-Archive -Path $TempFileName -DestinationPath $Env:TEMP -Force
-    Move-Item -Path "$Env:TEMP\$Name" -Destination "$WindowsPath\$Destination" -Force
+    echo $TempFileName
+    Expand-Archive -Path $TempFileName -DestinationPath $TempDir -Force
+    echo $WindowsPath\$Destination
+    Move-Item -Path "$TempDir\$Name" -Destination "$WindowsPath\$Destination" -Force
     Remove-Item -Path $TempFileName -Force
+
 }

 # Install VSSetup (Visual Studio detection), ASIO SDK and NSIS Installer
@@ -66,8 +71,8 @@ Function Install-Dependencies
     Install-Module -Name "VSSetup" -Scope CurrentUser -Force
     Install-Dependency -Uri "http://www.steinberg.net/sdk_downloads/ASIOSDK2.3.2.zip" `
         -Name "ASIOSDK2.3.2" -Destination "ASIOSDK2"
-    Install-Dependency -Uri "https://netix.dl.sourceforge.net/project/nsis/NSIS%203/3.05/nsis-3.05.zip" `
-        -Name "nsis-3.05" -Destination "NSIS"
+    Install-Dependency -Uri "https://jztkft.dl.sourceforge.net/project/nsis/NSIS%203/3.06.1/nsis-3.06.1.zip" `
+        -Name "nsis-3.06.1" -Destination "NSIS"
 }

 # Setup environment variables and build tool paths

There's something wrong with the temp variable

[ann0see]

Now it does start compiling but throws an error after a few minutes. Not sure, but there might be something wrong in the Jamulus Source code.

We could port at least a few changes from this branch. Not 100% sure how to fix the power shell script yet, but it should be possible.

Moved the code to an external drive and started compiling there. Now it compiled the 64 bit version...

[ann0see]

Got it to work (run on external harddisk and admin and my patch).

Unfortunately it installs jamulus into C:\Program Files\Jamulus\F\jamulus\deploy\x86_64...

[ann0see]

#794 could potentially fix this. @corrados could you please test it? You might have to change the path to VS 2015

Here:

https://github.com/ann0see/jamulus/blob/1ffb86c4a2c5d168581d1a79b8f709946ad618f4/windows/deploy_windows.ps1#L97

And here:

https://github.com/ann0see/jamulus/blob/1ffb86c4a2c5d168581d1a79b8f709946ad618f4/windows/deploy_windows.ps1#L102

[corrados]

You seem you have changed the file in the meantime. I guess I have to change this:

    [string] $QtCompile32 = "msvc2015",
    [string] $QtCompile64 = "msvc2015_64",

I am a newbe regarding Windows powershell. When I try to call the script, I get:

.\deploy_windows.ps1 : Die Datei "C:\[...]\deploy_windows.ps1" kann nicht
geladen werden, da die Ausführung von Skripts auf diesem System deaktiviert ist. Weitere Informationen finden Sie
unter "about_Execution_Policies" (https:/go.microsoft.com/fwlink/?LinkID=135170).
In Zeile:1 Zeichen:1
+ .\deploy_windows.ps1
+ ~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : Sicherheitsfehler: (:) [], PSSecurityException
    + FullyQualifiedErrorId : UnauthorizedAccess

Do I have to run it as administrator?

[ann0see]

@corrados yes, you should run it as Administrator from a location near to the “root“ like C:\Jamulus

Before you can execute it, you also need to execute

Set-ExecutionPolicy -ExecutionPolicy RemoteSigned -Scope CurrentUser

[corrados]

I have moved the jamulus directory to C:\ and run the power shell as admin. I still get an error:

C:\jamulus\windows\windows\ASIOSDK2
Move-Item : Ein Teil des Pfads konnte nicht gefunden werden.
In C:\jamulus\windows\deploy_windows.ps1:68 Zeichen:5
+     Move-Item -Path "$TempDir\$Name" -Destination "$WindowsPath\$Dest ...
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : WriteError: (C:\Users\corrad...mp\ASIOSDK2.3.2:DirectoryInfo) [Move-Item], DirectoryNotFoundException
    + FullyQualifiedErrorId : MoveDirectoryItemIOError,Microsoft.PowerShell.Commands.MoveItemCommand

[nico-dr]

You have to replace $PWD with C:\jamulus in your powershell script (or set this variable with your value).

No admin privilege are needed.

[ann0see]

@corrados how do you run the Script? You need to run it from C:\jamulus by executing something like .\windows\deploy_windows.ps1

The error could also be related to something wrong with the temp path.

No admin privilege are needed.

Not totally sure about that. The installer moved files to the wrong place as I executed the script without admin privileges.

[nico-dr]

still some corrections in installer.nsi needed, the (public) desktop link points to c:\Jamulus.exe

[ann0see]

I thought I fixed this error?

How does the Program Files\jamulus directory look like?

If there’s a folder called C then you should try to run the powershell script as Admin. We might have to find better ways to get the directory locations in the ps file.

[nico-dr]

You did, sorry my code wasn't up to date.

For QT v5.12.3 you have to set $QtCompile32 = "msvc2017" and $QtCompile64 = "msvc2017_64" and not 2019

I can confirm that this bug #792 is fixed with the new installer.

The installer may need some more testing (and path imporvments) but now the installer is state of the art, good job!

BTW, I would recommend to place the installer on the website instead of linking to sourceforge, some people may get nervous to download software from an unknow relinked website (in a foreign language?).

[drummer1154]

Two small comments:

• Does the updated installer remove previous misplaced Jamulus x64 installations from C:\Program Files (x86)\Jamulus?
• What about compatibility on WinXP? (NSIS supports it but Jamulus doesn't.)

Cheers
-hg

[ann0see]

Does the updated installer remove previous misplaced Jamulus x64 installations from C:\Program Files (x86)\Jamulus?

No. I don't think so. You should ask the person who originally wrote this script.

find better ways to get the directory locations in the ps file.

Maybe something with the $PSScriptRoot variable? https://stackoverflow.com/questions/5466329/whats-the-best-way-to-determine-the-location-of-the-current-powershell-script

BTW, I would recommend to place the installer on the website instead of linking to sourceforge, some people may get nervous to download software from an unknow relinked website (in a foreign language?).

I'm afraid that corrados doesn't want to move Jamulus from Sourceforge to GitHub #473

[drummer1154]

Does the updated installer remove previous misplaced Jamulus x64 installations from C:\Program Files (x86)\Jamulus?
No. I don't think so. You should ask the person who originally wrote this script.

From my PoV this answer is not very cooperative. If the installer is updated, it still needs to be compatible with previous versions (where the related bug was documented at #738) and it shall also satisfy the above request #792 (comment) - no matter who does the update.

Same with non-Win10 x86 systems.

[ann0see]

If the installer is updated, it still needs to be compatible with previous versions

I totally agree. The problem is that I don't fully understand the script either (I didn't write it), there must be a way to fix this problem. We could detect the folder and remove it? But what about people who have both versions installed? Is this even possible? What do you think?

The installer may need some more testing

Agree. And I think we should also rewrite some parts (#785 , the comment by @pljones
which @drummer1154 mentioned, ...). That's still a lot of work to do. Also the graphics must be updated,...

[drummer1154]

@ann0see Thanks for this answer.

I think the way to detect what is installed is reading the registry.

But what about people who have both versions installed?

I do not really understand this. What is meant by "both versions"? I do not currently see the possibility to have more than one version (x86 or x64) installed as the current NSIS script (wrongly) determines that Jamulus is always installed as x86 SW.

[ann0see]

Ok. I wasn't sure if it is possible to have 32Bit and 64Bit versions of Jamulus installed (and if there even is a 32Bit version).

I think the way to detect what is installed is reading the registry.

Ok. So if a specific key is found, we remove the 32Bit directory?

[ann0see]

Ok. For me Computer\HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Jamulus shows Jamulus (remove only) as DisplayName. So can we therefore execute the value of UninstallString if it's found?

[drummer1154]

Sorry for slow response but formatting "Insert code" correctly took me some time ...
I can only refer to the current installer.nsi where $INSTDIR == "C:\Program Files (x86)\Jamulus":
; add reg keys so that software appears in Windows "Add/Remove Software"
WriteRegStr HKLM "${UNINST_KEY}" "UninstallString" '"$INSTDIR\${UNINSTALL_EXE}"'
where
!define UNINST_KEY "Microsoft\Windows\CurrentVersion\Uninstall\${APP_NAME}"
and
!define APP_NAME "Jamulus"
would suggest that we could find the uninstaller via
HKLM\Microsoft\Windows\CurrentVersion\Uninstall\Jamulus\UninstallString
but - as already said - in Win10 it is really found in
HKLM\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Jamulus\UninstallString
as
"C:\Program Files (x86)\Jamulus\Uninstall.exe".
So
C:\>reg query HKLM\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Jamulus\ /v UninstallString
would return
HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\Jamulus
UninstallString REG_SZ "C:\Program Files (x86)\Jamulus\Uninstall.exe"
but I cannot say how to deduct this from the current installer.nsi - most likely Win10 automatically relocates the key to the correct location in the registry.
Therefore I withdraw my proposal to read the registry. Instead, the method to locate
"C:\Program Files (x86)\Jamulus\Uninstall.exe"
should be using the original (x86) $INSTDIR${UNINSTALL_EXE} (prior to setting the correct x64 path) or even simply checking if the fixed pathname "C:\Program Files (x86)\Jamulus\Uninstall.exe" exists and executing it.

WinXP?

[ann0see]

Ok. I already coded the "Registry" approach, but we can change it of course. If we have real 32 bit installs this could trigger the message all the time which would be really annoying. Should be fixed now since I moved the code in the 64Bit only sections (but it's not yet tested).

Nevertheless, we should find something which only the old installer does to detect a wrong install.

Is it ok to check if you're running 64Bit version of Windows but have Jamulus installed in the 32 Bit folder which we don't know of if it's 64 or 32 Bit?

How do we check if a 64Bit version of Jamulus is in the wrong folder and not a 32Bit version which the user might have installed since he wants the 32Bit version of Jamulus?

What would happen if someone wants to install a 32Bit version of Jamulus on his 64 Bit Windows OS?
Do we need to find a solution to this case?

See https://github.com/ann0see/jamulus/tree/win_installer for the old registry approach.

And I have no idea of Windows XP. My old XP machine doesn't really work so I can't check it unfortunately.

[ann0see]

@drummer1154 Now I check for the Uninstall.exe file and run it automatically if the user selects yes. Sorry for all the goto statements.

[ann0see]

@pljones coming back to your request:

The installer needs a good rewrite.

It should allow the individual user to decide how to install it.

I want to have an "all users" install. Some people might not want that, though.

Todo

I don't want desktop shortcuts. Some people might want them, though.

Todo (do we also need it for the start menu?)
On which page should we ask?
https://stackoverflow.com/questions/1517471/how-to-add-a-desktop-shortcut-option-on-finish-page-in-nsis-installer
See: ann0see#11

I only want the 64bit version installed. And I want the default install location to be correct (not the 32bit location).

Done

But some people may still need the 32bit version, as well, even when running on 64bit.

#792 (comment)

Not sure if that's possible at the moment?

I want to override what the install locations are.

This should be possible now.

Most of all, I want the installer to remember my choices.

Not sure, but we'd need a ini file for that probably.

[corrados]

@corrados Does the latest version work on your PC now? Do you have spaces in the path to the jamulus git repo?

No white spaces. This is what I get using the latest code:

**********************************************************************
Using Qt binaries for Visual C++ located at
C:\Qt\5.12.3\msvc2015_64\bin
**********************************************************************

The Qt binaries for Microsoft Visual C++ 2017 (msvc2017) could not be located. Please install Qt with support for MSVC 2017 before running this
script,then call this script with the Qt install location, for example C:\Qt\5.12.3
In C:\Users\corrados\Documents\TMP\installerbranch\jamulus\windows\deploy_windows.ps1:163 Zeichen:9
+         Throw "The Qt binaries for Microsoft Visual C++ 2017 (msvc201 ...
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : OperationStopped: (The Qt binaries...le C:\Qt\5.12.3:String) [], RuntimeException
    + FullyQualifiedErrorId : The Qt binaries for Microsoft Visual C++ 2017 (msvc2017) could not be located. Please install Qt with support for MSVC 201
   7 before running this script,then call this script with the Qt install location, for example C:\Qt\5.12.3

Maybe it is easier to let Github do the installer generation. Have you done some more tests regarding this?

[ann0see]

Maybe it is easier to let Github do the installer generation.

I already tried that: https://github.com/ann0see/jamulus/runs/1640919706?check_suite_focus=true but getting QT to work with 32 and 64 bit support seems quite difficult.

How did you run this? What's the content of C:\Qt\5.12.3\msvc2015_64
I use 2019 build tools with QT 5.15.2

[ann0see]

I have another idea: What happens if you remove https://github.com/corrados/jamulus/blob/85957edaf69b01a6fed2d07ac10c3a58e8e64f22/windows/deploy_windows.ps1#L122
until

https://github.com/corrados/jamulus/blob/85957edaf69b01a6fed2d07ac10c3a58e8e64f22/windows/deploy_windows.ps1#L138

and set the paths automatically?

[corrados]

I have changed the following code lines:
[string] $QtInstallPath = "C:\Qt\5.15.1",
[string] $QtCompile32 = "msvc2019",
[string] $QtCompile64 = "msvc2019_64",
and now the script succeeds.

[ann0see]

Ok. So you use the 2019 build tools now?

[corrados]

I am using VS2019 since a while now, actually.

[ann0see]

Ah. Ok. If that's not a problem for you, I think we can leave it like that.
Does the installer run correctly? Does it install to the right path and the right directories/files?

[ann0see]

@corrados I've assigned you to the wiki page and would like to have a review (if everything is ok there). Once we merge improve_win_ci into master, you can merge the mentioned pr on the documentation repo into changes.

[geheimerEichkater]

could you please have a look at the images here: https://github.com/corrados/jamulus/tree/improve_win_ci/windows ? We will have to update them.

I see three bmp, those the ones you're addressing? @ann0see Could you pls provide screenshots for me how they look inside the install app on windows (I have no windows machine around)? I'm left a bit clueless as to how exactly they're being displayed and why two of them have a very unusual resolution/aspect ratio, maybe screenshots can clarify that. Also the icons (client, server) in the main branch had already been updated, will they have to be replaced here to avoid merging conflicts?

[ann0see]

Yes. The bmp files are loaded in the installer.

Uninstaller start page:

One page in the installer/uninstaller:

Installer start page:

Also the icons (client, server) in the main branch had already been updated, will they have to be replaced here to avoid merging conflicts?

I've already replaced them. We could load them from src/res propably.

We can also discuss this this evening?

[corrados]

I just tried out the new installer on a Windows 32 bit version and got a strange result. Previously, there was an old Jamulus version installed which I did not uninstall prior to installing the new version. Then the new installer seemed to work fine (no error). I let the "create desktop icon" and "run Jamulus" ticked and after clicking finish, the old Jamulus version was started. Even if I click on the newly created icon on the desktop, the old Jamulus version is started. When I look into the program files/jamulus directory, it seems only the old files are there, nothing new added.

[corrados]

I now uninstalled the old Jamulus version and started the new installer again. It seems it does not install anything in C:\Program Files.

[ann0see]

That's really strange. Although I couldn't test it on a 32 Bit PC. Did the installer tell you where the files are installed?

Your comment reminds me of another issue:

We might need to remove old versions of Jamulus automatically with the installer. #814

[corrados]

Did the installer tell you where the files are installed?

No, the installer did tell me nothing. The installation process was way too fast. It seems it left out the actual step of copying the files somehow.

[ann0see]

Ah, yes. I had this issue once too. Are the last lines of the ps1 script commented? If that’s the case, you might have to uncomment that.

[geheimerEichkater]

We can also discuss this this evening?

Didn't get that far, though it sure was a pleasure getting to talk to you!

I´ve taken the screens (thanks a lot 👍) and updated them. Since the icon will already be on display in the upper left corner of the window, I would see it more as a branding opportunity rather than an analogy/display of what an un-/installation looks like and suggest the graphics this way:

Unless there's a useablilty reason for having two different graphics, I'd simply suggest the use of the same splash for installing and uninstalling, would you (not) agree?

For the EULA I'd also suggest the extended graphic rather than the icon.

If you're fine with that, I'll replace the bmp files in the branch.

[ann0see]

Didn't get that far, though it sure was a pleasure getting to talk to you!

It was a pleasure for me too. ;-)

If you're fine with that, I'll replace the bmp files in the branch.

Yes, this looks great.

I don't think we necessarily need two different pictures for install/uninstall. Any other thoughts on this question?

[ann0see]

Ok. Made some tests in a 32 bit Windows 8.1 VM and I can reproduce the error of @corrados. I'm doing some debugging. Since the compilation of the 32 and 64 bit versions succeed I assume it's a problem with NSIS.

Ok. It does install the 32 Bit version if I move the 64 bit "Install" section down. We might need to check for 32/64 Bit differently or rewrite this section

[ann0see]

Ok. Should be fixed now. I think the problem was that NSIS doesn't allow you to disable/enable sections. Not sure how clean my approach is, but it seems to work on 32 bit Windows 8.1 and 64 Bit Windows 10. The only problem I saw is that the installer now says it wants to install more than 100 MB although the directory is only 46.3 MB big on 32 Bit Win 8.1 and 55.9 MB on Win 10 64 Bit.

The installer might think that he has to install both versions although he doesn't.

[corrados]

@ann0see What is the current status of the new Windows installer on the Git branch? Is it ready for merge?

[ann0see]

It is ready to merge once the graphics are done.

I will open a PR and explain there what still needs to be done.

[geheimerEichkater]

Graphics are updated, pull request is online.