Adopt DMD's MSVC toolchain detection #3415
Conversation
|
@rainers: Looks usable, thx for this. I still didn't expect almost 10 ms for this stuff; the COM stuff alone takes 5-6 ms on my box (and I have 2 installations to iterate over). I changed it to use the latest version found via COM, not the first one (and check for an existing |
Very nice. |
kinke
force-pushed
the
vsoptions
branch
3 times, most recently
from
ed0629f
to
99cf321
Compare
|
For Azure (with unknown number of VS installations; the previous batch file took a few seconds):
2nd time a few seconds later:
|
I guess that's mostly loading DLLs as part of creating the COM object (I just stepped over it in the debugger, and it showed 6 ms, too - for 10 DLLs on a rather fast SSD).
Should be fine. That probably also results in more predictable behvior.
Should be no problem as far as I can see. |
dmd/vsoptions.d
Outdated
| continue; | ||
| scope(exit) SysFreeString(thisInstallDir); | ||
|
|
||
| if (versionString && wcscmp(thisVersionString, versionString) <= 0) |
There was a problem hiding this comment.
Is string comparison of the version good enough? Probably ok, as 3 digit versions are still some time in the future.
There was a problem hiding this comment.
I've verified that 1.x < 10.x.
There was a problem hiding this comment.
I meant 100.x is not less than 99.x.
packaging/README.txt
Outdated
| The MSVC toolchain setup is skipped if LDC is run inside a 'VS Native/Cross | ||
| Tools Command Prompt' (more precisely, if the VSINSTALLDIR environment variable | ||
| is set). You can set the LDC_VSDIR_FORCE environment variable (to some | ||
| non-empty value) to enforce setting up the MSVC toolchain. |
There was a problem hiding this comment.
I haven't checked the C++ code whether it is actually doing this, but I think it should be the default behavior to adapt to the architecture even if the VS/VC installation path is given by the environment variable. It is pretty annoying if you have to call vcvarsall.bat or similar to adapt the LIB environment variable although you have the option -m32/64 to specify just that to LDC.
There was a problem hiding this comment.
Nobody except you has ever complained about that ;), and I still have no idea why that's needed for the DMD build setup.
There was a problem hiding this comment.
I think I have a solution now, checking the VSCMD_ARG_TGT_ARCH env var (VS 2017+) for a mismatch and not skipping the MSVC setup in that case. The environment is still left untouched (thus avoiding potential issues) if the targets match up.
packaging/README.txt
Outdated
| You can set the LDC_VSDIR environment variable to select a specific version, | ||
| e.g., 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Community'. | ||
| The MSVC toolchain setup is skipped if LDC is run inside a 'VS Native/Cross | ||
| Tools Command Prompt' (more precisely, if the VSINSTALLDIR environment variable |
There was a problem hiding this comment.
Do the Build Tools set VSINSTALLDIR aswell? Shouldn't this check VCINSTALLDIR, too?
There was a problem hiding this comment.
It's been like this for years, I've tested it with Build Tools 2015 IIRC years ago, no complaints.
This reduces the overhead for auto-detecting and setting up an MSVC toolchain from a very rough 1 second to about 8 milliseconds on my box. Enabling the auto-detection by default (and so preferring MSVC over the 'internal' toolchain if there's a Visual C++ installation) is now possible, fixing issues like ldc-developers#3402. The MSVC setup now consists of the bare minimum - prepending 3 directories to the LIB env var and 1-2 directories to PATH.
…n't match This doesn't work for VS 2015, but 2017 and 2019. For VS 2015, I haven't found a suitable env var, and I don't want to parse LIB and/or PATH to guess the target architecture (LDC_VSDIR_FORCE can still be used).
This reduces the overhead for auto-detecting and setting up an MSVC toolchain from a very rough 1 second to about 8 milliseconds on my box. Enabling the auto-detection by default (and so preferring MSVC over the 'internal' toolchain if there's a Visual C++ installation) is now possible, fixing issues like #3402.
The MSVC setup now consists of the bare minimum - prepending 3 directories to the
LIBenv var and 1-2 directories toPATH.