Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable XP deprecation warning for Visual Studio projects #1439

Merged
merged 2 commits into from
May 1, 2020
Merged

Disable XP deprecation warning for Visual Studio projects #1439

merged 2 commits into from
May 1, 2020

Conversation

withmorten
Copy link
Contributor

Related issue: #1438

@withmorten
Copy link
Contributor Author

Looks like all the tests are failing (should have thought of that) - perhaps it at least should only be added to Windows projects (and I should maybe move it to the bottom of the globals list)?

@samsinsane
Copy link
Member

You should definitely ensure it only happens on Windows (os.target() == p.WINDOWS) but also you should ensure that it only happens for VS2019 (your issue mentions that it only started in VS2019), if this continues in VS2021 it can be updated to include that but we shouldn't have this happening when XP support is completely gone. Additionally, you should move the entry to the bottom of the group, you're going to break a lot of unit tests by putting it at the top. Once you've done this, I highly recommend you write unit tests to ensure that it doesn't happen in VS2017 or when system "android", and that it does happen in VS2019.

@withmorten
Copy link
Contributor Author

withmorten commented Apr 30, 2020

It also happens in VS2017 - it's a general issue with the toolset, because MS added the XPDeprecationWarning as true *in MSBuild\Microsoft\VC\v150\Platforms\Win32\PlatformToolsets\v141_xp\Toolset.targets, and I suspect it will happen in future versions *too (if _xp is even available there).

I don't have VS2017 installed right now, but I remember it from when I had and this stackoverflow *post seems to corroborate that: https://stackoverflow.com/questions/53841470/how-to-disable-warning-msb8051-support-for-targeting-windows-xp-is-deprecated

It doesn't however happen with VS2015, or when you select the v140_xp buildtools in VS2017 or VS2019 ...

Edit: Okay I solved it the following way. Instead of setting it globally for the entire project, I moved the function to the globalsCondition array, give it the cfg, and check if the toolset == msc-v141_xp.

Now I just need to have a look at the tests.

Edit2: Typos....

@samsinsane
Copy link
Member

@withmorten thanks for the additional context on that!

@withmorten
Copy link
Contributor Author

withmorten commented Apr 30, 2020

The final result then looks like this, instead of adding it to the Globals, it gets added to the Configuration specific parts:

  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Globals">
    <XPDeprecationWarning>false</XPDeprecationWarning>
  </PropertyGroup>
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'" Label="Globals">
    <XPDeprecationWarning>false</XPDeprecationWarning>
  </PropertyGroup>
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='ReleaseNO|Win32'" Label="Globals">
    <XPDeprecationWarning>false</XPDeprecationWarning>
  </PropertyGroup>`

That's okay? It disables the build warning just fine even though the placement in the vcxproj is a bit odd.

Tests are fine now too, but I probably would need to add a test for toolset == v141_xp, similar to the v120_xp test.

@samsinsane
Copy link
Member

That's okay?

Seems good to me 👍

@withmorten
Copy link
Contributor Author

Okay, now it only sets the warning to false when specifically choosing v141_xp, and only per config!

I hope the way I added the tests (one for global, one for specific config "Release" only) is okay!

@starkos starkos changed the title disable XP deprecation warning for visual studio projects Disable XP deprecation warning for Visual Studio projects May 1, 2020
@starkos starkos merged commit 09dbfca into premake:master May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants