When fixing another bug I found out the exit code lftp returned to shellscripts wasn't what I was expecting it to be.
The following code is in CMD(set) (commands.cc):
if(a==0)
{
xstring_ca s(ResMgr::Format(with_defaults,only_defaults));
OutputJob *out=new OutputJob(output.borrow(), args->a0());
Job *j=new echoJob(s,out);
return j;
}
You would think that if a set fails, the exit code wouldn't be 0.
However, that code spawns a new job, and that job ends the last. It
ends successfully and lftp returns a happy ending.
It makes sense, the exit code is supposed to be the exit code of the
last job executed. But I wouldn't say it is too intuitive, or useful.
You have the same problem when running several jobs in parallel,
because even if one fails, the others will probably return 0. However
I see CMD(set) as a whole, it doesn't matter if it makes a new job, if
it fails I would like to see an error there.
There are 2 scenarios:
-cmd:fail-exit=true:
You want lftp to stop when it finds an error, and chances are you want
that because the exit code is being checked in a shellscript that has
to do something when it fails. For example, download 10 files, 3 in
parallel, but if one fails, don't waste your time and stop, and don't
try to unzip them.
-cmd:fail-exit=false
You don't care if there is an error. In this case it may make sense to
leave the behavior as it is. I don't see how this could be useful
though, because unless the very last command gives you an error, you
won't know. 99% of the time you are going to get a 0.
Regardless of the value of fail-exit, you have the above scenario
where a failed set still returns a 0, and that's wrong, in my opinion.
The patch attached sets a variable with the last error code, and
CmdExec::ExitCode() returns that code if it's not 0. It only works
when fail-exit=true. So if there are any errors, that will be the
returned code, even if it wasn't the last job.
I'm not sure making the behavior between true and false inconsistent
is a good idea. Adding it to both cases of fail-exit is as simple as
moving it out of the fail-exit if (but notice that would need the
patch I included in other mail where exit_code is not 1 every time
exec_parsed_command() is called unless there is an actual error)
Perhaps adding an option to control this would be better (I prefer to
hardcode it).
return_error.patch
Description: Binary data
