-
Notifications
You must be signed in to change notification settings - Fork 342
[Bug] Posh v3+ ignores -Wait when run from cmd.exe #516
Conversation
Running choco install in a cmd window, spew out this error message: ``` DEBUG: wrapping 7za invocation with Write-FileUpdateLog DEBUG: Running 'Write-FileUpdateLog' with logFilePath:'C:\Chocolatey\lib\scala.portable.2.10.4\scala.portableInstall.zip.txt'', locationToMonitor:C:\Tools, Operation: ' param($7zip, $destination, $fileFullPath, [ref]$exitCodeRef) $p = Start-Process $7zip -ArgumentList "x -o`"$destination`" -y `"$fileFullPath`"" -Wait -WindowStyle Hidden -PassThru $exitCodeRef.Value = $p.ExitCode ' DEBUG: Tracking current state of 'C:\Tools' DEBUG: 7za exit code: DEBUG: Renaming 'C:\Users\Claudio\AppData\Local\Temp\chocolatey\scala.portable\success.log' to 'C:\Users\Claudio\AppData\Local\Temp\chocolatey\scala.portable\success.log.old' Write-Error : scala.portable did not finish successfully. Boo to the chocolatey gods! ----------------------- [ERROR] 7-Zip signalled an unknown error (code ) ----------------------- At C:\Chocolatey\chocolateyInstall\helpers\functions\Write-ChocolateyFailure.ps1:30 char:3 + Write-Error $errorMessage + ~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : NotSpecified: (:) [Write-Error], WriteErrorException + FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Write-Error ``` Oddly enough, the error code is not set. Looking in the update log, it seems that 7za stopped halfway through or was just not finished yet. Might be related to http://social.technet.microsoft.com/forums/en-US/3ec5c7b5-4a36-4890-98c2-ae654993f173/powershell-script-startprocess-no-exitcode-when-run-in-sccm-tasksequence
@@ -80,6 +80,7 @@ param( | |||
$unzipOps = { | |||
param($7zip, $destination, $fileFullPath, [ref]$exitCodeRef) | |||
$p = Start-Process $7zip -ArgumentList "x -o`"$destination`" -y `"$fileFullPath`"" -Wait -WindowStyle Hidden -PassThru | |||
Wait-Process -InputObject $p |
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.
It's weird that this would be required when start process above has -Wait
already on it.
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.
It's weird that this would be required when start process above has -Wait already on it.
Yep, that's puzzling. Probably a bug on the powershell side?
As a side note, it worked for me when I ran the cocolatey command from inside powershell, but not inside a cmd window.
I can reproduce the recipe mentioned here.
Indeed, it seems in PowerShell 3+ they started using a job object in Start-Process -Wait (but not in Wait-Process). In PowerShell 2 Start-Process -Wait and Wait-Process used equivalent code. @avdv what OS are you running this on - Win7 or older, by any chance? |
Yes, I'm on Windows 7. |
Is wait-process available in Posh v2? If not we'll need to guard that command. |
- This adds Wait-Process to anywhere Start-Process is called with a guard that this should only be done in PowerShell v3+.
This is merged in at b354494 |
First, a slight correction: I wrote "in PowerShell 3+ they started using a job object in Start-Process -Wait", but, to be precise, I only examined PS 2 and 4 (what I have on my system). So I'm not sure about PS 3. Not that it matters much.
Docs say "Applies To: Windows PowerShell 2.0, Windows PowerShell 3.0", the implemetation I examined was in a .NET 2.0 assembly (so PS 2). But wouldn't hurt to check on a pure PS 2.0 system.
Windows 8 brought a major improvement to job objects in that now they can be nested, so a process in a job can create another job itself. On earlier systems (e.g. 7), the attempt to create a job (to run a child process in, for example, as PS 4 does in Start-Process -Wait) would fail when the process was already part of one. Perhaps Start-Process -Wait does not cope well with the situation when it cannot create the job - the code seems to be written to handle it, but I don't have the time to debug and verify it. Powershell might be running in a job in various circumstances, such as when the application that starts it is itself in a job - e.g. due to Program Compatibility Assistant - or when that application wants to maintain control over PS - as SCCM probably does. As I use Win 8.1, which has nested jobs, the issue did not have a chance of manifesting on my computer. I agree that using Wait-Process is a reasonable workaround, now that a probable explanation of the issue might be given. Although dropping the -Wait and adding a comment would add more clarity to the code. |
This caused a major issue with chocolatey that made me yank the new release shortly after pushing it out. For reference see atom/chocolatey#23 (comment) |
Will need to add some more guards around this... |
The prior attempts at waiting the process resulted in error when the process had already exited. This caused really poor behavior and things to error when they should have been ignored. Further wrapping wait process in try catch is not enough as it will ignore the catch clause, so check to see if the process has exited first prior to continuing execution.
* stable: v0.9.8.27-beta1 (doc) update changelog / nuspec (maint) Debug should capture operating system information (GH-516) If process has not yet exited, wait the process
@jberezanski I added a comment noting why we would wait the process. |
Running choco install in a cmd window, spew out this error message:
Oddly enough, the error code is not set. Looking in the update log, it
seems that 7za stopped halfway through or was just not finished yet.
Might be related to
http://social.technet.microsoft.com/forums/en-US/3ec5c7b5-4a36-4890-98c2-ae654993f173/powershell-script-startprocess-no-exitcode-when-run-in-sccm-tasksequence