0.16.0-beta2 msvc automatic detection

[Daniel-N]

ldc2 works perfectly fine when running from "VS2015 x64 Native Tools Command Prompt", as always!

The new auto detect feature seems to have some issues though, but since it's a new feature... it's not a true regression.

ldc2 test.d
"The filename, directory name or volume label syntax is incorrect.
Error: C:\lang\ldc2\bin\amd64.bat failed with status: 1"
But when manually launching amd64.bat it works fine...
"Using Visual Studio: C:\lang\Microsoft Visual Studio 14.0"

Which I found curious, and decided to use process monitor to try and figure out what was up, it claimed that the following command failed:

"C:\lang\ldc2\bin\amd64.bat link.exe \NOLOGO \LARGEADDRESSAWARE \OPT:REF \OPT:ICF \OUT:test.exe test.obj \LIBPATH:C:\lang\ldc2"

All these options to link should use forward slashes!

[kinke]

Thx for reporting, I'll look into it tomorrow (I guess).

[kinke]

Okay I get the same error with the beta release - but a manually built & installed LDC (branch release-0.16.0) works flawlessly (I've obviously also tested the stuff manually before pushing the PR).
@Daniel-N: could you please test the current master CI build at https://github.com/ldc-developers/ldc/releases/tag/LDC-Win64-master and post your findings?

[kinke]

Btw, when using the verbose -v switch, you'll see the invoked command line, e.g.:

C:\ldc2-0.16.0-beta2-win64-msvc\bin\amd64.bat link.exe /NOLOGO /LARGEADDRESSAWARE /OPT:REF /OPT:ICF /OUT:hello.exe hello.obj /LIBPATH:C:\ldc2-0.16.0-beta2-win64-msvc\bin/../lib legacy_stdio_definitions.lib /LARGEADDRESSAWARE:NO phobos2-ldc.lib druntime-ldc.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib

[kinke]

Oh, looks like it's the couple of backslashes in /LIBPATH messing up LLVM's launch-process logic. Should be fixed by #1153.

[kinke]

@redstar: There's a -L/LARGEADDRESSAWARE:NO default switch in the release's ldc2.conf which isn't necessary anymore (set in linker.cpp - for debug builds, release ones are LA-aware as the problematic relocations only appear for debug infos) and should be removed.

[Daniel-N]

I retested with LDC-Win64-master-297, looking better, but I guess the final issue you fixed in #1153 is not yet included, so I'll check back in a while if there's a 298 build...

[kinke]

The 303 build (now available) includes it + is portable (the problem only surfaced in portable builds).

[Daniel-N]

confirmed, thanks! :)

[kinke]

Cool. Thx again for reporting, having that bug in the final 0.16 release would have been embarrassing. :)

[redstar]

@kinke Thanks for the hint. I remove the switch in the next release.

[rainers]

I still see the same issue caused by the path to the batch file containing back slashes.

[kinke]

@rainers: I guess your installation path includes a space - I've just tested that, and it doesn't work. I've coded a workaround, but it's ugly. All Microsoft's fault of course - this command line sent to CreateProcess():

"C:\LDC\amd64.bat" 1 2 "3" 4

is internally transformed to:

C:\Windows\system32\cmd.exe /c "C:\LDC\bin\amd64.bat" 1 2 "3" 4

Now guess what - cmd.exe treats the command string after /c or /k in a very special way if it begins with double quotes and is followed by other quoted args. In this example, it tries to invoke:

C:\LDC\bin\amd64.bat" 1 2 "3

Another example: C:\Windows\system32\cmd.exe /c "C:\L D C\bin\amd64.bat" 1 2 "3" 4 => C:\L!

The fix seems to be enclosing the whole command with additional double quotes (and no additional escaping!) and using /s for cmd.exe: C:\Windows\system32\cmd.exe /s /c ""C:\L D C\bin\amd64.bat" 1 2 "3" 4". So the command is quoted, but not escaped, i.e., no regular argument to cmd.exe, so we can't use LLVM's executeAndWait() to launch the cmd.exe process. :/
Additionally, LLVM's arg-quoting code is not public, so we can't use that to quote & escape the regular args inside the command... argh!

[rainers]

My path is c:\s\d\ldc\build-ldc2-x64\bin\Debug\ldc2.exe, so it doesn't include spaces. Running the batch file name through the \ -> / replacement works.
But as you pointed out, it's a more general problem with quotes.

I suspect we need the escaping inside linker.cpp anyway (see #1160 and #1163), so we might also use it for escaping the batch file name.

[kinke]

Okay, this should finally be fixed by #1165. @rainers: could you please verify the backslashes in your batch file path don't cause any trouble anymore? You could fetch the CI build from https://ci.appveyor.com/project/kinke/ldc/build/1.0.319/job/sh1hjkllr9ykn620/artifacts.

[rainers]

could you please verify the backslashes in your batch file path don't cause any trouble anymore?

I've merged your PR into my local branch. Seems to work fine. Thanks.

[kinke]

Perfect, thanks Rainer. I'll re-close the issue then; I've tested it with installation paths with and without spaces, in both a VS command prompt and a naked cmd.exe environment, and with 1 successful and 1 failing linking operation, and everything works as expected.