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

Fallback to per-machine appdata folder when per-user appdata folder is not found. #567

Closed
wants to merge 4 commits into from

Conversation

cornerbowlsoftware
Copy link

This update is an attempt to move past the "Failed to find local per-user appdata directory." when not found. Issue: wixtoolset/issues#8715

Copy link

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request

@cornerbowlsoftware
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@cornerbowlsoftware
Copy link
Author

@barnson can you please look at this PR when you have time?

Copy link
Member

@barnson barnson left a comment

Choose a reason for hiding this comment

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

I don't think this is the right change. The unelevated engine will always fail when caching to a per-machine path so it's not really a fallback. Off the top of my head, the lack of a per-user cache path should fail as quickly as possible when a per-user cache is needed.

@cornerbowlsoftware
Copy link
Author

I don't think this is the right change. The unelevated engine will always fail when caching to a per-machine path so it's not really a fallback. Off the top of my head, the lack of a per-user cache path should fail as quickly as possible when a per-user cache is needed.

@barnson In my case, I am only installing per-machine and the GMSA account that is remote installing does not have a per-user cache causing the engine to fail. Can we leave the values uninitialized then when they are needed for users installing per-user, fail when the values are read?

@barnson
Copy link
Member

barnson commented Sep 18, 2024

That's what I'm thinking. During planning, the engine knows if a per-user cache location is needed so if the path is NULL, throw an error. That prevents an access denied failure after elevating.

@cornerbowlsoftware
Copy link
Author

@barnson I can certainly help with the task, however it seems like you are the best person for it, and I suspect this is a two liner, one line to ignore the missing value in cache.cpp.CacheInitialize then another to throw the error in cache.cpp.GetRootPath but you are the expert. Please let me know if there is anything I can do to either implement it or help out. Thanks for looking at this and all your work on this project. I am grateful I no longer have to use InstallShield.

@barnson
Copy link
Member

barnson commented Sep 18, 2024

OK, then I'll mark this up for grabs so interested parties know it's available to work on.

@barnson barnson closed this Sep 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants