Fix #67 - Reduce command limit to 8161

[bryphe]

This fixes #67 - on esy, we were hitting a case where we were supplied a command with a length of 8178 - this would give us a 'Command line too long error'.

It's a bit strange, because the officially documented limit for the command line length is 8191.

However, I tested this with a quick sample program (attached: test.c.txt)

The results were surprising - I was getting the exception at 8161 characters long:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Iteration 8160 - return code: 0
The command line is too long.
Iteration 8161 - return code: 1

I considered that it might be a difference using the mingw toolchain vs a native compiler, but even compiling with the VS cl compiler, I hit the same limit.

Lowering this limit to 8161 allows for esy to build successfully on Windows.

[dra27]

Sorry for lag. The limit is indeed 8191, but that's for the actual call to CreateProcess. The system call goes via the shell, and the invocation to cmd is part of that. I haven't gone through the history of the call (because I haven't got that repo on a VM to hand!), but the current behaviour of the UCRT system function is to read the COMSPEC environment variable. The default on virtually all systems will be the 27-character string C:\WINDOWS\system32\cmd.exe. This is passed on to one of the exec functions with the arguments /c and whatever command you passed to system.

The 8160 maximum is therefore 8191

• less 27 characters in COMSPEC
• less 2 characters for /c
• less 2 padding space characters when execve converts the argument array to a single command line

If you run your sample after running set COMSPEC= you can get command lines up to 8180 since system then defaults to calling cmd.exe which is 20 characters shorter, which I expect is approximately where the original came from.

[bryphe]

Sorry for lag.

No worries @dra27 ! Thank you for all the details, and for solving the mystery of why the length had to be reduced to 8161 (and the mystery of how the original constant - 8182 - was arrived at!)

And I've incorporated your suggestion - it'll certainly be helpful in the future to have that code documenting how the max_command_length is determined, as opposed to just a magic number 👍

[dra27]

Thanks for the quick patches @bryphe! @alainfrisch, I think this is good to merge.

[dra27]

Actually, while looking at something else, there's a similar problem in Reloc.run_command - it has an incorrect hard-coded constant of 8192, this line should become:

     String.length cmd_quiet > max_native_command_length

[bryphe]

Good catch @dra27 ! That does look suspicious - I replaced that hardcoded value with max_command_length. I also did a pass to see if I could find any other related hardcoded constants, but didn't see any.

[dra27]

@bryphe - AppVeyor should be working again now, would you be able to rebase this one last time and then hopefully I can merge it!

[bryphe]

Sure thing @dra27 , thanks for the appveyor fix! Rebased, fingers-crossed it's green this time

[dra27]

Huzzah - we have an AppVeyor pass! Many thanks, @bryphe!

[bryphe]

Hooray! Thank you for your help and time, @dra27