Closed
Bug 591576
Opened 14 years ago
Closed 14 years ago
[comm-central] widget/src/windows/GfxInfo.cpp now reports various errors
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
mozilla2.0b5
People
(Reporter: sgautherie, Assigned: jrmuizel)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
745 bytes,
patch
|
khuey
:
review+
philip.chee
:
feedback+
|
Details | Diff | Splinter Review |
First, all SM and TB builds failed due to 1(+) other cause, which regression timeframe was http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6c18a123fbb&tochange=9f8fc8a50525 When that was fixed, this new Windows-only error was revealed: http://tinderbox.mozilla.org/showbuilds.cgi?tree=Thunderbird&maxdate=1282937546&hours=24&legend=0&norules=1 http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1282925383.1282926551.31277.gz WINNT 5.2 comm-central build on 2010/08/27 09:09:43 { widget/src/windows/GfxInfo.cpp(194) : fatal error C1903: unable to recover from previous error(s); stopping compilation } Regression timeframe: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6c18a123fbb&tochange=a8328013a773 Likely culprits: bug 586046 (and/or bug 586048). (Damn, I missed to find bug 586046 comment 35 in my search :-/ Anyway.) Please, fix or back out! This has been going on for 16 hours!! :-(
Reporter | ||
Updated•14 years ago
|
Summary: widget/src/windows/GfxInfo.cpp now reports various errors → [comm-central] widget/src/windows/GfxInfo.cpp now reports various errors
Just wondering why this doesn't show for Firefox windows builds, some difference in build config? Does changing back to WCHAR from TCHAR fix it?
Assignee | ||
Comment 2•14 years ago
|
||
I believe this should be fixed now...
Assignee | ||
Comment 3•14 years ago
|
||
Apparently there are still linker errors. These should be able to be fixed by changing the ifdef MOZ_CRASHREPORTER in GfxInfo.cpp to #if MOZ_CRASHREPORTER && MOZ_ENABLE_LIBXUL or something like that.
Reporter | ||
Comment 4•14 years ago
|
||
(In reply to comment #3) > Apparently there are still linker errors. { GfxInfo.obj : error LNK2019: unresolved external symbol "unsigned int __cdecl CrashReporter::AppendAppNotesToCrashReport(class nsACString_internal const &)" (?AppendAppNotesToCrashReport@CrashReporter@@YAIABVnsACString_internal@@@Z) referenced in function "private: void __thiscall mozilla::widget::GfxInfo::AddCrashReportAnnotations(void)" (?AddCrashReportAnnotations@GfxInfo@widget@mozilla@@AAEXXZ) GfxInfo.obj : error LNK2019: unresolved external symbol "unsigned int __cdecl CrashReporter::AnnotateCrashReport(class nsACString_internal const &,class nsACString_internal const &)" (?AnnotateCrashReport@CrashReporter@@YAIABVnsACString_internal@@0@Z) referenced in function "private: void __thiscall mozilla::widget::GfxInfo::AddCrashReportAnnotations(void)" (?AddCrashReportAnnotations@GfxInfo@widget@mozilla@@AAEXXZ) NEXT ERROR gkwidget.dll : fatal error LNK1120: 2 unresolved externals }
Could that be from the landing for Bug 586048?
Assignee | ||
Comment 6•14 years ago
|
||
Does this patch fix it?
Comment 7•14 years ago
|
||
Alternatively SHARED_LIBRARY_LIBS += $(DEPTH)/toolkit/crashreporter/$(LIB_PREFIX)exception_handler_s.$(LIB_SUFFIX) Might work in widget/src/windows/Makefile.in
Comment 8•14 years ago
|
||
Comment on attachment 470194 [details] [diff] [review] Fix link error [Checkin: Comment 21] Patch works. I built SeaMonkey 2.1b1pre successfully.
Attachment #470194 -
Flags: feedback+
This patch: * Uses Callek's Makefile.in suggestion If this does not work, try doing between the ifdef/endif: SHARED_LIBRARY_LIBS += \ $(DEPTH)/toolkit/crashreporter/$(LIB_PREFIX)exception_handler_s.$(LIB_SUFFIX) $(DEPTH)/toolkit/crashreporter/google-breakpad/src/client/windows/handler/$(LIB_PREFIX)exception_handler_s.$(LIB_SUFFIX) \ $(DEPTH)/toolkit/crashreporter/google-breakpad/src/client/windows/crash_generation/$(LIB_PREFIX)crash_generation_s.$(LIB_SUFFIX) \ $(DEPTH)/toolkit/crashreporter/google-breakpad/src/common/windows/$(LIB_PREFIX)breakpad_windows_common_s.$(LIB_SUFFIX)
Attachment #470271 -
Flags: feedback?(philip.chee)
Comment 10•14 years ago
|
||
Comment on attachment 470271 [details] [diff] [review] Makefile.in patch > +ifdef MOZ_CRASHREPORTER > +SHARED_LIBRARY_LIBS += $(DEPTH)/toolkit/crashreporter/$(LIB_PREFIX)exception_handler_s.$(LIB_SUFFIX) > +endif I tried that earlier but got four unresolved externals instead of two.
Attachment #470271 -
Flags: feedback?(philip.chee)
Comment 11•14 years ago
|
||
Attachment #470271 -
Attachment is obsolete: true
Attachment #470279 -
Flags: feedback?(philip.chee)
Comment 12•14 years ago
|
||
(In reply to comment #6) > Created attachment 470194 [details] [diff] [review] > Fix link errror > > Does this patch fix it? This does work for me, I think we should just take it other than my suggestion.
Comment 13•14 years ago
|
||
Comment on attachment 470279 [details] [diff] [review] Makefile.in patch v2 Thanks this one works.
Attachment #470279 -
Flags: feedback?(philip.chee) → feedback+
Comment 14•14 years ago
|
||
(In reply to comment #13) > Comment on attachment 470279 [details] [diff] [review] > Makefile.in patch v2 > > Thanks this one works. From a chat with khuey on IRC, I doubt this one works, "right" (yes it links and runs). But I don't think linking in the crashreporter multiple times actually is correct.
Comment on attachment 470279 [details] [diff] [review] Makefile.in patch v2 SHARED_LIBRARY_LIBS says "take these static libs and make me a shared library out of them". This will link two separate copies of crashreporter, one into gkwidget.dll and another into wherever it lives ATM (probably xul.dll).
Attachment #470279 -
Flags: review-
Comment on attachment 470194 [details] [diff] [review] Fix link error [Checkin: Comment 21] This is what you want.
Attachment #470194 -
Flags: review+
Comment 17•14 years ago
|
||
(In reply to comment #16) > Comment on attachment 470194 [details] [diff] [review] > Fix link errror > > This is what you want. Except this version means unless you building with enable-xul you won't get crash reporter information for this part of the code.
(In reply to comment #17) > (In reply to comment #16) > > Comment on attachment 470194 [details] [diff] [review] [details] > > Fix link errror > > > > This is what you want. > > Except this version means unless you building with enable-xul you won't get > crash reporter information for this part of the code. It may be suboptimal, but I'm not going to take a patch that links crashreporter in twice for this. Ted, of course, is the module owner, so you may make your case to him if you disagree. Non-libxul will be dead soon enough anyways.
Comment 19•14 years ago
|
||
(In reply to comment #17) > (In reply to comment #16) > > Comment on attachment 470194 [details] [diff] [review] [details] > > Fix link errror > > > > This is what you want. > > Except this version means unless you building with enable-xul you won't get > crash reporter information for this part of the code. Correct, but I don't think this data is valuable enough to warrant a lot of extra time on this (for us), whereas libxul will let us get it for free. Also I don't want to take a patch that could also damage non-libxul being able to actually _use_ crashreporter.
Comment 20•14 years ago
|
||
(In reply to comment #19) > (In reply to comment #17) > > (In reply to comment #16) > > > Comment on attachment 470194 [details] [diff] [review] [details] [details] > > > Fix link errror > > > > > > This is what you want. > > > > Except this version means unless you building with enable-xul you won't get > > crash reporter information for this part of the code. > > Correct, but I don't think this data is valuable enough to warrant a lot of > extra time on this (for us), whereas libxul will let us get it for free. Also I > don't want to take a patch that could also damage non-libxul being able to > actually _use_ crashreporter. True, ship it (as they say).
Attachment #470279 -
Attachment is obsolete: true
Comment 21•14 years ago
|
||
Comment on attachment 470194 [details] [diff] [review] Fix link error [Checkin: Comment 21] http://hg.mozilla.org/mozilla-central/rev/4f6d1a4cd8ee
Attachment #470194 -
Attachment description: Fix link errror → Fix link error [Checkin: Comment 21]
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 22•14 years ago
|
||
V.Fixed, per http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1283123989.1283130605.8915.gz WINNT 5.2 comm-central-trunk build on 2010/08/29 16:19:49
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
Target Milestone: --- → mozilla2.0b5
You need to log in
before you can comment on or make changes to this bug.
Description
•