1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl

Status: RESOLVED FIXED

(Firefox Build System :: Toolchains, enhancement)

Description

[Masatoshi Kimura [:emk]]

Apparently the latest Windows SDK fixed a bug of Fall Creator Update SDK.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

Review commit: https://reviewboard.mozilla.org/r/245096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245096/

Comment 2

[(Away)]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

https://reviewboard.mozilla.org/r/245096/#review251014

Please update https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Building_Firefox_on_Windows_with_clang-cl#Prerequisites when this lands.

::: build/moz.configure/windows.configure:240
(Diff revision 1)
>                                ' version can be installed using the Visual'
>                                ' Studio installer.'
>                                % (version, minimum_ucrt_version))
>  
>      broken_ucrt_version = Version('10.0.16299.0')
> -    if c_compiler.type == 'clang-cl' and version >= broken_ucrt_version:
> +    if c_compiler.type == 'clang-cl' and version == broken_ucrt_version:

Out of curiosity, what version number did you test?

Are we sure that any version >16299 will work?

::: build/moz.configure/windows.configure:243
(Diff revision 1)
>  
>      broken_ucrt_version = Version('10.0.16299.0')
> -    if c_compiler.type == 'clang-cl' and version >= broken_ucrt_version:
> +    if c_compiler.type == 'clang-cl' and version == broken_ucrt_version:
>          raise FatalCheckError('Found SDK version %s but clang-cl builds'
> -                              ' currently don\'t work with SDK version %s'
> -                              ' and later. You should use version %s,'
> +                              ' currently don\'t work with SDK version %s.'
> +                              ' You should use a different version, either'

I wonder if we should include a message here for people who previously didn't want to downgrade their SDK, since they can now upgrade it instead.

Maybe something like "Note: Version NNNNN now works with clang-cl."

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245096/diff/1-2/

Comment 4

[Masatoshi Kimura [:emk]]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

https://reviewboard.mozilla.org/r/245096/#review251014

> Out of curiosity, what version number did you test?
> 
> Are we sure that any version >16299 will work?

I confirmed it with 10.0.17134.0.

I changed this check to ">=16299 and <17134". Although I didn't test versions 16300 to 17133, I don't think we have to support preview versions of SDK.

About >17134, I do not have a crystal ball :) But usually we assume that any future SDK versions will work.

> I wonder if we should include a message here for people who previously didn't want to downgrade their SDK, since they can now upgrade it instead.
> 
> Maybe something like "Note: Version NNNNN now works with clang-cl."

Added a note as you proposed.

Comment 5

[Masatoshi Kimura [:emk]]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245096/diff/2-3/

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

https://reviewboard.mozilla.org/r/245096/#review251502

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c085e1b32fb9
Unblock Windows 10 April 2018 Update SDK with clang-cl. r=ted

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c085e1b32fb9

Comment 9

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Current tooling used to build central may fail to build esr60 on Windows using either msvc or clang. This is a hindrance to devs, who either need to rollback their tooling, or manually apply the fixes from bug 1484184, and bug 1462616 (this bug) for clang. Uplifting these patches smooths development of esr60 on Windows, and avoids devs having to manually discover these fixes and apply them. Please note, only uplifting this patch will not fix clang builds as bug 1484184 needs to be uplifted also.

User impact if declined: End users are not impacted. Devs will not be able build esr60 without finding and applying the above patches.

Fix Landed on Version: 62

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The change is small and to the build system. The change has baked since 62. I have verified that esr60 Windows builds work with clang following the application of this and the other mentioned patch.

String or UUID changes made by this patch: None

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8976949 [details]
Bug 1462616 - Unblock Windows 10 April 2018 Update SDK with clang-cl.

Makes life easier for devs compiling ESR60 with newer MSVC toolchains. Approved for 60.5.0esr.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/811ae8044ce4