added a 'latest' systemversion for vs2017 #994
Conversation
forgot to add the ".0" suffix added unit tests for systemversion "latest"
|
|
||
| function suite.windowsTargetPlatformVersionLatest_on2015() | ||
| p.action.set("vs2015") | ||
| systemversion "10.0.10240.0" |
There was a problem hiding this comment.
Is this supposed to be latest?
There was a problem hiding this comment.
Ah ! You're totally right ! Is there a way to run specific tests ? (Couldn't run the whole test suite because of some other tests failing before this one)
|
I'm with @samsinsane here... |
There was a problem hiding this comment.
Hi, like I said in the first comment, it's not this test which fails, it's test_http.lua. So I couldn't run test_globals.lua, thus the dumb copy/paste error. But anyway it should be fixed now, thanks :)
Edit: seeing the result of appveyor, it seems the problem with test_http is only on my setup, and there's a problem with the test for "latest" and vs2017. I'll get back to it, sorry for the inconvenience
|
|
||
| function suite.windowsTargetPlatformVersionLatest_on2015() | ||
| p.action.set("vs2015") | ||
| systemversion "10.0.10240.0" |
There was a problem hiding this comment.
Ah ! You're totally right ! Is there a way to run specific tests ? (Couldn't run the whole test suite because of some other tests failing before this one)
|
Yeah, I ran into similar issues with some of the http test failures... it's kind of why I added the --insecure option.... I run my tests locally with that option, so the ssl tests pass... it's often because of some firewall stuff in corporate organizations... |
|
Ok I could run the tests, and I have a problem: I need to override |
| function suite.windowsTargetPlatformVersionLatest_on2017() | ||
| p.action.set("vs2017") | ||
| systemversion "latest" | ||
| prepare() |
There was a problem hiding this comment.
prepare is what will call os.getWindowsRegistry, so you'll need to put this just above the test.capture
Woops, sorry about that, I completely forgot that you mentioned the tests failing in your original comment. I was referring to the failed builds with appveyor and travis, in my head it made sense at the time, sorry for the confusion!
I've added a comment for this, that should fix it. |
|
@samsinsane thanks, that was it ! some other tests still fail on my machine, but at least appveyor seems to be happy now :) |
modules/vstudio/vs2010_vcxproj.lua
Outdated
| function m.latestSDK10Version() | ||
| local arch = iif(os.is64bit(), "\\WOW6432Node\\", "\\") | ||
| local version = os.getWindowsRegistry("HKLM:SOFTWARE" .. arch .."Microsoft\\Microsoft SDKs\\Windows\\v10.0\\ProductVersion") | ||
| return iif(version ~= nil, version .. ".0", nil) |
There was a problem hiding this comment.
This took me a little bit to figure out but these tests fail on Linux and macOS because version .. ".0" is being evaluated here. To resolve this issue, you'll need to break this out into a proper if statement.
|
@dcourtois No worries, glad it sorted it out, just noticed another issue when I checked the Travis builds. I've added a comment for that, hopefully that's the last thing! I'm sure you're just as keen to merge this as I am at this point. 😄 Thanks for doing this! |
|
My worst PR ever, thanks for the help :D Fingers crossed ... |
|
Everything looks good now! We'll merge this when the mac build finally runs - seems like it's time for TravisCI to buy some more macs haha. |
|
That could be days ;) Their mac builds are so overloaded all the time, it often takes 4-5 hours for that queue to clear... I wouldn't wait... if linux is passing, bsd/mac will pass too on this one, there is no difference in codepath for this particular test between the two... |
|
@tvandijck you raise some good points. Merging. |
|
Now I can suggest to update the Premake and modules' scripts to use |
Sorry for the delay, but here is the pull request for issue #935 and the other related ones.
You can now specify
systemversion "latest". If the action isvs2017, it will query the registry to retrieve the latest SDK 10 version number. If anything's wrong (action less than vs2017, the registry does not exist, etc.) no system version will be written in the vcxproj.There are 2 basic unit tests, but I couldn't run them, tests currently fail on
test_http.luaon my setup...I tested this on my personal project, and also to build Premake itself using vs2017, and it works fine (just needed to add
systemversion "latest"on the various Premake scripts for the Windows config.