632148 - Port bug 630628 - Extract d3dx9_??.dll and d3dcompiler_??.dll from DirectX SDK at build time, and ship them with the build

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Mark Banner (:standard8)]

+++ This bug was initially created as a clone of Bug #630628 +++

WebGL on Windows relies on using ANGLE for rendering, which relies on a DLL, namely d3dx9_42.dll, to be present. It is part of the Microsoft DirectX SDK runtime, which we don't want to have to ask users to install by themselves. Therefore we want to ship this file with the build. This patch extracts it from the DirectX SDK at build time, and copies it into dist/bin.

Comment 1

[Serge Gautherie (:sgautherie)]

Should we file blocking bugs for SM and TB to install DirectX SDK?

Comment 2

[Mark Banner (:standard8)]

(In reply to comment #1)
> Should we file blocking bugs for SM and TB to install DirectX SDK?

No, they don't block this bug. The two are independent (FTR bug 632147 is the TB one).

Comment 3

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> The two are independent (FTR bug 632147 is the TB one).

I filed bug 632325 for SM.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Is MozillaBuild installed on all build slaves?

If yes, probably the simplest solution is to implement the "let's add these bits of DirectX SDK to MozillaBuild and let's edit configure.in so it finds and uses that" idea. This is also very useful to keep Firefox/SeaMonkey easier to build on Windows for everybody.

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> Is MozillaBuild installed on all build slaves?
> 
> If yes, probably the simplest solution is to implement the "let's add these
> bits of DirectX SDK to MozillaBuild and let's edit configure.in so it finds and
> uses that" idea. This is also very useful to keep Firefox/SeaMonkey easier to
> build on Windows for everybody.

That's a possibility, but in Thunderbird land we generally prefer to keep our system the same as Firefox, when we diverge is when we hit problems.

Comment 6

[Robert Kaiser]

We're all trying to follow the Firefox build setups as closely as possible, to avoid problems that are specific to our setup - but integrating this into MozillaBuild and using that on FF machines as well as others sounds like a good idea, esp. as devs for all our products are building with MozillaBuild as well.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think we want to move the DirectX SDK logic into MozillaBuild anyways.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We can't ship any of the dxsdk bits in MozillaBuild.  We can, however, add any logic in finding it into mb, but I don't know that there's any point -- bjacob did a bunch of work to find it in the configure step.

Comment 9

[Mark Banner (:standard8)]

Attachment: The fix [Checked in: See comment 14]

This does the necessary porting and additions.

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 519379 [details] [diff] [review]
The fix
[Checked in: See comment 14]

>+case "$target_os" in
>+    *msvc*|*mks*|*cygwin*|*mingw*)

Nit: no need to add "cygwin" support anymore.

Comment 11

[Mark Banner (:standard8)]

FTR bug 632325 doesn't actually block this - the patch should be able to land without DirectX SDK being installed on the boxes.

Comment 12

[Justin Wood (:Callek)]

Comment on attachment 519379 [details] [diff] [review]
The fix
[Checked in: See comment 14]

>Directx package try: -b do -p linux,win32 -u all -t none

Be sure to remove try-checkin-commands from here for when you do land.

>diff --git a/configure.in b/configure.in
>+MOZ_D3DCOMPILER_DLL=
>+case "$target_os" in
>+    *msvc*|*mks*|*cygwin*|*mingw*)

Nit: |*mingw*)| only here (matches projects/build-system)


And yes, I know this bug wasn't necessarily blocked by the install-DX-to-slaves bug for SeaMonkey, *but* I did set the dep, as this wouldn't actually do anything on the SeaMonkey slaves until I installed that.

Comment 13

[Justin Wood (:Callek)]

Comment on attachment 519379 [details] [diff] [review]
The fix
[Checked in: See comment 14]

I did not verify if TB utilizes the same about:licence as Firefox, but if not you need to port: https://bugzilla.mozilla.org/attachment.cgi?id=509196&action=diff as well.

Comment 14

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/a5aaf273af00

Also did a follow up for the nit: http://hg.mozilla.org/comm-central/rev/afd1d64f20a5

We get the license file from toolkit, so we don't need an additional bug.

Comment 15

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Fix nit in configure.in, Add missed suite/installer/Makefile.in part [Checked in: See comment 20]

Untested, but trivial copy&paste.

Should hopefully fix
{
make package-compare
...
+bin/D3DCompiler_42.dll
...
+bin/d3dx9_42.dll
}

Would you accept the removed-files.in counterpart?

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 520435 [details] [diff] [review]
(Bv1) Fix nit in configure.in, Add missed suite/installer/Makefile.in part
[Checked in: See comment 20]

> case "$target_os" in
>-*mingw*)
>-    MOZ_ANGLE=1
>-    ;;
>+    *mingw*)
>+        MOZ_ANGLE=1
>+        ;;

I purposely made the indentation match what mozilla-central has so that a diff between the two wouldn't show as different. I suggest if you want to change this that you file a separate bug.

> Would you accept the removed-files.in counterpart?

I thought about that and thought that it isn't necessary. We're not likely to be turning this on/off whilst shipping or updating people.

Comment 17

[Mark Banner (:standard8)]

and thanks for catching the other part, guess I missed that in the copy n paste.

Comment 18

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> I purposely made the indentation match what mozilla-central has so that a diff

Afaict, m-c is indented (as your attached patch was)...

Comment 19

[Justin Wood (:Callek)]

Comment on attachment 520435 [details] [diff] [review]
(Bv1) Fix nit in configure.in, Add missed suite/installer/Makefile.in part
[Checked in: See comment 20]

>diff --git a/configure.in b/configure.in

Please don't change this file, that section matches what is in build-system repo.

Comment 20

[Serge Gautherie (:sgautherie)]

Comment on attachment 520435 [details] [diff] [review]
(Bv1) Fix nit in configure.in, Add missed suite/installer/Makefile.in part
[Checked in: See comment 20]

http://hg.mozilla.org/comm-central/rev/2b95c4960211
Bv1, with comment 19 suggestion(s).

Comment 21

[Serge Gautherie (:sgautherie)]

V.Fixed, SeaMonkey part, per bug 632325 comment 9.