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

Tab Completion - Modify profile if file exists but is empty #991

Closed
ownmaster opened this issue Sep 29, 2016 · 23 comments
Closed

Tab Completion - Modify profile if file exists but is empty #991

ownmaster opened this issue Sep 29, 2016 · 23 comments

Comments

@ownmaster
Copy link

ownmaster commented Sep 29, 2016

Just tried to install latest Chocolatey on freshly installed PowerShell 5 (Windows 7 x64).

I did not have profile file, so I've got

Not setting tab completion: Profile file does not exist at '$profileFile'.

warning.

Then I created an empty Microsoft.PowerShell_profile.ps1, deleted choco folder and ran setup again.
This time I've got

Not setting tab completion: File is Authenticode signed at '$profile'.

So I checked chocolateysetup.psm1 code, and found that Get-AuthenticodeSignature $profile produces 'UnknownError' Status on my profile file, I guess it does that for any file smaller than 4 bytes.
Fixed that by adding dummy comment to the file and running choco setup again.

So, I offer:

  1. add an option to installation script - create profile if not exists
  2. before checking for signature, check profile file size and modify it if its empty

Related to #894

@ferventcoder
Copy link
Member

Option 1 is a no-go, it causes issues.

Option 2 is definitely doable.

@ohadschn
Copy link

@ferventcoder out of curiosity, what issues would creating a blank PS file (with a dummy comment) cause? Presumably that's what the user would do anyway to get rid of this warning?

@ferventcoder
Copy link
Member

@ohadschn #834 for reference.

@ferventcoder
Copy link
Member

Causes issues with packer as well.

@ohadschn
Copy link

@ferventcoder thanks. So what's the bottom line for the user here? Should we create a blank profile manually like ownmaster did for the tab completion, or would that cause issues?

@ferventcoder
Copy link
Member

If the file exists, ensure it is not empty before the authenticode check (which errors when the file is less than 4 bytes). Then it will add the profile automatically.

The check will come at

$signature = Get-AuthenticodeSignature $profile
if ($signature.Status -ne 'NotSigned') {
Write-Warning "Not setting tab completion: File is Authenticode signed at '$profile'."
return
}
, right after we've determined the file exists.

@ferventcoder ferventcoder modified the milestones: 0.10.4, 0.10.x Nov 14, 2016
@ohadschn
Copy link

Thank you. So that raises the question, why not incorporate that same logic (if file exist, create one with a dummy comment) into the Chocolatey installer?

@ferventcoder
Copy link
Member

ferventcoder commented Nov 14, 2016

@ohadschn if file exists or if file does not exist?

We can't simply create the file - that causes issues in packer, vagrant and other scenarios (linked previously at #834).

However if the file already exists but is empty, that's what we are already talking about here, add the profile to it as part of the changes that would come from this issue.

@ohadschn
Copy link

ohadschn commented Nov 14, 2016

Thanks, that settles the case where it does exist and is empty.

Now suppose the file doesn't exist. You're saying I shouldn't create it myself due to possible issues (in packer, vagrant, and others)? If so, I'm guessing those packages should be fixed (as I'm assuming an empty PS profile file is legitimate)?

@ferventcoder
Copy link
Member

Now suppose the file doesn't exist. You're saying I shouldn't create it myself due to possible issues (in packer, vagrant, and others)? If so, I'm guessing those packages should be fixed (as I'm assuming an empty PS profile file is legitimate)?

Those are not package issues.

@ferventcoder
Copy link
Member

Sorry, what I am saying is that it is not a "package" that is having an issue - we are talking environments and tools that are running into PowerShell issues because of having a profile. It's actually most likely a PowerShell bug more than it is anything else.

@ohadschn
Copy link

Understood, thanks. So to sum up, a person encountering this issue currently has two options:

  1. Create an empty PS profile with a dummy comment as described by ownmaster at his own peril (as the mere existence of a PS profile can evidently cause issues). Once that is done, uninstall and reinstall choco and tab completion should work.
  2. Eschew PS profiles and therefore tab autocompletion.

If I got it right, perhaps the warning message could be modified to explain this?

@ferventcoder
Copy link
Member

Other options:

@ohadschn
Copy link

OK but those two will still result in a PS profile in place, and hence possibly cause issues with the packages we've mentioned, correct?

@ferventcoder
Copy link
Member

ferventcoder commented Nov 15, 2016

Again, not packages. The packages have no issues. Perhaps you have never used either tool.

When you are running Vagrant or Packer (not with choco install packagename, but actually using those tools and installing Chocolatey itself and using PowerShell), you won't set up a profile in the machine you are building or running unless you explicitly set it up that way.

@ferventcoder
Copy link
Member

ferventcoder commented Nov 15, 2016

Please read over #834 and let me know where you need help understanding so I can help fill in the gaps.

@ohadschn
Copy link

ohadschn commented Nov 15, 2016

Thanks @ferventcoder . You were right, I never used either tool, but quickly glancing over the Vagrant docs I think I get it (kind of like docker).

So let me try that again -
In environments created by tools such as Vagrant or Packer, it is undesirable to have PS profiles as those might cause issues, specifically when trying to manage those environments using said tools. For that reason, it would be problematic for choco to automatically add a PS profile without understanding the environment under which it is installed. As such, #833 was implemented.

As for "home" users, they can either add the profile manually, or install it using choco profile add once #894 is completed.

If I got it right so far, I just have one suggestion to consider. It seems like a very specialized scenario to affect all choco installations, so perhaps make the profile installation the default. Then have some environment variable (or a parameter to the installer) that allows overriding it for such environments.

@ferventcoder
Copy link
Member

Docker may also suffer the same issues - it may feel like specialized scenarios at first glance, but requiring one more switch always tends to lead to confusion, as we've learned, especially so if it is required in some settings and not in others.

Currently we are dealing with an example of this is in useWindowsCompression needing to be set false prior to install of choco in some environments, like Server Core.

@ohadschn
Copy link

ohadschn commented Nov 16, 2016

Fair enough, I suppose it makes sense to have the default as compatible as possible. It's pretty strange though that the mere existence of a PS profile, even an empty one, trips up such environments. Do we understand why?

@ferventcoder
Copy link
Member

ferventcoder commented Nov 17, 2016

Something, possibly a bug, in PowerShell v5. There are many areas to focus on, and sometimes some things really are not worth diving into understanding why. This to me felt like one of them. There was a place someone mentioned it triggers some update progress bar in regular PowerShell calls (outside of Chocolatey) that completely destroys the automation. Just due to having a profile file.

@ohadschn
Copy link

Interesting. I totally get not diving into this, if anything it's something the vagrant et al. devs should probably look into. Thanks for all your help and patience!

@ferventcoder ferventcoder modified the milestones: 0.10.7, 0.10.8 Jun 6, 2017
@ferventcoder ferventcoder modified the milestones: 0.10.9, 0.10.10 Aug 27, 2017
@ferventcoder ferventcoder modified the milestones: 0.10.10, 0.10.12 Mar 27, 2018
@ferventcoder ferventcoder modified the milestones: 0.10.12, 0.10.13 May 3, 2018
@ferventcoder ferventcoder self-assigned this May 4, 2018
@ferventcoder ferventcoder modified the milestones: 0.10.13, 0.10.11 May 4, 2018
@ferventcoder ferventcoder changed the title Not setting tab completion tab completion: modify profile if file exists but is empty May 4, 2018
@ferventcoder ferventcoder changed the title tab completion: modify profile if file exists but is empty Tab Completion: modify profile if file exists but is empty May 4, 2018
@ferventcoder ferventcoder changed the title Tab Completion: modify profile if file exists but is empty Tab Completion - Modify profile if file exists but is empty May 4, 2018
ferventcoder added a commit that referenced this issue May 4, 2018
When installing Chocolatey, install the tab completion if the file
exists and is empty. Previously, it would check authenticode signature
on an empty file if it existed and that would error, assuming that the
file was authenticode signed. Now check the file size first to be sure
that it is bigger than 4 bytes. If it is not, then it an empty file and
it is safe to assume it is not signed either.
ferventcoder added a commit that referenced this issue May 4, 2018
* stable:
  (version) 0.10.11
  (doc) update release notes for 0.10.11
  (GH-1540) Fix - AutoUninstaller can't find escaped quotes
  (GH-1543) Fix - log format error on install location with GUID
  (GH-1500) Fix - Wrap Write-Host if used in setup
  (GH-991) Install - Modify profile if file exists & empty
  (GH-1443) Set original config before loop
  (maint) add a note for GH-1548
  (GH-1557) Upgrade 7z to 18.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants