The default bug view has changed. See this FAQ.

nsMailComps.o: In function `DirectoryProvider': mail/components/build/../shell/DirectoryProvider.h:35: undefined reference to `vtable for mozilla::mail::DirectoryProvider'

RESOLVED FIXED in Thunderbird 42.0

Status

MailNews Core
Build Config
--
blocker
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ewong, Assigned: Ian Neal)

Tracking

({dogfood, regression})

Trunk
Thunderbird 42.0
All
Linux
dogfood, regression
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 6 obsolete attachments)

(Reporter)

Description

2 years ago
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

2 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 (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Updated

2 years ago
Component: Build Config → Build Config
Product: MailNews Core → Thunderbird
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Updated

2 years ago
OS: Unspecified → Linux
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Comment 43

2 years ago
Created attachment 8638356 [details] [diff] [review]
proposed patch(v1)

This went through try:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f77164f048cc
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #8638356 - Flags: review?(Pidgeot18)
(Reporter)

Comment 44

2 years ago
Created attachment 8638385 [details] [diff] [review]
proposed patch (v2)
Attachment #8638356 - Attachment is obsolete: true
Attachment #8638356 - Flags: review?(Pidgeot18)
Attachment #8638385 - Flags: review?(Pidgeot18)

Updated

2 years ago
Severity: normal → blocker
Keywords: dogfood

Updated

2 years ago
Keywords: regression

Comment 45

2 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

2 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

2 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

2 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 (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 67

2 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 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

2 years ago
Created attachment 8639041 [details] [diff] [review]
GTK changes for TB, SM and MailNews

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)

Updated

2 years ago
Hardware: Unspecified → All
Version: unspecified → Trunk
(Assignee)

Updated

2 years ago
Component: Build Config → Build Config
Product: Thunderbird → MailNews Core
(Assignee)

Comment 70

2 years ago
Created attachment 8639042 [details] [diff] [review]
GTK changes for IM
Attachment #8639042 - Flags: review?(florian)
Attachment #8639042 - Flags: review?(clokep)
(Assignee)

Updated

2 years ago
Blocks: 882036
(Assignee)

Comment 71

2 years ago
Created attachment 8639044 [details] [diff] [review]
GTK changes for TB, SM and MailNews v1.1

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

2 years ago
Created attachment 8639045 [details] [diff] [review]
GTK changes for IM v1.1

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+
(Assignee)

Comment 74

2 years ago
Created attachment 8639095 [details] [diff] [review]
GTK changes for TB, SM and MailNews v1.2 [Checked in: Comment 76 & Comment 79]

Added other-licenses changes for TB, carrying forward r=
Attachment #8639044 - Attachment is obsolete: true
Attachment #8639095 - Flags: review+
(Reporter)

Updated

2 years ago
Assignee: ewong → iann_bugzilla
(Assignee)

Comment 75

2 years ago
Created attachment 8639096 [details] [diff] [review]
GTK changes for IM v1.2 [Checked in: Comment 78]

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

2 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]
(Assignee)

Updated

2 years ago
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+
(Assignee)

Comment 78

2 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

2 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]
(Assignee)

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Depends on: 1188965
You need to log in before you can comment on or make changes to this bug.