1430638 - Update Windows SDK on builders to the April 2018 Update version (10.0.17134.0)

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Jonathan Kew [:jfkthame]]

From the failure logs in https://treeherder.mozilla.org/#/jobs?repo=try&revision=62ca33aa87216b1cba8429bb768fda2daeb8fdfa, it looks like our builders currently have SDK vers. 10.0.15063.0, which corresponds to Windows 10 Creators Update.

It would be helpful to update the SDK version to the Fall Creators Update (SDK vers. 10.0.16299.15) in order to access declarations of new types/interfaces.

Comment 1

[Gregory Szorc [:gps]]

dmajor was the last person to package the Visual Studio toolchain and he could potentially help you here (although a few of us have created these toolchains in the past, including me).

However, we had to degrade the SDK version in bug 1413675 to make clang builds work. So this may not be trivial.

Alternatively, we could perhaps use a separate, older SDK version for clang builds. But since we use clang for e.g. asan, varying the SDK version feels wrong.

dmajor appears away from the Bugzilla name field. froydnj: do you have any context to add?

Comment 2

[Nathan Froyd [:froydnj]]

I have no additional context.  It feels like making the MSVC Windows builds and the clang Windows builds use different SDKs would be asking for trouble down the line: if somebody uses types/interfaces only available on newer versions of the SDK, the clang builds are going to fall over and will not be fixable, AFAICT.

I don't know what a good solution here looks like short of copying the necessary declarations into our own code?  But then--presumably--functions that accept newer types wouldn't exist in the libraries we would link against in older SDKs, unless we dynamically looked up those symbols at runtime?

Comment 3

[Jonathan Kew [:jfkthame]]

The specific use-case that prompted this is the implementation of Variation Font support in bug 1430632, which depends on new DirectWrite interfaces introduced in the Fall Creators Update; see the second patch in https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcac12062637ac8d453c558a905560627f3908c1 for an example of the necessary declarations that we'd need to copy/paste into our code somewhere.

Being new versions of COM interfaces, these are handled at runtime by QI-ing to see if they're available; but the declarations need to be visible to the compiler. Are we OK with copying a chunk of declarations from the new SDK, as per the scratch dw-extra patch in that try job? (FWIW, I've just requested Win64 ASAN builds on that push, to see what clang thinks of it.)

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Jonathan Kew (:jfkthame) from comment #3)
> Being new versions of COM interfaces, these are handled at runtime by QI-ing
> to see if they're available; but the declarations need to be visible to the
> compiler. Are we OK with copying a chunk of declarations from the new SDK,
> as per the scratch dw-extra patch in that try job? (FWIW, I've just
> requested Win64 ASAN builds on that push, to see what clang thinks of it.)

clang-cl seems pretty happy about those patches!

Once we copy-and-paste those declarations, what happens for people with newer SDKs?  The patches will compile fine on automation using an older SDK, but will people using a newer SDK run into errors about multiply-defining enums/interfaces/etc.?  That doesn't seem nice.  Is it possible to gate including those definitions on the SDK version in use?  I don't think we've ever imposed a maximum SDK version (which we could do, to some complaining).

Comment 5

[Jonathan Kew [:jfkthame]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> (In reply to Jonathan Kew (:jfkthame) from comment #3)
> > Being new versions of COM interfaces, these are handled at runtime by QI-ing
> > to see if they're available; but the declarations need to be visible to the
> > compiler. Are we OK with copying a chunk of declarations from the new SDK,
> > as per the scratch dw-extra patch in that try job? (FWIW, I've just
> > requested Win64 ASAN builds on that push, to see what clang thinks of it.)
> 
> clang-cl seems pretty happy about those patches!
> 
> Once we copy-and-paste those declarations, what happens for people with
> newer SDKs?  The patches will compile fine on automation using an older SDK,
> but will people using a newer SDK run into errors about multiply-defining
> enums/interfaces/etc.?  That doesn't seem nice.  Is it possible to gate
> including those definitions on the SDK version in use?

Yes, we should be able to add some #if-checks to do that; the patch in that try push was just my quick-'n'-dirty test to see if it would work at all.

Can we assume there's no licensing problem with copy-pasting declarations like this from a new SDK?

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)
> > Once we copy-and-paste those declarations, what happens for people with
> > newer SDKs?  The patches will compile fine on automation using an older SDK,
> > but will people using a newer SDK run into errors about multiply-defining
> > enums/interfaces/etc.?  That doesn't seem nice.  Is it possible to gate
> > including those definitions on the SDK version in use?
> 
> Yes, we should be able to add some #if-checks to do that; the patch in that
> try push was just my quick-'n'-dirty test to see if it would work at all.

Actually, as things stand that's not even necessary, because our WINVER setting in configure causes the new SDK to not expose its new declarations anyway. So it would only become an issue once we bump WINVER (which bug 1430634 says we're not doing at this time).

Comment 7

[Gregory Szorc [:gps]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Can we assume there's no licensing problem with copy-pasting declarations
> like this from a new SDK?

When it comes to legal matters, never assume.

It's best to file a bug in the Legal bug product (General bug component??). Point them at the license of the Windows SDK. You may have to upload the sdk_license.rtf from an SDK install. Should be in Program Files (x86)\Windows Kits\10\Licenses\<version>\. I'm sure build peers would appreciate a CC so we have access to the discussion.

Comment 8

[Masatoshi Kimura [:emk]]

Fall Creators Update SDK will never work with clang-cl.

Comment 9

[Masatoshi Kimura [:emk]]

Fixed by bug 1485224.