ChocolateyInstall.ps1 is not returning exit codes to the BAT scripts

[codearoo]

To avoid forgetting about this, I'm logging this issue. I have already discussed with Rob and he acknowledged.

If you run something like 'cinst yyyy' where yyyy is a package that fails to install but DOES EXIST in Chocolatey, it will not trap any errors, and the %errorlevel% after it will always be 0.

If you run 'cinst xxxx' where xxx is some package it can't find, it DOES work correctly:
C:\Users\Administrator>cinst sadfsdfsd
Chocolatey (v0.9.8.23) is installing 'sadfsdfsd' and dependencies. By installing
you accept the license for 'sadfsdfsd' and each dependency you are installing.
Unable to find package 'sadfsdfsd'.

Command 'install' failed (sometimes this indicates a partial failure). Additiona
l info/packages: sadfsdfsd
Reading environment variables from registry. Please wait... Done.

C:\Users\Administrator>echo %errorlevel%
1

And by the way, 'cinst tunnelier' 'cinst vim' always fail on Win2008R1 on Choco 0.9.8.23, but did not fail with older versions. It seems like there is bunch of stuff that just is not working on Win2008R1.. maybe you should advertise that on the main site unless you fix it.

[ferventcoder]

This may or may not have been fixed in source... Thanks for adding the issue though.

[nzbart]

I can confirm that the cpack command doesn't return the error code that I am expecting:

C:\dev\ChocolateyPackages [master +0 ~3 -0]> cpack .\vifm\vifm.nuspec
Calling 'C:\Chocolatey\chocolateyinstall\nuget.exe pack .\vifm\vifm.nuspec -NoPackageAnalysis'.
Attempting to build package from 'vifm.nuspec'.
The element 'metadata' in namespace 'http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd' cannot contain text. List of possible elements expected: 'releaseNotes, frameworkAssemblies, tags, copyright, iconUrl, dependencies, references, language, requireLicenseAcceptance, licenseUrl' in namespace 'http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd'.
Reading environment variables from registry. Please wait... Done.
C:\dev\ChocolateyPackages [master +0 ~3 -0]> $LASTEXITCODE
0

I am using 0.9.8.23.

EDIT: This is covered by Issue #256.

[ferventcoder]

so if you call C:\Chocolatey\chocolateyinstall\nuget.exe pack .\vifm\vifm.nuspec -NoPackageAnalysis directly, what exit code do you get?

[nzbart]

nuget.exe returns 1:

C:\dev\ChocolateyPackages [master +1 ~1 -0 !]> C:\Chocolatey\chocolateyinstall\nuget.exe pack .\vifm\vifm.nuspec -NoPackageAnalysis
Attempting to build package from 'vifm.nuspec'.
The element 'metadata' in namespace 'http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd' cannot contain text. List of possible elements expected: 'releaseNotes, frameworkAssemblies, tags, copyright, iconUrl, dependencies, references, language, requireLicenseAcceptance, licenseUrl' in namespace 'http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd'.
C:\dev\ChocolateyPackages [master +1 ~1 -0 !]> $LASTEXITCODE
1

Note that I manufactured the error in both cases to test the return code by editing my nuspec file to be invalid XML.

[nzbart]

I've written a failing test. I've never used pester before, but hopefully it is correct...

Describe "When calling cpack with an invalid nuspec" {
  cpack 'c:\some\bogus\path'
  $cinstExitCode = $LASTEXITCODE

  It "should return a non-zero exit code" {
    $cinstExitCode | Should Not Be 0
  }
}

[ferventcoder]

Awesome. If you want to take that a step further and provide the fix in a pull request with the tests to prove it, it gets pulled in quite a bit quicker. Also review the contributing guide https://github.com/chocolatey/chocolatey/blob/master/CONTRIBUTING.md

[nzbart]

I've had a go at this, but am struggling to come up with a valid solution and test.

1. The fix appears to be to uncomment #throw $errors at line 22 in Chocolatey-Pack.ps1 and remove Write-Host $errors -BackgroundColor Red -ForegroundColor White from line 21.
2. I have had a look through the git history of that file, but struggled to understand why the line is currently commented out, so I don't really understand the ramifications of reinstating it.
3. My test code in a previous comment (above) is flawed because it will call cpack.bat from the o/s path, not from the current repository. Because cpack.bat is generated, it will not be possible to call it from the current repository.
4. If I call Chocolatey-Pack directly, it will look for nuget.exe in the wrong place.

The only viable solution I can think of (but haven't tried yet) is to mock out Start-Process and Get-Content and call Chocolatey-Pack directly to ensure that it throws an exception. Unfortunately, it is possible for a regression to appear in the .bat file or exception handling some time in the future because this is not a complete end-to-end test. My questions are, do you think this solution is viable, and do you think it is sufficient?

Note: this is my proposed fix (without test):

diff --git a/src/functions/Chocolatey-Pack.ps1 b/src/functions/Chocolatey-Pack.ps1
index c46b2f1..4122389 100644
--- a/src/functions/Chocolatey-Pack.ps1
+++ b/src/functions/Chocolatey-Pack.ps1
@@ -18,7 +18,6 @@ param(
   }
   $errors = Get-Content $errorLogFile
   if ($errors -ne '') {
-    Write-Host $errors -BackgroundColor Red -ForegroundColor White
-    #throw $errors
+    throw $errors
   }
 }
\ No newline at end of file

[ferventcoder]

I would also need to look into why we commented that out.

[ferventcoder]

Going back in time here....:
https://github.com/chocolatey/chocolatey/blob/5525103c3269d9a5abf1e4c795fc057707fdcc6f/src/chocolatey.ps1#L547 and then blame leads to 317ffed#diff-11f8e3cf38099e79da8cf4d78b2edb4dR553 - it looks like it has been there since the beginning and it's beyond me why I did that at the time (other than nuget ALWAYS returning a bad exit code at that version). So with some testing it might be an okay change to make.

[codearoo]

Is this still related to the error not getting returned when install
packages fail? That continues to happen in .23 of course.
On Jan 19, 2014 1:00 PM, "Rob Reynolds" notifications@github.com wrote:

I would also need to look into why we commented that out.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/384#issuecomment-32714705
.

[ferventcoder]

@codearoo Ha, looks like we hijacked this thread. I didn't even pay attention to what the main issue was that was reported.

[nzbart]

Looks like #256 is what we're discussing here...

[ferventcoder]

I believe this is now fixed