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

(GH-629) Separate app.manifest #630

Merged

Conversation

ferventcoder
Copy link
Member

@ferventcoder ferventcoder commented Dec 5, 2018

By default highestAvailable is desired for UAC prompting behavior, but sometimes there are groups that provide some level of elevation (Network Configuration Operators Group) that also mean that UAC will
prompt. This is undesirable behavior in shops that might want to use self-service and have Chocolatey GUI run as the user that invoked without any elevation being requested. To allow this behavior, app.manifest can not be embedded, but must sit next to ChocolateyGUI.exe so that it can be changed for those situations that require it by those users.

Closes #629

@ferventcoder
Copy link
Member Author

This is in WIP because it appears there is some work to do to see that the ChocolateyGUI.exe.manifest gets included in the MSI

By default highestAvailable is desired for UAC prompting behavior, but
sometimes there are groups that provide some level of elevation
(Network Configuration Operators Group) that also mean that UAC will
prompt. This is undesirable behavior in shops that might want to use
self-service and have Chocolatey GUI run as the user that invoked
without any elevation being requested. To allow this behavior,
app.manifest can not be embedded, but must sit next to
ChocolateyGUI.exe so that it can be changed for those situations that
require it by those users.
@ferventcoder ferventcoder force-pushed the ticket/629_separate_app_manifest branch from 8d62716 to dc7bc74 Compare December 5, 2018 23:51
@ferventcoder
Copy link
Member Author

I took a cursory look but I don't know where I might need to change to be sure that ChocolateyGUI.exe.manifest gets included. The XSLT filters "ChocolateyGUI.exe" files out and then somewhere else the config and exe are put back in as named components. I think that the manifest needs the same treatment but I have not tracked down where that is just yet. @gep13 ?

@ferventcoder ferventcoder requested a review from gep13 December 6, 2018 00:18
@gep13
Copy link
Member

gep13 commented Dec 6, 2018

@ferventcoder I have just pushed a new commit onto your branch.

With this change in place, the MSI that is generated now includes the new manifest file, and on installation, it can be found here:

image

Next to the ChocolateyGUI.exe

@ferventcoder ferventcoder changed the title [WIP] (GH-629) Separate app.manifest (GH-629) Separate app.manifest Dec 6, 2018
@ferventcoder ferventcoder changed the title (GH-629) Separate app.manifest [WIPdd] (GH-629) Separate app.manifest Dec 6, 2018
@ferventcoder ferventcoder changed the title [WIPdd] (GH-629) Separate app.manifest (GH-629) Separate app.manifest Dec 6, 2018
@ferventcoder

This comment has been minimized.

@@ -92,6 +92,9 @@
<Component Id="ChocolateyGui.exe.config" Guid="6C3AE56F-1AA3-4145-BFA9-345BBCC5C3D1">
<File Id="ChocolateyGui.exe.config" Source="$(var.GuiTargetDir)/ChocolateyGui.exe.config" KeyPath="yes" Checksum="yes" />
</Component>
<Component Id="ChocolateyGui.exe.manifest" Guid="AEA5D8AE-4274-444E-AEB5-988492C6919E">
<File Id="ChocolateyGui.exe.manifest" Source="$(var.GuiTargetDir)/ChocolateyGui.exe.manifest" KeyPath="yes" Checksum="yes" />
</Component>
Copy link
Member Author

Choose a reason for hiding this comment

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

I figured it was something like that. Thanks for catching that!

@ferventcoder

This comment has been minimized.

@ferventcoder
Copy link
Member Author

We should add these notes in as well for folks changing this so they don't scratch their heads - chocolatey/choco#1292 (comment)

Windows behavior of manifest caching is enough to make even the best of
admins and developers want to tear out their hair until they realize
there is a cache and changes to the manifest are completely ignored by
Windows. So leave a breadcrumb here so that folks know if they make a
change to the manifest file, they will need to "touch" the file they
are trying to get the manifest to take effect for - in this case
ChocolateyGUI.exe. The modification date will need changed to clue
Windows in that there is a new manifest.

This is only when changing the file AFTER the exe / modification date
has been run even once. It's better to just say to always update the
modification date as someone may not know whether the file has been ran
before.
@ferventcoder
Copy link
Member Author

I added some notes to keep folks from being perplexed IF they do change this file. This is ready for review.

Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13 gep13 merged commit 30ae51e into chocolatey:develop Dec 6, 2018
@gep13
Copy link
Member

gep13 commented Dec 6, 2018

@ferventcoder your changes have been merged, thanks for your contribution 👍

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.

2 participants