`Exec` is not friendly to valid exit codes not equal to 0 #16

Closed
nightroman opened this Issue Jul 20, 2011 · 6 comments

Comments

Projects
None yet
3 participants
Contributor

nightroman commented Jul 20, 2011

Just like in MSBuild the Exec task in psake is not friendly to valid exit codes not equal to 0. A practical example: for the popular tool robocopy.exe exit codes 1, 2, 3 are valid exit codes and codes greater than 3 indicate real errors.

MSBuild at least allows ignoring exit codes. That is only half a solution, actually. It also allows analysis of the exit code but the syntax is so convoluted that I never can remember it.

In the case of robocopy it would be nice if we can tell the build to fail if and only if an exit code is greater than 3.

Owner

JamesKovacs commented Aug 5, 2011

This is non-standard shell behaviour where 0 indicates success and anything
else indicates failure. The only reason for exec {} in psake is to turn a
non-zero exit code into an exception. You can easily write your own version
of exec {} that handles your case. Take a look at "function Exec" in
psake.psm1, which is currently on line 127.

HTH,

James

James Kovacs, B.Sc., M.Sc.
http://jameskovacs.com
jkovacs@post.harvard.edu
@jameskovacs (Twitter)
403-397-3177 (mobile)
jameskovacs (Skype)

On Wed, Jul 20, 2011 at 5:00 AM, nightroman <
reply@reply.github.com>wrote:

Just like in MSBuild the Exec task in psake is not friendly to valid exit
codes not equal to 0. A practical example: for the popular tool robocopy.exe
exit codes 1, 2, 3 are valid exit codes and codes greater than 3 indicate
real errors.

MSBuild at least allows ignoring exit codes. That is only half a solution,
actually. It also allows analysis of the exit code but the syntax is so
convoluted that I never can remember it.

In the case of robocopy it would be nice if we can tell the build to fail
if and only if an exit code is greater than 3.

Reply to this email directly or view it on GitHub:
https://github.com/JamesKovacs/psake/issues/16

Contributor

nightroman commented Aug 6, 2011

The only reason for exec {} in psake is to turn a non-zero exit code into an exception.

If it is then I agree - the current Exec will do.

But I see other reasons as well. Diagnostics, logging, error messages (nice default one: ($msgs.error_bad_command -f $cmd)), etc, etc. can be done by the Exec and the psake engine uniformly. A caller of Exec should not invent a wheel.

In case of robocopy.exe (non-standard, yes) a user has to invent his own simple but boring solutions like
If ($lasexitcode -gt 3) { ... here comes an invented wheel, with mistakes, perhaps, lousy error messages, etc. ... }

Well, that was just a note to consider. We can live without proposed feature just fine.
Still, why not this easy and handy extra parameter:
[parameter()]
$ValidateExitCode = { $lastexitcode -ne 0 }
{
& $cmd
if (!(& $ValidateExitCode)) {
throw ("Exec: " + $errorMessage) #### at least this piece should not be invented by a user
}
}

Contributor

nightroman commented Aug 6, 2011

Of course it should be
$ValidateExitCode = { $lastexitcode -eq 0 }

Owner

JamesKovacs commented Aug 6, 2011

I can see where this type of helper function would be useful for certain
people. I would recommend submitting a pull request to psake-contrib, which
is meant for non-core helper functions and extensions.

https://github.com/JamesKovacs/psake-contrib

James

James Kovacs, B.Sc., M.Sc.
http://jameskovacs.com
jkovacs@post.harvard.edu
@jameskovacs (Twitter)
403-397-3177 (mobile)
jameskovacs (Skype)

On Fri, Aug 5, 2011 at 10:52 PM, nightroman <
reply@reply.github.com>wrote:

Of course it should be
$ValidateExitCode = { $lastexitcode -eq 0 }

Reply to this email directly or view it on GitHub:
https://github.com/JamesKovacs/psake/issues/16#issuecomment-1740776

nightroman closed this Sep 6, 2011

@nightroman, I'm with you! I've been automating Windows configuration with PowerShell and psake and just about every native command I've used has non-zero successful exit codes. And these utilities are from Microsoft. A few examples are

  • dism (3010) - Restart required. You aren't going to restart in the middle of a script.
  • nvspbind (11) - No action taken. You don't want to fail because a NIC protocol was already disabled.

Remember, an exit code is not automatically an error code.

Owner

JamesKovacs commented Jul 14, 2012

In the vast majority of cases, non-zero exit codes mean error. If a non-zero exit code is NOT an error, then you're best writing a custom PowerShell function that takes this into account rather than using exec. The problem here is that the successful (or non-erroneous) non-zero exit code is going to be command dependent. I don't want to pollute exec with a whole lot of knowledge about different commands. To be perfectly honest, I'd rather exec didn't exist at all and PowerShell handled exit codes correctly - turning them into exceptions where appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment