Win installer - add option to install in Current User (default) or All Users#5276
Win installer - add option to install in Current User (default) or All Users#5276arnonm wants to merge 1 commit intoportfolio-performance:masterfrom
Conversation
|
Hi @arnonm, let me share an initial reaction. First, it would be super helpful to squash these many commits into one. That makes it much easier to review. Additionally, it is always helpful if you rebase your contributions instead of merging the upstream master into your branch. Can you be so kind and force push the squashed changes into this pull request? Second, about the add-ons:
I was able to build it successfully on macOS.
Third, on the Github action workflow. I am building the release installer locally and signing with my code signing certificate (on a secure stick). |
|
Besides the comments above: I think it is a reasonable feature to allow the users to pick the installation method and have the registry keys written accordingly. Before the recent change it was unfortunate: a user specific location and global keys. |
|
I stand corrected. It didn't work for me after I added the extensions for
commandline and guy, and the documentation pointed to a Windows only
solution.
You are correct, it works on a mac. I will remove the windows-build.yml
from the PR and squash the rest
|
buchen
left a comment
There was a problem hiding this comment.
Moving this pull request to "changes requested":
- squash and rebase the many changes to master
- remove the GitHub workflow
- undo the changes to the pom.xml
portfolio-product/pom.xml
Outdated
| <version>${tycho-version}</version> | ||
| <executions> | ||
| <execution> | ||
| <id>materialize-products</id> |
There was a problem hiding this comment.
I believe this configuration is not needed here.
portfolio-product/pom.xml
Outdated
| <argument>PortfolioPerformance-${project.version}-setup.exe</argument> | ||
| </arguments> | ||
| <skip>${installer.skip}</skip> | ||
| <skip>${exesigner.skip}</skip> |
There was a problem hiding this comment.
The problem here is: this section should be skipped either if the installer is not being build (then it cannot be signed) or if the signing is skipped (then we want to skip the signing). Unfortunately, I was not able to figure out how to check for two properties here. And I did not want to introduce yet another combined property.
Therefore one can pick either of both properties. I picked the installer because I (more often) skip the windows installer while still wanting to sign the executable in the update site. I know it can be either/or, but I would prefer not to change this here.
… users Issue: portfolio-performance#5276 Signed-off-by: arnonm <3808041+arnonm@users.noreply.github.com> [squashed commits; rebased to master] Signed-off-by: Andreas Buchen <andreas.buchen@gmail.com>
dae8a5d to
1a6d9d3
Compare
|
@arnonm, FYI, for readability, I have squashed all commits and force pushed them into the pull request |
|
|
||
| AccessControl::GrantOnFile "$INSTDIR" "(BU)" "GenericRead + GenericWrite + Delete" | ||
| Pop $0 | ||
| ${If} $0 == error | ||
| Pop $0 | ||
| DetailPrint `AccessControl error: $0` | ||
| ${EndIf} | ||
| # AccessControl plugin not available in standard NSIS installation | ||
| # AccessControl::GrantOnFile "$INSTDIR" "(BU)" "GenericRead + GenericWrite + Delete" | ||
| # Pop $0 | ||
| # ${If} $0 == error | ||
| # Pop $0 | ||
| # DetailPrint `AccessControl error: $0` | ||
| # ${EndIf} |
There was a problem hiding this comment.
I understand that the plugin is not available in a standard NSIS installation. It needs to be installed.
The purpose is this: if the application is installed into the system location, then the self-update of Eclipse needs the additional permissions. I added this a long time ago because otherwise users get error messages when updating the installation.
My thinking is: we can "just" keep it here as it is. Why? If the user installs for "All users", then he must already have the permission to run GrantOnFile. If the user installed for the current user only, it is not really needed, but it also does not make a difference if it is called.
What do you think?
There was a problem hiding this comment.
I agree. It is not worth the overhead of installing the additional plugin

Add ability to install in Current User w/o Admin rights (default) or to All Users
Can be specificed in command line /CurrentUser or /AllUsers, or via GUI if no command line is given.
The addition of the GUI and Command line require two nsis add-ons which are not available on MacOS, only on Windows, which means the windows executable has to be build on Windows.
Added a flow on windows to build the windows artifact
Currently active on a WinInstaller branch. Leaving it up to the maintainer to decide when this github Actions is called