Closed
Bug 1186782
Opened 9 years ago
Closed 9 years ago
nsMailComps.o: In function `DirectoryProvider': mail/components/build/../shell/DirectoryProvider.h:35: undefined reference to `vtable for mozilla::mail::DirectoryProvider'
Categories
(MailNews Core :: Build Config, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 42.0
People
(Reporter: ewong, Assigned: iannbugzilla)
References
Details
(Keywords: dogfood, regression)
Attachments
(2 files, 6 obsolete files)
26.21 KB,
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
12.93 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
On bld-linux64-spot-201 for TB Linux x86-64 comm-central build, Getting: ./../mail/components/build/nsMailComps.o: In function `DirectoryProvider': /builds/slave/tb-c-cen-l64-00000000000000000/build/mail/components/build/../shell/DirectoryProvider.h:35: undefined reference to `vtable for mozilla::mail::DirectoryProvider' /usr/bin/ld: ../../mail/components/build/nsMailComps.o: relocation R_X86_64_PC32 against undefined hidden symbol `_ZTVN7mozilla4mail17DirectoryProviderE' can not be used when making a shared object /usr/bin/ld: final link failed: Bad value collect2: error: ld returned 1 exit status make[5]: *** [libxul.so] Error 1 make[5]: Leaving directory `/builds/slave/tb-c-cen-l64-00000000000000000/build/objdir-tb/toolkit/library' make[4]: *** [toolkit/library/target] Error 2 make[4]: *** Waiting for unfinished jobs....
Reporter | ||
Updated•9 years ago
|
Summary: nsMailComps.o: In function `DirectoryProvider': /builds/slave/tb-c-cen-l64-00000000000000000/build/mail/components/build/../shell/DirectoryProvider.h:35: undefined reference to `vtable for mozilla::mail::DirectoryProvider' → nsMailComps.o: In function `DirectoryProvider': mail/components/build/../shell/DirectoryProvider.h:35: undefined reference to `vtable for mozilla::mail::DirectoryProvider'
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Updated•9 years ago
|
Product: MailNews Core → Thunderbird
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Updated•9 years ago
|
OS: Unspecified → Linux
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 43•9 years ago
|
||
This went through try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f77164f048cc
Reporter | ||
Comment 44•9 years ago
|
||
Attachment #8638356 -
Attachment is obsolete: true
Attachment #8638356 -
Flags: review?(Pidgeot18)
Attachment #8638385 -
Flags: review?(Pidgeot18)
Updated•9 years ago
|
Keywords: regression
Comment 45•9 years ago
|
||
Comment on attachment 8638385 [details] [diff] [review] proposed patch (v2) Review of attachment 8638385 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/components/shell/moz.build @@ +13,5 @@ > > if CONFIG['OS_ARCH'] == 'WINNT': > SOURCES += ['nsMailWinIntegration.cpp'] > > +if CONFIG['MOZ_WIDGET_GTK']: You have many places where you replaced the == with just a def check. Shouldn't this be instead: if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('gtk2', 'gtk3'): (here and various other places)?
Comment 46•9 years ago
|
||
OK I picked a bad spot in the previous comment, but the question still stands. You replaced equality with gtk2 to checking for existence in a number of places. Why?
Comment 47•9 years ago
|
||
Comment on attachment 8638385 [details] [diff] [review] proposed patch (v2) Review of attachment 8638385 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/app/Makefile.in @@ +109,5 @@ > libs:: $(addprefix $(DIST)/branding/,$(BRANDED_ICON_FILES)) > $(INSTALL) $(IFLAGS1) $^ $(DIST)/bin/chrome/icons/default > endif > > +ifdef MOZ_WIDGET_TOOLKIT This spot is a better example.
Reporter | ||
Comment 48•9 years ago
|
||
(In reply to Kent James (:rkent) from comment #45) > Comment on attachment 8638385 [details] [diff] [review] > proposed patch (v2) > > Review of attachment 8638385 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mail/components/shell/moz.build > @@ +13,5 @@ > > > > if CONFIG['OS_ARCH'] == 'WINNT': > > SOURCES += ['nsMailWinIntegration.cpp'] > > > > +if CONFIG['MOZ_WIDGET_GTK']: > > You have many places where you replaced the == with just a def check. > Shouldn't this be instead: > > if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('gtk2', 'gtk3'): > > (here and various other places)? MOZ_WIDGET_GTK is defined only on platforms that use either cairo-gtk2 or cairo-gtk3, so it is sufficient (from what I see) to just check the existence of the value. It's actually equivalent to what you put; but cleaner. :)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 67•9 years ago
|
||
As I said, my comment 45 was an error, I meant to comment (per my correction in comment 47) about using MOZ_WIDGET_TOOLKIT, such as this change: -ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2) +ifdef MOZ_WIDGET_TOOLKIT I see lots of code like this: elif CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows': that shows that MOZ_WIDGET_TOOLKIT is not linux-specific.
Comment 68•9 years ago
|
||
Comment on attachment 8638385 [details] [diff] [review] proposed patch (v2) Review of attachment 8638385 [details] [diff] [review]: ----------------------------------------------------------------- You're missing a few changes where MOZ_WIDGET_GTK2 is used in code: <https://dxr.mozilla.org/comm-central/search?q=MOZ_WIDGET_GTK2&case=true>. The changes in nsMailComps.cpp are absolutely necessary to get the default application stuff working. The change to enable_drag_images are less clear if they're necessary (although that looks to be bug 450971). ::: mail/branding/aurora/Makefile.in @@ +20,5 @@ > cp $(srcdir)/dsstore $(DIST)/branding/dsstore > cp $(srcdir)/disk.icns $(DIST)/branding/disk.icns > cp $(srcdir)/thunderbird.icns $(DIST)/branding/thunderbird.icns > endif > +ifdef MOZ_WIDGET_TOOLKIT As rkent says, these lines are busted, since it's selecting these files for all platforms, whereas the desire is only for Linux ones.
Attachment #8638385 -
Flags: review?(Pidgeot18) → review-
Assignee | ||
Comment 69•9 years ago
|
||
The only thing I wasn't sure about was whether it was worth making changes to mailnews/build/nsMailModule.cpp Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fc221c9a961a
Attachment #8638385 -
Attachment is obsolete: true
Attachment #8639041 -
Flags: review?(Pidgeot18)
Assignee | ||
Comment 70•9 years ago
|
||
Attachment #8639042 -
Flags: review?(florian)
Attachment #8639042 -
Flags: review?(clokep)
Assignee | ||
Comment 71•9 years ago
|
||
Changes since last patch: * Missed package-manifest entries for mozgtk* in suite/
Attachment #8639041 -
Attachment is obsolete: true
Attachment #8639041 -
Flags: review?(Pidgeot18)
Attachment #8639044 -
Flags: review?(Pidgeot18)
Assignee | ||
Comment 72•9 years ago
|
||
Changes since previous patch: * Missed package-manifest changes for mozgtk* entries.
Attachment #8639042 -
Attachment is obsolete: true
Attachment #8639042 -
Flags: review?(florian)
Attachment #8639042 -
Flags: review?(clokep)
Attachment #8639045 -
Flags: review?(florian)
Attachment #8639045 -
Flags: review?(clokep)
Comment 73•9 years ago
|
||
Comment on attachment 8639044 [details] [diff] [review] GTK changes for TB, SM and MailNews v1.1 Review of attachment 8639044 [details] [diff] [review]: ----------------------------------------------------------------- You're missing other-licenses/branding/thunderbird/Makefile.in, but otherwise everything looks good. Although it looks like our removed-files.in could use a bit of pruning.
Attachment #8639044 -
Flags: review?(Pidgeot18) → review+
Assignee | ||
Comment 74•9 years ago
|
||
Added other-licenses changes for TB, carrying forward r=
Attachment #8639044 -
Attachment is obsolete: true
Attachment #8639095 -
Flags: review+
Reporter | ||
Updated•9 years ago
|
Assignee: ewong → iann_bugzilla
Assignee | ||
Comment 75•9 years ago
|
||
Added changes in other-licenses for InstantBird
Attachment #8639045 -
Attachment is obsolete: true
Attachment #8639045 -
Flags: review?(florian)
Attachment #8639045 -
Flags: review?(clokep)
Attachment #8639096 -
Flags: review?(florian)
Attachment #8639096 -
Flags: review?(clokep)
Assignee | ||
Comment 76•9 years ago
|
||
Comment on attachment 8639095 [details] [diff] [review] GTK changes for TB, SM and MailNews v1.2 [Checked in: Comment 76 & Comment 79] https://hg.mozilla.org/comm-central/rev/ae95dbce7c48
Attachment #8639095 -
Attachment description: GTK changes for TB, SM and MailNews v1.2 → GTK changes for TB, SM and MailNews v1.2 [Checked in: Comment 76]
Comment 77•9 years ago
|
||
Comment on attachment 8639096 [details] [diff] [review] GTK changes for IM v1.2 [Checked in: Comment 78] Thanks!
Attachment #8639096 -
Flags: review?(florian)
Attachment #8639096 -
Flags: review?(clokep)
Attachment #8639096 -
Flags: review+
Assignee | ||
Comment 78•9 years ago
|
||
Comment on attachment 8639096 [details] [diff] [review] GTK changes for IM v1.2 [Checked in: Comment 78] https://hg.mozilla.org/comm-central/rev/a875b1e5aa1c
Attachment #8639096 -
Attachment description: GTK changes for IM v1.2 → GTK changes for IM v1.2 [Checked in: Comment 78]
Assignee | ||
Comment 79•9 years ago
|
||
Comment on attachment 8639095 [details] [diff] [review] GTK changes for TB, SM and MailNews v1.2 [Checked in: Comment 76 & Comment 79] Remove unneeded gtk argument: https://hg.mozilla.org/comm-central/rev/28bda87ffa1f
Attachment #8639095 -
Attachment description: GTK changes for TB, SM and MailNews v1.2 [Checked in: Comment 76] → GTK changes for TB, SM and MailNews v1.2 [Checked in: Comment 76 & Comment 79]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•