deplinenoise / tundra

Tundra is a code build system that tries to be accurate and fast for incremental builds
MIT License
438 stars 75 forks source link

Initial support for Visual Studio 2017 #291

Closed kayru closed 6 years ago

kayru commented 7 years ago

Is there anything preventing this from being merged?

leidegre commented 7 years ago

I'm trying to test this right now, I'll let you know what I find. Microsoft sure did mix things up for this release. And I'm sure @deplinenoise will merge the changes as soon as he's confident they work.

leidegre commented 7 years ago

It looks like Microsoft has decided to not use the registry from now on. Starting with Visual Studio 2017 they have a different installation process altogether.

A direct consequence of this is that there isn't a registry entry for the build tools only installation option, so the lines that check for that in msvc-common.lua won't work with 2017 build tools. I'm assuming @kayru got them working with VS actually installed, I've opted to not install the VS IDE.

Furthermore, there does appear to be a vswhere utility that can locate installations for you based on various parameters, I think such a utility could be useful in building a future vswhere-* tool chain script that gets the necessary information by querying this utility. I will put it on my TODO list because it doesn't appear to work with the VS 2017 build tools specifically, right now.

Meanwhile, it think we should forgo the registry check when targeting VS 2017. There's absolutly no reason to not just hard code the VS 2017 root dir to C:\Program Files (x86)\Microsoft Visual Studio\2017 it's not going to be anything else and it's not something you have control over. Then provide the edition you'd like to use as an option.

For example, my installation looks like this

C:\Program Files (x86)\Microsoft Visual Studio\2017>dir /b
BuildTools

C:\Program Files (x86)\Microsoft Visual Studio\2017>set VSINSTALLDIR && set VCINSTALLDIR
VSINSTALLDIR=C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\
VCINSTALLDIR=C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\

I'm guessing editions other than BuildTools here would be Community,Professional and Enterprise. We can default to something but I think it needs to be configurable externally, like arch and platform target. I think we should add an Edition option with a sensible default.

Also, the VS 2017 build tools are using the Windows 10 SDK

WindowsSdkBinPath=C:\Program Files (x86)\Windows Kits\10\bin\
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\
WindowsSDKLibVersion=10.0.15063.0\
WindowsSdkVerBinPath=C:\Program Files (x86)\Windows Kits\10\bin\10.0.15063.0\
WindowsSDKVersion=10.0.15063.0\

With these changes, I can get the VS 2017 build tools to run just fine like this:

Tools = { { "msvc-vs2017"; TargetArch = "x64"; Edition = "BuildTools", SdkVersion = "v10.0.15063.0" } },

I have staged my changes on top of your latest commit @kayru if you want to incorporate any of these changes into your pull request.

I think a clean break is in order, given how much VS has diverged from its historical design with VS 2017 and Win10 SDK. A msvc-common2 or similar might be better in the future because that file is getting quite tricky to navigate and I think it's mostly legacy that will remain unchanged from now on.

deplinenoise commented 7 years ago

Sorry for the slow replies, I've been under water with other stuff.

Based on John's findings, I agree it would make sense to provide vs2017 as a new, streamlined module without any of the cruft. I'm reluctant to merge as-is.

kayru commented 7 years ago

Agreed. It does seem like some additional infrastructure is going to be required to handle new VS layout schemes. I probably won't have time to look at this for a few weeks myself, but don't mind if @leidegre takes it over :)

leidegre commented 7 years ago

@kayru I'll fix the tool chain support to begin with. I know Microsoft has had an internal spat regarding MSBuild project files, I don't know what the end result was. I figured out how the vswhere tool works as well so we might use that as an option. I'm currently installing additional editions of VS 2017 and SDKs so that I can test these a bit.

deplinenoise commented 7 years ago

Thanks for taking that on. Looking forward to your PR!

leidegre commented 7 years ago

See https://github.com/deplinenoise/tundra/pull/292 for further discussion

deplinenoise commented 6 years ago

I'm OK with auto-confing the install location of VS2017 as long as there is a way to override it. E.g. if I have VS2017 as part of source control and I want to point to that version this shouldn't prevent me.

leidegre commented 6 years ago

We should close this in favor of #292.