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

Install to ProgramData when LocalAppData is broken #308

Merged
merged 12 commits into from
Apr 21, 2015

Conversation

anaisbetts
Copy link
Contributor

For a dizzying array of reasons, installing anything to %LocalAppData% simply doesn't work, or throws bizarre errors. It turns out though, that since nobody knows what the hell to do with C:\ProgramData, nobody looks there either. When installations fail to %LocalAppData%, we're gonna fall back to C:\ProgramData\$PackageName. On Windows 7 where C:\ProgramData doesn't exist, we'll just make it.

This PR also adds an IsInstalledApp property on UpdateManager which lets you tell if the application is currently installed.

Pay Attention To This!

If your application currently is hard-coded to look in %LocalAppData% for stuff, this PR will break you (well technically, these users were never able to install in the first place, but now they will be able to and now you're gonna see weirdness). Instead, use UpdateManager's RootAppDirectory property, or do all of your operations relative to Assembly.GetExecutingAssembly().Location.

TODO

  • Remove our use of LocalAppData in Update.exe
  • Rig Setup to fall back to ProgramData
  • Remove LocalAppData from Squirrel
  • Make sure this works everywhere

if (!useFallbackDir) {
SHGetFolderPath(NULL, CSIDL_LOCAL_APPDATA, NULL, SHGFP_TYPE_CURRENT, targetDir);
} else {
ExpandEnvironmentStrings(L"%HOMEDRIVE%\\ProgramData", targetDir, _countof(targetDir));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this just CSIDL_COMMON_APPDATA? (Older versions use C:\Documents and Settings\All Users\Application Data).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably a better idea

@sandyarmstrong
Copy link

Are there any issues filed describing any of these "dizzying array of reasons"? I'd like to understand this better before committing to a local user install approach.

@anaisbetts
Copy link
Contributor Author

I'd like to understand this better before committing to a local user install approach.

Strangely enough, none of these issues are filed on Squirrel - I suspect that Squirrel hasn't gotten enough Traction In The Enterprise(tm) yet, but we definitely see it on Atom and in the Slack Windows app. Apparently a lot of IT shops have followed the steps in http://www.computerworld.com/article/2485214/microsoft-windows/cryptolocker-how-to-avoid-getting-infected-and-what-to-do-if-you-are.html which ends up blocking anything in AppData. Others have completely borked permissions on AppData because Security(tm), or crappy Antivirus goes insane and blocks anyone executing in AppData

@sandyarmstrong
Copy link

...wow. This sounds like it's going to need to evolve into a "your company fucked up permissions, where would you like to install?" dialog, ultimately.

@anaisbetts
Copy link
Contributor Author

...wow. This sounds like it's going to need to evolve into a "your company fucked up permissions, where would you like to install?" dialog, ultimately.

Well, so this PR does mostly set us up to run anywhere, including (vomits in mouth) Program Files. After this PR, all we need to do is detect when we need to elevate and rerun ourselves.

@stajs
Copy link
Contributor

stajs commented Apr 21, 2015

Strangely enough, none of these issues are filed on Squirrel - I suspect that Squirrel hasn't gotten enough Traction In The Enterprise(tm) yet

Sadly, I work at an Enterprise(tm) place and yep, our Group Policy blocks executables in %LocalAppData%. I didn't file an issue about it, because TBH it is the fault of the Enterprise not Squirrel. I did however ask that /docs/troubleshooting.md be updated with a note about this for other Enterprise users.

anaisbetts added a commit that referenced this pull request Apr 21, 2015
Install to ProgramData when LocalAppData is broken
@anaisbetts anaisbetts merged commit 7be8bb0 into master Apr 21, 2015
@anaisbetts anaisbetts deleted the program-data-insanity branch April 21, 2015 01:12
@BarryThePenguin
Copy link
Contributor

image

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