Skip to content
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

installer: Change installer privileges to 'lowest' #86

Merged
merged 1 commit into from
Oct 21, 2015

Conversation

kjeremy
Copy link
Contributor

@kjeremy kjeremy commented Oct 20, 2015

Allows the installer to be run with normal user permissions again.

Fixes git-for-windows/git#497

Allows the installer to be run with normal user permissions again.

While http://www.innosetup.org/ishelp/topic_setup_privilegesrequired.htm
does not list `none` as a valid value http://jrsoftware.github.io/issrc/whatsnew.htm
says:

  If you were using PrivilegesRequired=none before, it is recommended to switch
  to PrivilegesRequired=lowest.

which is what this fix does.  The previous value of `admin` was too high.

Tested locally with the 64 bit build.

Fixes git-for-windows/git#497

Signed-off-by: Jeremy Kolb <[email protected]>
@kjeremy kjeremy force-pushed the lowest-user-permissions branch from 25ddadd to 1f3e1f7 Compare October 20, 2015 18:15
@kjeremy
Copy link
Contributor Author

kjeremy commented Oct 20, 2015

I just force pushed this with the suggested changes and did not realize that it would remove the previous discussion. Sorry :(

@dscho
Copy link
Member

dscho commented Oct 21, 2015

I just force pushed this with the suggested changes and did not realize that it would remove the previous discussion. Sorry :

No harm done.

Instead, this is now an excellent Pull Request and it is fun to merge it.

dscho added a commit that referenced this pull request Oct 21, 2015
installer: Change installer privileges to 'lowest'
@dscho dscho merged commit 9d623b4 into git-for-windows:master Oct 21, 2015
@dscho
Copy link
Member

dscho commented Oct 21, 2015

[...force-push...] would remove the previous discussion.

It might have removed the original discussion, but it now lives on in your excellent commit message ;-)

@XhmikosR
Copy link

FYI, this breaks upgrades if the previous version was installed with admin rights.

@White-Tiger
Copy link

well, for me the previous uninstaller asked for admin rights and the new installation continued. Though it was stuck for unknown reasons at 100% installation and I had to kill it but that should be unrelated^^ (some certificate setup sh was "running")

What was/is your problem in more detail?

@dscho
Copy link
Member

dscho commented Nov 11, 2015

this breaks upgrades if the previous version was installed with admin rights.

@XhmikosR would you please provide more information so I have a prayer to understand what problem you are talking about?

@XhmikosR
Copy link

---------------------------
Git 2.6.3 Setup
---------------------------
Setup was unable to create the directory "C:\Program Files\Git\tmp".



Error 5: Access is denied.
---------------------------
OK   
---------------------------

  1. Have 2.6.2 installed elevated in Program Files
  2. Run 2,6.3 installer

When it tries to install the new version after uninstalling the old one, the process isn't elevated hence the issue. The warning message doesn't show up in this specific case so the upgrade process isn't smooth. One has to run the installer again to actually see the message that they need to run elevated manually.

@paladox
Copy link
Contributor

paladox commented Nov 11, 2015

This may be causing git-for-windows/git#526

@XhmikosR
Copy link

That is the same issue, yes.

@dscho
Copy link
Member

dscho commented Nov 15, 2015

So: ouch. What a terrible mistake to merge this. My bad.

@dscho
Copy link
Member

dscho commented Nov 16, 2015

Bah! I should have known!!! The none setting is not at all the same as the lowest one:

https://github.com/jrsoftware/issrc/blob/841a86c32c44a44789e88ad1b74328cfe9842fa7/Projects/Main.pas#L2939-L2940

As you can see, if the setting is lowest, InnoSetup will never elevate. But if it is none, it will elevate because then the AEmulateHighestAvailable flag is true: https://github.com/jrsoftware/issrc/blob/ea65d280c3157d154c07e653bb5b1d9cb2a11d40/Projects/SpawnServer.pas#L217

So: I will revert this change and put none again.

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

Successfully merging this pull request may close these issues.

5 participants