699385 - (RequireWin7SDK) Remove support for pre-Windows 7 SDKs

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Rain [:rain1] (inactive)]

We either have to have MOZ_NTDDI_ checks or redefine enums all over the place. It'd be nice to get rid of everything. (Do we still care about mingw builds at all?)

This should probably be done once we move to MSVC 2010.

Comment 1

[Rain [:rain1] (inactive)]

Attachment: proposed patch

I think it'll be good to leave the infrastructure in so that we can use it if we need it again later.

Comment 2

[Rain [:rain1] (inactive)]

Attachment: proposed patch

sorry, detritus sneaked in.

Comment 3

[(not currently active) Ted Mielczarek]

Yeah, we'll probably wind up using this again in the future. I'm fine with making Windows 7 the minimum required SDK. And yes, Jacek is still actively maintaining a mingw port, so let's not go out of our way to break him.

Comment 4

[Jacek Caban]

Comment on attachment 571630 [details] [diff] [review]
proposed patch

Thanks for taking case of mingw port. I've fixed building with win7 sdk target on mingw a few moths ago on both mingw-w64 and Mozilla sides. A few Mozilla patches are still waiting for review, but that's usual for mingw build fixes, so as far as I'm concerned, the patch is fine.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 571630 [details] [diff] [review]
proposed patch

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

Might as well wait to land this till after the Aurora migration just to give everyone an extra cycle to deal with it.

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 571630 [details] [diff] [review]
proposed patch

Sorry for jumping on this, but I feel I must.

MOZ_WINSDK_TARGETVER

We should cleanup these tests in the tree, (followup ok) but the ones in configure.in (and maybe js/src/configure.in) should be cleaned up here imo.

In hunk 1 in this diff we have a | if test "$MOZ_WINSDK_TARGETVER" -lt "06000000"; then| already, which is currently invalid anyway.

It also might obsolete other config vars we can drop entirely from the tree.

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to Justin Wood (:Callek) from comment #6)

> the ones in
> configure.in (and maybe js/src/configure.in) should be cleaned up here imo.

If not in this patch, at least in a second one in this bug ;-)
(Yes, JS is usually updated in sync', unless it wouldn't be wanted here.)

> It also might obsolete other config vars we can drop entirely from the tree.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to Ted Mielczarek [:ted, :luser] from comment #5)
> Might as well wait to land this till after the Aurora migration just to give
> everyone an extra cycle to deal with it.

1 or 2 "Aurora migration" must have happened since then.
Can this land now, per bug 629827 comment 4?

Comment 9

[Rain [:rain1] (inactive)]

(In reply to Serge Gautherie (:sgautherie) from comment #8)
> (In reply to Ted Mielczarek [:ted, :luser] from comment #5)
> > Might as well wait to land this till after the Aurora migration just to give
> > everyone an extra cycle to deal with it.
> 
> 1 or 2 "Aurora migration" must have happened since then.
> Can this land now, per bug 629827 comment 4?

No, this shouldn't land before we switch to MSVC10.

Comment 10

[(not currently active) Ted Mielczarek]

Any particular reason? I don't think they have to be tied together, do they?

Comment 11

[Rain [:rain1] (inactive)]

(In reply to Ted Mielczarek [:ted, :luser] from comment #10)
> Any particular reason? I don't think they have to be tied together, do they?

Well, Callek mentioned that the Windows 7 SDK doesn't work with MSVC8's express edition, so people trying to build with the free version of the compiler we use for official builds are going to be screwed in the meanwhile. On the other hand, I think the SDK is as big a difference as the compiler, so people haven't been able to get official-like builds with free tools for a while...

Comment 12

[(not currently active) Ted Mielczarek]

Yeah, I'm not particularly worried about that. I think it's a pain in the ass to even find VC8EE for download now anyway. Most people that want to do builds with free tools are probably getting VC10EE now.

Comment 13

[Justin Wood (:Callek)]

(In reply to Ted Mielczarek [:ted, :luser] from comment #12)
> Yeah, I'm not particularly worried about that. I think it's a pain in the
> ass to even find VC8EE for download now anyway. Most people that want to do
> builds with free tools are probably getting VC10EE now.

I'm a firm believer that as long as our official builds are MSVC8 we should support MSVC8 EE.

A binary extension built with MSVC9EE won't work with MSVC8 Pro binaries (c.f. Lightning, for example -- I know building locally with MSVC9 won't let lightning run on official builds, but works fine on a local build of lightning)

Just because its hard does not mean its impossible, and I explicitly took MSVC8EE on my Win7 machine because that is our officially supported version, being unable to build with the used SDK on the officially supported [and currently used] free version of MSVC compiler is a huge burden with no real benefit to this landing earlier.

Comment 14

[Rain [:rain1] (inactive)]

> being unable to build with the used SDK on the officially supported [and
> currently used] free version of MSVC compiler is a huge burden with no real
> benefit to this landing earlier.

I don't really follow this -- this is how it is right now, since VC8 EE doesn't work with the Windows 7 SDK.

Anyway, this patch is inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/3950aa85276d

Comment 15

[Jim Mathies [:jimm]]

So we are landing this without msvc10 support? If so it shouldn't block that bug, and the white board can be cleared.

Comment 16

[Jim Mathies [:jimm]]

(eh, "depend on" that bug I mean)

Comment 17

[Rain [:rain1] (inactive)]

(In reply to Jim Mathies [:jimm] from comment #15)
> So we are landing this without msvc10 support?

Yeah.

Comment 18

[Rain [:rain1] (inactive)]

(Sorry, forgot to mention that I did a bit of additional cleanup in the patch I checked in. All of it's r=ted over irc.)

Comment 21

[Rain [:rain1] (inactive)]

Followup patch removing a bit from js/src/configure.in I forgot to tackle the first time round: http://hg.mozilla.org/integration/mozilla-inbound/rev/0eef9179ab0d

Comment 22

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/3950aa85276d
https://hg.mozilla.org/mozilla-central/rev/0eef9179ab0d

Comment 23

[Eric Shepherd [:sheppy]]

This page was already updated:

https://developer.mozilla.org/En/Windows_SDK_versions

Mentioned on Firefox 12 for developers.