Closed Bug 1186782 Opened 10 years ago Closed 10 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)

All
Linux
defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 42.0

People

(Reporter: ewong, Assigned: iannbugzilla)

References

Details

(Keywords: dogfood, regression)

Attachments

(2 files, 6 obsolete files)

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....
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'
Product: MailNews Core → Thunderbird
OS: Unspecified → Linux
Attached patch proposed patch(v1) (obsolete) — Splinter Review
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #8638356 - Flags: review?(Pidgeot18)
Attached patch proposed patch (v2) (obsolete) — Splinter Review
Attachment #8638356 - Attachment is obsolete: true
Attachment #8638356 - Flags: review?(Pidgeot18)
Attachment #8638385 - Flags: review?(Pidgeot18)
Severity: normal → blocker
Keywords: dogfood
Keywords: regression
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)?
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 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.
(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. :)
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 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-
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)
Hardware: Unspecified → All
Version: unspecified → Trunk
Product: Thunderbird → MailNews Core
Attached patch GTK changes for IM (obsolete) — Splinter Review
Attachment #8639042 - Flags: review?(florian)
Attachment #8639042 - Flags: review?(clokep)
Blocks: 882036
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)
Attached patch GTK changes for IM v1.1 (obsolete) — Splinter Review
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 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+
Added other-licenses changes for TB, carrying forward r=
Attachment #8639044 - Attachment is obsolete: true
Attachment #8639095 - Flags: review+
Assignee: ewong → iann_bugzilla
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)
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]
Target Milestone: --- → Thunderbird 42.0
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+
Attachment #8639096 - Attachment description: GTK changes for IM v1.2 → GTK changes for IM v1.2 [Checked in: Comment 78]
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: 10 years ago
Resolution: --- → FIXED
Depends on: 1188965
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: