471854 - Build error in accessible/public/msaa on x64 Windows with VC8

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Masatoshi Kimura [:emk]]

You will fail to build Firefox with the following message:
   Creating library AccessibleMarshal.lib and object AccessibleMarshal.exp
dlldata.obj : error LNK2001: unresolved external symbol _ISimpleDOMText_ProxyFileInfo
dlldata.obj : error LNK2001: unresolved external symbol _ISimpleDOMNode_ProxyFileInfo
dlldata.obj : error LNK2001: unresolved external symbol _ISimpleDOMDocument_ProxyFileInfo
AccessibleMarshal.dll : fatal error LNK1120: 3 unresolved externals

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: patch

I also removed old workaround. (see bug 471451.)

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 355095 [details] [diff] [review]
patch

Could you import timeless' patch from bug 471451 and make your patch apply on top of that? As written your patch will still trip his bug.

Comment 3

[(not currently active) Ted Mielczarek]

Also, can you check with Makoto about this? He has a bug on making the Windows x64 build work, and your "-env win32" switch might be a problem for him.

Comment 4

[Masatoshi Kimura [:emk]]

Attachment: Integrate bug 471451 patch

I removed midl stuff from js/src/configure.in completely per bug 471451 comment #3.
Kato-san, do I have to check for x86_64 target?

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 355134 [details] [diff] [review]
Integrate bug 471451 patch

You should add it when target is i*86-* only.

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Added platform check

I found that it was not required to check whether string is empty because |=| will perform a string compare rather than an arithmetic compare.

Comment 7

[Makoto Kato [:m_kato]]

Kimura-san, can we remove no_robut options?  (anyone may use old version of MIDL)

And I believe MIDL version 7 (or maybe from VC7.1?) supports /env win32 options, so if MIDL is version 7, we must always add it, I think.

Comment 8

[Masatoshi Kimura [:emk]]

(In reply to comment #7)
> Kimura-san, can we remove no_robut options?  (anyone may use old version of
> MIDL)
See bug 378928. (I thought I referred it in comment #1. The bug number was wrong...)
Although older midl will generate no_robust proxy/stub, it is harmless.

> And I believe MIDL version 7 (or maybe from VC7.1?) supports /env win32
> options, so if MIDL is version 7, we must always add it, I think.
I can build Firefox using VC9 without /env win32 switch.
I believe VC8 has a bug because midl should generate x86 compatible proxy/stub if the build environment is configured for x86 target. Therefore I wrote a patch as a workaround for specific midl version.

Comment 9

[Makoto Kato [:m_kato]]

Comment on attachment 355463 [details] [diff] [review]
Added platform check

Agree

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: sync to trunk

Thank you, Kato-san.
Asking build-config owner for review again.
The patch becomes obsolete since the fix of bug 471451 landed. I have updated a patch.

Comment 11

[Serge Gautherie (:sgautherie)]

Do the 4 other lines in /js/src/configure.in need to be kept ?

Comm-central will want to stay in sync, I guess.

Comment 12

[Masatoshi Kimura [:emk]]

Attachment: Removed from js/src/configure.in completely

> Do the 4 other lines in /js/src/configure.in need to be kept ?
Removed.
> Comm-central will want to stay in sync, I guess.
Plaease file a separate bug for comm-central.

Comment 13

[(not currently active) Ted Mielczarek]

Comment on attachment 355989 [details] [diff] [review]
Removed from js/src/configure.in completely

+            if test \( "$_MIDL_MAJOR_VERSION" = "7" -a "$_MIDL_MINOR_VERSION" = "00" -a "$_MIDL_REV_VERSION" = "0499" \); then

I trust that this line won't regress timeless' bug if midl is not found? (It looks like you're only doing string comparisions, so I hope so.)

Can you also remove MIDL/MIDL_FLAGS from js/src/config/autoconf.mk.in?

Comment 14

[Masatoshi Kimura [:emk]]

Attachment: Removed midl stuff from autoconf.mk.in [Checkin: Comment 15]

Carrying forward r+.
(In reply to comment #13)
> I trust that this line won't regress timeless' bug if midl is not found? (It
> looks like you're only doing string comparisions, so I hope so.)
Honestly, I have no idea how to make a midl-less build environment.
Both VC8 and VC9 are shipped with midl. Windows SDK also has. MinGW build environment won't even execute the codepath in question. I doubt we need to support a midl-less MSVC environment in the first place.
Having said that, my patch should not regress the issue.
> Can you also remove MIDL/MIDL_FLAGS from js/src/config/autoconf.mk.in?
Done.

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 356727 [details] [diff] [review]
Removed midl stuff from autoconf.mk.in
[Checkin: Comment 15]


http://hg.mozilla.org/mozilla-central/rev/24a0ddf8cdfa

Comment 16

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Sync comm-central after (m-c) bug 471451 and bug 471854 [Checkin: Comment 19]

(untested) trivial copy.


(In reply to comment #12)
> > Comm-central will want to stay in sync, I guess.
> Plaease file a separate bug for comm-central.

It's easier to keep both in the same bug, I think.

Comment 17

[Robert Kaiser]

Comment on attachment 357968 [details] [diff] [review]
(Bv1) Sync comm-central after (m-c) bug 471451 and bug 471854
[Checkin: Comment 19]

I also would have liked a separate bug better, but here it goes. I can't test this, so r=me by looking at the code and not spotting anything that looks wrong to me. I trust what was reviewed for Mozilla should work for us as well.

Comment 18

[Masatoshi Kimura [:emk]]

Attachment: branch patch

I want to land this on the branch for comm-central.
The risk is very low.

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 357968 [details] [diff] [review]
(Bv1) Sync comm-central after (m-c) bug 471451 and bug 471854
[Checkin: Comment 19]


http://hg.mozilla.org/comm-central/rev/1a9b710f8ea6

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 358031 [details] [diff] [review]
branch patch

a191=beltzner

Comment 21

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/65317b1d28b1