Closed
Bug 937359
Opened 11 years ago
Closed 11 years ago
Build failure when reordering objects when building xul.dll
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla28
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
The error is:
LIBCMTD.lib(crt0.obj) : error LNK2019: unresolved external symbol _main referenced in
function ___tmainCRTStartup
xul.dll : fatal error LNK1120: 1 unresolved externals
This happens when building with the patches from bug 935881, but this can be easily reproduced by moving xulapp_s after the addition of component libs:
diff --git a/toolkit/library/Makefile.in b/toolkit/library/Makefile.in
--- a/toolkit/library/Makefile.in
+++ b/toolkit/library/Makefile.in
@@ -16,7 +16,6 @@ VPATH += $(topsrcdir)/build/
SHARED_LIBRARY_LIBS += \
$(DEPTH)/media/kiss_fft/$(LIB_PREFIX)kiss_fft.$(LIB_SUFFIX) \
- $(DEPTH)/toolkit/xre/$(LIB_PREFIX)xulapp_s.$(LIB_SUFFIX) \
$(NULL)
ifdef ACCESSIBILITY
@@ -177,6 +176,7 @@ COMPONENT_LIBS += \
$(NULL)
SHARED_LIBRARY_LIBS += \
+ $(DEPTH)/toolkit/xre/$(LIB_PREFIX)xulapp_s.$(LIB_SUFFIX) \
$(DEPTH)/docshell/base/$(LIB_PREFIX)basedocshell_s.$(LIB_SUFFIX) \
$(DEPTH)/uriloader/base/$(LIB_PREFIX)uriloaderbase_s.$(LIB_SUFFIX) \
$(DEPTH)/uriloader/exthandler/$(LIB_PREFIX)exthandler_s.$(LIB_SUFFIX) \
Further hacking indicate the directshow part of gklayout is the trigger.
LIBCMTD.lib is the multithreaded static linking run time library. We're supposed to use MSVCRTD.lib. This would suggest we are building some things with -MTd instead of -MDd.
Comment 1•11 years ago
|
||
I think this is the bug I hit when I naively attempted to reorder libraries by size in dependency libs foo! Can't remember the bug # though...
Assignee | ||
Comment 2•11 years ago
|
||
And the winner is the updater:
http://hg.mozilla.org/mozilla-central/file/581d180a37f3/toolkit/mozapps/update/common/Makefile.in#l6
Assignee | ||
Comment 3•11 years ago
|
||
So, looking around for USE_STATIC_LIBS, there are a few more than updatecommon that look problematic: one in xpcom/typelib/xpt/src, and those in browser/components.
I can understand why some executables we build may want USE_STATIC_LIBS, but I'm not sure why browser/components would need that. Benjamin or Ted, would you happen to know? I can understand if third party binary components would want that, but not those we ship with firefox.
Flags: needinfo?(ted)
Flags: needinfo?(benjamin)
Assignee | ||
Comment 4•11 years ago
|
||
Same for toolkit/crashreporter/breakpad-windows-standalone, which ends up in breakpadinjector.dll?
Comment 5•11 years ago
|
||
breakpadinjector must statically link the CRT because of the way we load it into the Flash process (we cannot link against any DLL except kernel32 and maybe user32).
browser/components is probably safe to remove: we linked it statically against the CRT back when the CRT was a Windows assembly and it was causing problems that components/browsercomps.dll was in a subdirectory and couldn't find the assembly.
xpcom/typelib was for the binary version of the xpt tools (xpidl and xpt_link), both of which are python now and it probably doesn't matter.
Flags: needinfo?(benjamin)
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #5)
> breakpadinjector must statically link the CRT because of the way we load it
> into the Flash process (we cannot link against any DLL except kernel32 and
> maybe user32).
Fair enough, I see the other parts of breakpad injector are also with USE_STATIC_LIBS, so at least from consistency perspective, it's fine.
> browser/components is probably safe to remove: we linked it statically
> against the CRT back when the CRT was a Windows assembly and it was causing
> problems that components/browsercomps.dll was in a subdirectory and couldn't
> find the assembly.
Digging in the history of the corresponding Makefiles, I found bug 713167 in which it is claimed that the problem may have gone with msvc 2010, which we now require. I'll try this in a separate bug, as it can have implications that the other parts don't.
Flags: needinfo?(ted)
Assignee | ||
Comment 7•11 years ago
|
||
Took on the occasion to cleanup some Makefile variables.
Note xpt is linked to xpt tests, but those are not built with USE_STATIC_LIBS anyways.
Attachment #831144 -
Flags: review?(ted)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 831144 [details] [diff] [review]
Make all objects built into xul.dll built with -MD/-MDd, not -MT/-MTd
(for whichever of you two can review first)
Attachment #831144 -
Flags: review?(benjamin)
Updated•11 years ago
|
Attachment #831144 -
Flags: review?(benjamin) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #831144 -
Flags: review?(ted)
Assignee | ||
Comment 9•11 years ago
|
||
Comment 10•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•