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):

     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:

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.

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).

Attachment: return_error.patch
Description: Binary data

Reply via email to