The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 5.0b1

Status

MailNews Core
Build Config
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 5.0b1
x86
Windows 7
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking-thunderbird5.0 needed, blocking-seamonkey2.1 final+)

Details

(Whiteboard: [needed for platform parity])

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
+++ 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.
(Assignee)

Updated

6 years ago
blocking-thunderbird5.0: --- → needed
Whiteboard: [needed for platform parity]
Version: unspecified → Trunk

Updated

6 years ago
blocking-seamonkey2.1: --- → final+
Should we file blocking bugs for SM and TB to install DirectX SDK?
(Assignee)

Comment 2

6 years ago
(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).
(In reply to comment #2)
> The two are independent (FTR bug 632147 is the TB one).

I filed bug 632325 for SM.
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.
(Assignee)

Comment 5

6 years ago
(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

6 years ago
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.
I think we want to move the DirectX SDK logic into MozillaBuild anyways.
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → bugzilla
(Assignee)

Comment 9

6 years ago
Created attachment 519379 [details] [diff] [review]
The fix
[Checked in: See comment 14]

This does the necessary porting and additions.
Attachment #519379 - Flags: review?(bugspam.Callek)
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.

Updated

6 years ago
Depends on: 632325
(Assignee)

Comment 11

6 years ago
FTR bug 632325 doesn't actually block this - the patch should be able to land without DirectX SDK being installed on the boxes.
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.
Attachment #519379 - Flags: review?(bugspam.Callek) → review+
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.
(Assignee)

Comment 14

6 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
Created attachment 520435 [details] [diff] [review]
(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?
Attachment #520435 - Flags: review?(bugspam.Callek)
(Assignee)

Comment 16

6 years ago
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.
(Assignee)

Comment 17

6 years ago
and thanks for catching the other part, guess I missed that in the copy n paste.
(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 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.
Attachment #520435 - Flags: review?(bugspam.Callek) → review+
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).
Attachment #520435 - Attachment description: (Bv1) Fix nit in configure.in, Add missed suite/installer/Makefile.in part → (Bv1) Fix nit in configure.in, Add missed suite/installer/Makefile.in part [Checked in: See comment 20]
Attachment #519379 - Attachment description: The fix → The fix [Checked in: See comment 14]
V.Fixed, SeaMonkey part, per bug 632325 comment 9.
Depends on: 603367
You need to log in before you can comment on or make changes to this bug.