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

Fix sc.ini permissions issue on Windows 10 #166

Closed

Conversation

brad-anderson
Copy link
Member

No description provided.

@CyberShadow
Copy link
Member

If you upload an installer EXE, I can test it in an XP VM if you like.

@brad-anderson
Copy link
Member Author

http://gnuk.net/dmd-2.069.2.exe

I might change this to just have it inherit the permissions like you mentioned in the other pull request.

@CyberShadow
Copy link
Member

The installation seemed to have been completed successfully. The log window is closed right after that step though, so I didn't see what any log output would have been.

@brad-anderson
Copy link
Member Author

A silent failure is pretty much exactly what we wanted to happen.

I've pushed a new version that follows you recommendation to inherit permissions instead of granting read to Users. I just did it on the whole installation directory to catch everything.

I've tested and this approach works fine on Windows 10.

@rainers
Copy link
Member

rainers commented Jan 18, 2016

The problem is not specific to Windows 10, I can reproduce it with 8.1, and it should exist for all versions that allow restricted users including XP IIRC. icacls seems available starting with Windows 8, so it might not fit all cases. I think we should still support Windows 7, maybe by a fallback to cacls.

@CyberShadow
Copy link
Member

@rainers Is there an AccessControl API to reset ACLs on a file? If so then we should use that.

@CyberShadow
Copy link
Member

BTW, there should be a Bugzilla issue for this, linking to the forum thread and these discussions, and a link to the Bugzilla in a code comment.

@CyberShadow
Copy link
Member

Would it be realistic to fix the root of the issue (the string replace function using a system temp directory) instead?

@CyberShadow
Copy link
Member

Alternatively, does NSIS provide a way to copy a file's contents but not attributes?

@rainers
Copy link
Member

rainers commented Jan 18, 2016

@rainers
Copy link
Member

rainers commented Jan 18, 2016

Would it be realistic to fix the root of the issue (the string replace function using a system temp directory) instead?

Seems easy: just replace

  GetTempFileName $R2               ; who's new?

with

  Push "$2.tmp"                     ; temp file in same directory
  Pop $R2

in ReplaceInFile.nsh

@rainers
Copy link
Member

rainers commented Jan 18, 2016

I also noticed that the file owner of sc.ini could not be displayed. Not going through the temp folder fixed that, too.
I've made a new PR with the change to ReplaceInFile.nsh: #167

@WalterBright
Copy link
Member

I think we should still support Windows 7

This exact problem was reported to me by a 64 Windows 7 Pro user.

In any case, we must still support Windows 7. A lot of developers are not wanting to move to newer versions of Windows.

@MartinNowak
Copy link
Member

My understanding is that #167 supercedes this PR, so can it be closed?

@brad-anderson
Copy link
Member Author

icacls is available in Windows 7. It has been included in every version of Windows since 2003. #167 works for me anyway, though.

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