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)
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: bobowen, Assigned: bobowen)
References
Details
Attachments
(1 file)
1.26 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 2•7 years ago
|
||
gps - do you know the answer to the question I posed to ted in the comment 0?
Flags: needinfo?(gps)
Comment 3•7 years ago
|
||
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)
Comment 4•7 years ago
|
||
> It should be installed with VS2015
It's not, by default, that's the problem.
Comment 5•7 years ago
|
||
Also, there are fishy things about what SDKs are registered in the registry. See bug 1364137.
Comment 6•7 years ago
|
||
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.
Comment 7•7 years ago
|
||
OK, bug 1364137 looks like a legit blocker to this. I thought this was a solved problem. Ugh.
Depends on: 1364137
Comment 8•7 years ago
|
||
See also: bug 1356493
Assignee | ||
Comment 9•7 years ago
|
||
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)
Comment 10•7 years ago
|
||
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)
Comment 11•7 years ago
|
||
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.
Assignee | ||
Comment 12•7 years ago
|
||
Thanks both, blocking it is then. I should have what is hopefully a satisfactory solution up on that bug soon.
Assignee | ||
Comment 13•7 years ago
|
||
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)
Assignee | ||
Comment 15•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c758d82fa76fd1f5ec21daee4b48d8b2f824989e
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 16•7 years ago
|
||
Attachment #8906564 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Comment 17•7 years ago
|
||
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+
Comment 18•7 years ago
|
||
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
Comment 19•7 years ago
|
||
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
Comment 20•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0ddab9b0527d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 21•7 years ago
|
||
(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.
Comment 22•7 years ago
|
||
(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.
Comment 23•7 years ago
|
||
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
Comment 24•7 years ago
|
||
(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)
Comment 25•7 years ago
|
||
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)
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•