-
Notifications
You must be signed in to change notification settings - Fork 136
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-60) fix upgrade/installation Filestream errors on PSv2 #92
Conversation
|
||
function Download-File { | ||
param ( | ||
[string]$url, | ||
[string]$file | ||
) | ||
Write-Host "Downloading $url to $file" | ||
Write-Output "Downloading $url to $file" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fully on board with this idea. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've replaced the final ones of these to write-output in chocolateySetup.psm1 as well.
} | ||
else { | ||
$output = Invoke-Expression $chocInstallPS1 | ||
$output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean we'll be waiting the entire run before are able to see output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you will see the output, but not the error log... if you want to see the errors, like on all the silentlycontinue stuff, it is going to throw a lot of people off, on perfectly fine installs due to inconsequentialities. i tried on a dozen servers of varying configs, and think that if people have an issue with an install gone bad, they can be referred to the error log.
We can add a write-output that states the error logs for PS2 version folks are found in $env:temp if they have any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like that. Adding a little message just in here. Nevermind, I see you are doing that already.
…on PSv2 Powershell version 2, has several issues in handling output redirection This is best documented here: https://connect.microsoft.com/PowerShell/feedback/details/587165/ powershell-double-redirection-problem Another obscure bug that occurs in powershell is here. https://connect.microsoft.com/PowerShell/feedback/details/371321/ benign-errors-for-missing-closing-are-left-in-the-error-variable These 2 issues are compounded in the provider, because the benign error sent to $error, then triggers the double redirection issue within puppet. This will automatically fail the resource. This patch is confined to Powershell v2 (Win2008r2/Win7) and essentially captures the output from running chocolateyInstall.ps1 via a variable. We can now see the errors that are thrown instead of the FileStream errors in a log stored here: $env:TEMP\chocoError.log While this may cause a false positive success to be returned to puppet, IMHO this is better than a false failure, based on upstream legacy bugs to be returned, particularly because of the transparency around the $error content. This fix leaves the existing logic intact for Powershell versions greater than 2 as I have not observed this behavior on them. Additionally write-host calls were switched to write-output as a best practice to avoid observed Filestream errors within this edge case. The recommendation to avoid this scenario altogether on Win2008r2 is to install .NET 4.5 and then WMF 4.0, or use Win2012+
$output = Invoke-Expression $chocInstallPS1 | ||
$output | ||
write-output "Any errors that occured during install or upgrade are logged here: $chocoErrorLog" | ||
$error | out-file $chocErrorLog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this will help. its an edge case thing, and ppl who encounter a real issue, we'll see why when they post their log, instead of the filestream bs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that would be fantastic.
(GH-60) fix upgrade/installation Filestream errors on PSv2
(maint) modulesync cd884db Remove AppVeyor OpenSSL update on Ruby 2.4
Powershell version 2, has several issues in handling output
redirection
This is best documented here:
https://connect.microsoft.com/PowerShell/feedback/details/587165/
powershell-double-redirection-problem
Another obscure bug that occurs in powershell is here.
https://connect.microsoft.com/PowerShell/feedback/details/371321/
benign-errors-for-missing-closing-are-left-in-the-error-variable
These 2 issues are compounded in the provider, because the benign
error sent to $error, then triggers the double redirection issue
within puppet. This will automatically fail the resource.
This patch is confined to Powershell v2 (Win2008r2/Win7) and
essentially captures the output from running chocolateyInstall.ps1
via a variable. We can now see the errors that are thrown instead
of the FileStream errors in a log stored here:
$env:TEMP\chocoError.log
While this may cause a false positive success to be returned to
puppet, IMHO this is better than a false failure, based on upstream
legacy bugs to be returned, particularly because of the transparency
around the $error content.
This fix leaves the existing logic intact for Powershell versions
greater than 2 as I have not observed this behavior on them.
Additionally write-host calls were switched to write-output as a best
practice to avoid observed Filestream errors within this edge case.
The recommendation to avoid this scenario altogether on Win2008r2
is to install .NET 4.5 and then WMF 4.0, or use Win2012+