1114577 - Drop support for Windows SDK versions <8.1

Status: RESOLVED FIXED

Description

[Ryan VanderMeulen [:RyanVM]]

MSVC2012 ships with version 8.0, which is now our minimum supported compiler. We should update the Windows SDK requirements accordingly.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Once bug 1117820 lands, 8.1 is the only release we'll need to support (it comes with MSVC2013). It would be good if we could get this done prior to the next uplift as well.

Comment 2

[:Ehsan Akhgari]

I'm not very familiar with how this should be done, it's best for someone else to take this.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Looks like jimm was the last person to play around with this.
http://hg.mozilla.org/mozilla-central/file/912036eeb024/configure.in#l415

Comment 4

[Ryan VanderMeulen [:RyanVM]]

I'm going to take a stab at this.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Created attachment 8544879 [details] [diff] [review]
Set SDK version 603 as the minimum supported

Looks like this works. With my usual MSVC2012 + WinSDK 8.1 environment, the build succeeds. When I force my local environment to useSDK8 instead of 8.1, configure errors out as expected.

https://tbpl.mozilla.org/?tree=Try&rev=582fabc8fb21 for sanity.

And I intend to create a follow-up patch for other miscellaneous clean-ups.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8544879 [details] [diff] [review]
Set SDK version 603 as the minimum supported

Review of attachment 8544879 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +437,4 @@
>  MOZ_ARG_WITH_STRING(windows-version,
>  [  --with-windows-version=WINSDK_TARGETVER
> +                          Windows SDK version to target. Win8.1 (603) is
> +                          the only version currently allowed.],

Please rephrase such that we don't need to update (or risk to forget to update) this sentence when a new version appears.

@@ +693,5 @@
>          # strsafe.h on mingw uses macros for function deprecation that pollutes namespace
>          # causing problems with local implementations with the same name.
>          AC_DEFINE(STRSAFE_NO_DEPRECATE)
>  
> +        MOZ_WINSDK_MAXVER=0x06030000

I'm not sure the tests that use this variable will be happy on mingw builds with that value. Jacek?

Note there are many places in configure that test this value, and if it is never going to be below 0x06030000, there are branches of those tests that can be removed as well.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

(In reply to Mike Hommey [:glandium] from comment #6)
> I'm not sure the tests that use this variable will be happy on mingw builds
> with that value. Jacek?

In that case, wouldn't a mingw build die during the version check earlier in configure?

Comment 8

[Mike Hommey [:glandium]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #7)
> (In reply to Mike Hommey [:glandium] from comment #6)
> > I'm not sure the tests that use this variable will be happy on mingw builds
> > with that value. Jacek?
> 
> In that case, wouldn't a mingw build die during the version check earlier in
> configure?

which one?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Ah, I see what you mean now. Yeah, I can see the logic in dropping those changes then (though I do wonder if js/src/configure.in should at least be in sync).

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Created attachment 8545017 [details] [diff] [review]
GFX cleanups

Mike, can you please take a look at this from a build system standpoint for sanity? And Jeff, can you make sure that gfx changes look OK?

One interesting thing to note: AFAICT, the check in gfxDWriteFontList.cpp appears to have been broken before, so this may in fact change compilation slightly.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Created attachment 8545018 [details] [diff] [review]
build system cleanups

winsdk.m4 is much simpler now :)

I've confirmed that these build locally and on Try.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Created attachment 8545020 [details] [diff] [review]
Set SDK version 603 as the minimum supported

Mike mentioned on IRC that in the long run, we may want to move the mingw max version setting into winsdk.m4 as well. That seems like a sensible idea, but more than I want to do right now. But Jacek, I'm happy to remove those two hunks from the first patch if you want (or change the version in js/src to at least match the other).

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8545017 [details] [diff] [review]
GFX cleanups

Review of attachment 8545017 [details] [diff] [review]:
-----------------------------------------------------------------

gfxDwriteFontList.cpp looks fine.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8545017 [details] [diff] [review]
GFX cleanups

Review of attachment 8545017 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/moz.build
@@ +63,3 @@
>          'SourceSurfaceD2DTarget.cpp',
>      ]
> +    DEFINES['USE_D2D1_1'] = True

Might as well remove the define and remove the ifdefs in the code.

Comment 15

[Jacek Caban]

Thanks for the notice. I tested those patches. A small change for a missing enum values is needed in mingw-w64 itself, but I will take care of that.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/98162547b084
https://hg.mozilla.org/integration/mozilla-inbound/rev/84708ff7c9fe
https://hg.mozilla.org/integration/mozilla-inbound/rev/6399ee520bcc

I did a quick scan of https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Windows_Prerequisites and didn't see anything obvious that needs updating with this change. Arguably, https://developer.mozilla.org/En/Windows_SDK_versions could use some updating, but it should probably wait until all supported releases are on MSVC2013+ (i.e. when esr31 is EOL).

Comment 17

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/98162547b084
https://hg.mozilla.org/mozilla-central/rev/84708ff7c9fe
https://hg.mozilla.org/mozilla-central/rev/6399ee520bcc

Comment 18

[Massimo Gervasini [:massimo]]

Created attachment 8546118 [details] [diff] [review]
Bug 1114577 - Drop support for Windows SDK versions <8.1 - l10n-mozconfig.patch

I think we need to set ac_add_options --with-windows-version=603 in win32/l10n-mozconfig too

Comment 19

[Massimo Gervasini [:massimo]]

Created attachment 8546488 [details] [diff] [review]
Bug 1114577 - Drop support for Windows SDK versions <8.1 - l10n-mozconfig.patch

re-generated the l10n-mozconfig patch (r+ by mshal) with the right headers.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6efc910604a2

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6efc910604a2