This looks like a good change, although it's a bit annoying that it can't be more specific with the error message. Unfortunately, this isn't fixable without reimplementing system: sh returns 127 when the command isn't found, and if exec fails, then system behaves as though the command returned 127, so there's no way to distinguish an E2BIG error from exec from a shell exiting with "command not found".

I suppose the best option would be to reimplement system and do better error reporting, but failing that this patch is definitely an improvement over the current state of affairs.

Thanks for looking at this @stedolan !

Unless someone has another opinion on the matter I will merge this in a few days.

I'm curious: in the end, what error message is shown to the user?

At any rate, I'm in favor of merging this, since it cannot be worse than the error message currently displayed...

In my case, something like this:

Error: I/O error: gcc -shared -o '/tmp/camlTOP5b12000.so'   '-L/path'  ... '-L/pathN
   '-Lstdlib' '-Lotherlibs/unix' '-Lotherlibs/str' '-Lotherlibs/dynlink'
  '-Lotherlibs/bigarray' '-Lotherlibs/raw_spacetime_lib' '-Lotherlibs/graph'
  '/tmp/camlTOP5b12000.o'

(where "..." is, obviously, fairly long)

You can test it out with

$ make natruntop
# #directory "/some/genuinly/long/path";;
# ...
# #directory "/some/genuinly/long/path";;
# 3 + 5;;

To confirm, the strace output when this happened was:

[pid 10550] execve("/bin/sh", ["sh", "-c", "gcc -shared -o '/tmp/camlTOP5b12"...], [/* 44 vars */]) = -1 E2BIG (Argument list too long)

That's a scary error message... but better than nothing. Please go ahead with the merge.

Any opinion on whether this should be backported to 4.07?

Meh.

If there was clearly no risk of regression for anyone's workflow, I would say "why not", but the fact that I'm not knowledgeable enough about the horrible things that people do with return codes is giving me pause.

Ack.