[PATCH] Ideas about the exit code of lftp

--bcaec5215e293723c904a4a96f66
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

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

=C2=A0 if(a=3D=3D0)
=C2=A0 {
=C2=A0 =C2=A0 =C2=A0xstring_ca s(ResMgr::Format(with_defaults,only_defaults=
));
=C2=A0 =C2=A0 =C2=A0OutputJob *out=3Dnew OutputJob(output.borrow(), args->a=
0());
=C2=A0 =C2=A0 =C2=A0Job *j=3Dnew echoJob(s,out);
=C2=A0 =C2=A0 =C2=A0return j;
=C2=A0 }


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=3Dtrue:
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=3Dfalse
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=3Dtrue. 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).

--bcaec5215e293723c904a4a96f66
Content-Type: application/octet-stream; name="return_error.patch"
Content-Disposition: attachment; filename="return_error.patch"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_go4lxms70

LS0tIG9yaWcvQ21kRXhlYy5jYwkyMDExLTA0LTI5IDA2OjU4OjI3LjAwMDAwMDAwMCArMDIwMAor
KysgcGF0Y2hlZF9sZnRwLTQuMi4zL3NyYy9DbWRFeGVjLmNjCTIwMTEtMDUtMjUgMTQ6Mzk6NTQu
MDAwMDAwMDAwICswMjAwCkBAIC0xNTYsNiArMTU2LDcgQEAKICAgIGNhc2UoQ09ORF9BTlkpOgog
ICAgICAgaWYoZXhpdF9jb2RlIT0wICYmIFJlc01ncjo6UXVlcnlCb29sKCJjbWQ6ZmFpbC1leGl0
IiwwKSkKICAgICAgIHsKKyAgICAgICAgIGZhaWxlZF9leGl0X2NvZGU9ZXhpdF9jb2RlOwogCSB3
aGlsZShmZWVkZXIpCiAJICAgIFJlbW92ZUZlZWRlcigpOwogCSBjbWRfYnVmLkVtcHR5KCk7Ci0t
LSBvcmlnL0NtZEV4ZWMuaAkyMDExLTA0LTI5IDA2OjU4OjI3LjAwMDAwMDAwMCArMDIwMAorKysg
cGF0Y2hlZF9sZnRwLTQuMi4zL3NyYy9DbWRFeGVjLmgJMjAxMS0wNS0yNSAxNDo0MToxMi4wMDAw
MDAwMDAgKzAyMDAKQEAgLTczLDYgKzczLDcgQEAKICAgIEJ1ZmZlciBjbWRfYnVmOwogICAgYm9v
bCBwYXJ0aWFsX2NtZDsKICAgIGludCBhbGlhc19maWVsZDsgLy8gbGVuZ3RoIG9mIGV4cGFuZGVk
IGFsaWFzIChhbmQgdHRsIGZvciB1c2VkX2FsaWFzZXMpCisgICBpbnQgZmFpbGVkX2V4aXRfY29k
ZTsKIAogICAgVG91Y2hlZEFsaWFzICp1c2VkX2FsaWFzZXM7CiAgICB2b2lkIGZyZWVfdXNlZF9h
bGlhc2VzKCk7CkBAIC0xNzQsNyArMTc1LDcgQEAKIAogICAgYm9vbCBJZGxlKCk7CS8vIHdoZW4g
d2UgaGF2ZSBubyBjb21tYW5kIHJ1bm5pbmcgYW5kIGNvbW1hbmQgYnVmZmVyIGlzIGVtcHR5CiAg
ICBpbnQgRG9uZSgpOwotICAgaW50IEV4aXRDb2RlKCkgeyByZXR1cm4gZXhpdF9jb2RlOyB9Cisg
ICBpbnQgRXhpdENvZGUoKSB7IHJldHVybiBmYWlsZWRfZXhpdF9jb2RlID8gZmFpbGVkX2V4aXRf
Y29kZSA6IGV4aXRfY29kZTsgfQogICAgaW50IERvKCk7CiAgICB4c3RyaW5nJiBGb3JtYXRTdGF0
dXMoeHN0cmluZyYsaW50LGNvbnN0IGNoYXIgKnByZWZpeD0iXHQiKTsKICAgIHZvaWQgU2hvd1J1
blN0YXR1cyhjb25zdCBTTVRhc2tSZWY8U3RhdHVzTGluZT4mIHMpOwoK
--bcaec5215e293723c904a4a96f66--