Closed Bug 1380609 Opened 7 years ago Closed 7 years ago

Make Win10 SDK (minimum v10.0.10586.0) required for building Firefox

Categories

(Firefox Build System :: General, enhancement)

All
Windows
enhancement
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

References

Details

Attachments

(1 file)

Some parts of the chromium sandbox require the Win10 SDK v10.0.10586.0 or later, so it would be good if we could move to making that our minimum required version.

I had to #if out this part of the chromium sandbox code last time I updated.

ted - I think it might have been some of your changes that meant we started to pick up the Win10 SDK correctly (which I think was blocking this before).
Do you think we can make Win10 SDK required now?
gps - do you know the answer to the question I posed to ted in the comment 0?
Flags: needinfo?(gps)
The current code looks for the SDK in HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows Kits\Installed Roots\KitsRoot* registry keys. My machine has a Windows 10 SDK registered there and configure does pick it up. So I think all we need for this is a patch.

I do support the change to require the Windows 10 SDK. It should be installed with VS2015, which we require to build (we require vs2015u3 or newer).

The official builds are currently using 10.0.14393. If we're incurring a version bump, I wouldn't be opposed to requiring 10.0.14393 locally. This SDK version corresponds to the release made with Windows 10 Anniversary Update and it can be installed via the VS2015 installer. A few people might need to install it. But if we can converge local builds with automation, that's a win worth pursuing IMO.
Flags: needinfo?(gps)
> It should be installed with VS2015

It's not, by default, that's the problem.
Also, there are fishy things about what SDKs are registered in the registry. See bug 1364137.
But the VS2015 installer gives you an option to install this version of the SDK. If you go to Add/Remove Programs and hit "modify" for Visual Studio 2015, you can install it with just a few clicks. A one-time annoyance, sure. But if it converges toolchain configs and makes it easier to develop for Windows by limiting compatibility requirements, it should be a net win.
OK, bug 1364137 looks like a legit blocker to this. I thought this was a solved problem. Ugh.
Depends on: 1364137
Bug 1364137, is about issues with using 64-bit python.
While a related issue, given that 64-bit python isn't currently part of our default build tools (as far as I'm aware), should this really block this bug?

I've just posted a patch for bug 1356493, which hopefully means we can remove the work-around on the build instructions.
Flags: needinfo?(gps)
We'll be shipping a 64-bit Python with the next release of MozillaBuild. People are actively using the pre-release. So I think it is still a necessary dependency.
Flags: needinfo?(gps)
Also, the information in bug 1364137 is not entirely about 64-bits python. I pointed there for a reason, and that wasn't 64-bits specifics, but the fact what the registry contains about SDKs present is fishy.
Thanks both, blocking it is then.

I should have what is hopefully a satisfactory solution up on that bug soon.
I got side-tracking with PTO and other things.

With bug 1356493 and bug 1364137 now fixed, do we think we can make Win10 SDK the minimum now?

If we can I'll update the "Building Firefox for Windows" instructions.
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(gps)
I don't see any blockers to making this change.
Flags: needinfo?(gps)
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Comment on attachment 8906564 [details] [diff] [review]
Make Windows Universal CRT SDK version 10.0.10586.0 the minimum

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

I guess, let's see how many complaints we get from people who can't build after this.

::: build/moz.configure/windows.configure
@@ +222,5 @@
>                                'Please install it.')
>  
>      version, sdk = sdks[valid_sdks[0]]
> +    minimum_ucrt_version = Version('10.0.10586.0')
> +    if version < minimum_ucrt_version:

version is a Version, so you can actually just compare to a string directly, like in the test further down that does `if c_compiler.version < '19.10':`
Attachment #8906564 - Flags: review?(mh+mozilla) → review+
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ddab9b0527d
Make Windows Universal CRT SDK version 10.0.10586.0 the minimum. r=glandium
just a note to clarify that taskcluster windows builders (gecko-[123]-b-win2012 worker types) are using version 10586 (and have been since Oct 5th, 2016).

https://github.com/mozilla-releng/OpenCloudConfig/search?utf8=%E2%9C%93&q=https%3A%2F%2Fgo.microsoft.com%2Ffwlink%2Fp%2F%3FLinkID%3D698771

the installer used originates from https://go.microsoft.com/fwlink/p/?LinkID=698771 which corresponds to "Windows 10 SDK (10586)" according to the version archive at: https://developer.microsoft.com/en-us/windows/downloads/sdk-archive
https://hg.mozilla.org/mozilla-central/rev/0ddab9b0527d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(In reply to Rob Thijssen (:grenade - UTC+3) from comment #19)
> just a note to clarify that taskcluster windows builders
> (gecko-[123]-b-win2012 worker types) are using version 10586 (and have been
> since Oct 5th, 2016).

The build system downloads an archive containing Visual Studio and the Windows SDK from tooltool. So I don't think we need to install the Windows SDK on the workers themselves - at least not for Firefox builds.
(In reply to Gregory Szorc [:gps] from comment #21)
> The build system downloads an archive containing Visual Studio and the
> Windows SDK from tooltool. So I don't think we need to install the Windows
> SDK on the workers themselves - at least not for Firefox builds.

removed WindowsSDK on gecko-1-b-win2012-beta. will run some try builds on this worker type today.
https://github.com/mozilla-releng/OpenCloudConfig/commit/bae0c1c78410197cb64a2e9e122b37e6e515255e
if/when builds succeed will promote to gecko-[123]-b-win2012.
builds using workertype gecko-1-b-win2012-beta (with Windows 10 SDK removed) appear to fail. it seems our taskcluster windows builds either depend on the Windows SDK being installed on the system, or there is some missing environment configuration that would point the build at the SDK components included from tooltool.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96171acfa7265312e7be5878ee12139f0dddcbf6&group_state=expanded
(In reply to Rob Thijssen (:grenade - UTC+3) from comment #23)
> builds using workertype gecko-1-b-win2012-beta (with Windows 10 SDK removed)
> appear to fail. it seems our taskcluster windows builds either depend on the
> Windows SDK being installed on the system, or there is some missing
> environment configuration that would point the build at the SDK components
> included from tooltool.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=96171acfa7265312e7be5878ee12139f0dddcbf6&group_state=e
> xpanded

See http://logs.glob.uno/?c=mozilla%23build&s=15+Sep+2017&e=15+Sep+2017#c58102

I've backed out the patch to remove the system installation of Windows SDK (comment 22) since it looks like the firefox builds do need it.
Flags: needinfo?(gps)
Bug 1400354 has been filed to track adding the removed file via tooltool. We can re-land the removal of the Windows 10 SDK once the eventual change in that bug is deployed everywhere.
Flags: needinfo?(gps)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.