Undefined references to IID_* symbols on mingw.

RESOLVED FIXED in mozilla1.9.3a1

Status

defect
RESOLVED FIXED
10 years ago
Last year

People

(Reporter: jacek, Assigned: jacek)

Tracking

Trunk
mozilla1.9.3a1
x86
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

10 years ago
Posted patch fixSplinter Review
I get following errors when compiling on mingw:

netwerk/system/win32/nsNotifyAddrListener.cpp:459: undefined reference to `_IID_INetSharingManager'
netwerk/system/win32/nsNotifyAddrListener.cpp:459: undefined reference to `_CLSID_NetSharingManager'
netwerk/system/win32/nsNotifyAddrListener.cpp:478: undefined reference to `_IID_IEnumNetSharingPrivateConnection'
netwerk/system/win32/nsNotifyAddrListener.cpp:502: undefined reference to `_IID_INetConnection'

It's because uuid is not linked. Even if I work around it for netwerk module, I get another errors. It looks like the build system depends on uuid being linked by default and thus I've added it to LIBS in configure.in.
Assignee

Updated

10 years ago
Attachment #412892 - Attachment is patch: true
Attachment #412892 - Flags: review?(cls)

Updated

10 years ago
Attachment #412892 - Flags: review?(cls) → review-

Comment 1

10 years ago
Comment on attachment 412892 [details] [diff] [review]
fix

First, add a library to $LIBS isn't going to fix a compile issue.  It may fix the link issue but I'm not convinced from that single error that uuid needs to be linked into everything.  It should just be added to EXTRA_DSO_LDOPTS as needed.
Assignee

Comment 2

10 years ago
Thanks for review. I think I should add more info here. The reason why MSVC doesn't need this value added to libs is because it uses
#pragma comment(lib,"uuid.lib")
that many COM-related include files contain. MinGW doesn't handle these macros. That's why linking of all dlls and exes that use COM will fail. I could add uuid to every such build, but it would be unmaintainable. Every time a new dll/exe will use COM, compilation on mingw will be broken (because most devs don't test mingw builds) unless we'd disable handling of these pragmas on MSVC builds (I guess it's not acceptable?).

How about adding uuid to LIBS only for mingw builds? Would it be acceptable?

Comment 3

10 years ago
You're assuming that all COM users will use that pragma.  That hasn't been the case so far or else uuid would have already been added everywhere.  In fact, I don't see that pragma at all with a quick grep of a mozilla-central pull.

I'm assuming that pragma isn't included in <windows.h> so I don't see the need to add an equivalent global for mingw.  I'd rather handle it on a case-by-case basis so that we're not needlessly linking a shared lib against every binary in the tree.

The existing patch already only sets LIBS for mingw.  One of the current build maintainers (Ben or Ted) might sign off on it but I don't think it's the correct approach.
Assignee

Comment 4

10 years ago
This pragma is in system headers, not in mozilla code. That's why it's used by default and we'd have to disable it to make things maintainable.

Comment 5

10 years ago
You mean, you'd have to disable pragmas to make the MSVC build behave the same way as the mingw build.  Given the mingw build's support level, I don't think that's going to be acceptable to the powers that be.

Adding |OS_LIBS += -luuid| where needed is a perfectly maintainable solution.  It just requires mingw builders to do the maintaining just like we already do for differences between the MSVC & mingw system headers.

Your proposed change would result in non-COM code always linking against uuid as well.  Is MSVC's linker smart about ignoring the listed libraries if no symbols from that library are used?  gcc/binutils are not last I checked.  If the library is listed, it becomes a runtime dependencies.
Assignee

Comment 6

10 years ago
(In reply to comment #5)
> You mean, you'd have to disable pragmas to make the MSVC build behave the same
> way as the mingw build.  Given the mingw build's support level, I don't think
> that's going to be acceptable to the powers that be.

I thought you will think so and I understand that.
 
> Adding |OS_LIBS += -luuid| where needed is a perfectly maintainable solution. 
> It just requires mingw builders to do the maintaining just like we already do
> for differences between the MSVC & mingw system headers.

Sorry for OT:
Currently I'm probably the only person who build on mingw every few days. There are quite a few patches that are needed to fix the build (I'm Wine developer and we ship our Gecko build as a package used for our mshtml.dll replacement), most of them are waiting for attention in bugzilla. I hope that quality of mingw support will change once bug 421095 will be fixed (we're slowly getting there) and perhaps as soon as it will work well, developers using Linux will consider it for quick Windows code testing using cross compilation (and perhaps even Wine:-) ). Anyway, the problem is that even trivial patch that I can write in a few minutes and is approved on the first review takes me weeks to get reviewed and committed. I don't complain and I understand that if someone is not a core developer, his patches are lower priority, but it means that the tree will always be broken. There will be another breakage before previous bug will be fixed. That's why I'm hoping to get a patch that may be not pretty, but will help with maintaining.

> Your proposed change would result in non-COM code always linking against uuid
> as well.  Is MSVC's linker smart about ignoring the listed libraries if no
> symbols from that library are used?  gcc/binutils are not last I checked.  If
> the library is listed, it becomes a runtime dependencies.

I've just tested it with GCC 3.4.6 (mingw32 target) and MSVC and both don't seem link libraries that are not truly needed. It means that adding uuid to LIBS won't hurt resulting binaries. If we'd add it to LIBS for mingw, there is no reason to change MSVC build system.
It sounds like adding it for the mingw build in configure is a reasonable solution.
Assignee

Updated

10 years ago
Attachment #412892 - Flags: review- → review?(ted.mielczarek)
Attachment #412892 - Flags: review?(ted.mielczarek) → review+
Assignee

Updated

10 years ago
Keywords: checkin-needed
Assignee: nobody → jacek
http://hg.mozilla.org/mozilla-central/rev/c5d0457346b6
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.