1359552 - Angle Build Broken on MinGW

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Tom Ritter [:tjr]]

In Bug 1314979, I turned off an Angle-related section of old-configure.in when we were compiling on Linux with MinGW for Windows. I did this thinking it was unneeded; and since the section errorred about the lack of a Windows SDK being present, that was the way to fix the error.

That was a mistake. We need to hit that section to set MOZ_HAS_WINSDK_WITH_D3D - if that define is not set we don't include d3d11.h which (surprise) is required for stuff.  

The mistake about why the patch worked or not was caused by some confusion about the state of my tree while waiting for Bug 1331335 to get reviewed.

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1359552 ANGLE requires MOZ_HAS_WINSDK_WITH_D3D to be set for required includes in MINGW

The first attempt at this patch in Bug 1314979 turned off the entire
ANGLE section for MINGW builds on Linux for Windows. This was
incorrect, as it also turned off includes of very important files
like d3d11.h

This changes it so we go through the ANGLE section, but if we don't find
a Windows SDK we only consider that an error when we're compiling with
MinGW for Windows _on_ Windows. If the Host and Target OSes don't match
(meaning we're compiling on Linux for Windows) we ignore the lack of a
Windows SDK.

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

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8861570 [details]
Bug 1359552 ANGLE requires MOZ_HAS_WINSDK_WITH_D3D to be set for required includes in MINGW

https://reviewboard.mozilla.org/r/133536/#review137074

glandium is on holiday. So stealing this review so it doesn't sit idle.

This change looks fine. Thank you for writing a good commit message.

You can ignore the indentation nit if you want: it shouldn't be relevant.

::: old-configure.in:3132
(Diff revision 1)
>  
>  if test "$COMPILE_ENVIRONMENT" ; then
>  case "$target_os" in
>  *mingw*)
> -    if test "$OS_ARCH" = "$HOST_OS_ARCH"; then
>        MOZ_ANGLE_RENDERER=1

This line should be unindented.

Comment 3

[Tom Ritter [:tjr]]

Comment on attachment 8861570 [details]
Bug 1359552 ANGLE requires MOZ_HAS_WINSDK_WITH_D3D to be set for required includes in MINGW

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

Comment 4

[Tom Ritter [:tjr]]

Fixed, thanks.

Comment 5

[Mozilla Autoland]

hg error in cmd: hg rebase -s 50fd9cf1d1fd -d 1219abce0035: abort: can't rebase public changeset 50fd9cf1d1fd
(see 'hg help phases' for details)

Comment 6

[Mozilla Autoland]

hg error in cmd: hg rebase -s 50fd9cf1d1fd -d c2b3ef7f8efe: abort: can't rebase public changeset 50fd9cf1d1fd
(see 'hg help phases' for details)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/21fb5fb40df5
ANGLE requires MOZ_HAS_WINSDK_WITH_D3D to be set for required includes in MINGW r=gps

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Unsure what was wrong with autoland. I just pushed the patch to inbound instead.

Comment 9

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/21fb5fb40df5

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8861570 [details]
Bug 1359552 ANGLE requires MOZ_HAS_WINSDK_WITH_D3D to be set for required includes in MINGW

https://reviewboard.mozilla.org/r/133536/#review140410

::: old-configure.in:3164
(Diff revision 2)
>    fi
>  
>    ######################################
>    # Find _46+ for use by Vista+.
>  
>    # Find a D3D compiler DLL in a Windows SDK.

If you ignore the error, you might as well skip the entire test.

That being said, on reflection, it seems like we should still require the d3d compiler dll for those builds... except if the gfx team says otherwise.