388663 - Export thebes symbols

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Stuart Parmenter]

Thebes was built such that it could be used extensions and shouldn't be hidden away.

Comment 1

[Benjamin Smedberg]

As noted in the newsgroup discussion, I strongly disagree with this decision: exposing symbols that we don't intend to keep stable even on a stable branch is a recipe for users who either won't upgrade (because their extensions would break), or blame Firefox for breaking their extensions in security releases.

http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/100fa38c7eb15fcf/207e02714a96554c#207e02714a96554c

Comment 2

[Stuart Parmenter]

Attachment: fix

Comment 3

[Stuart Parmenter]

Benjamin: I don't think we're going to come to an agreement on this issue.

Comment 4

[Benjamin Smedberg]

Well, since you and I both claim module ownership over the decision, I guess we have no choice but to appeal to Brendan.

Comment 5

[Brendan Eich [:brendan]]

Security releases are not likely to break extensions using thebes unless the security fix affects thebes. I do not believe we'll face ABI shifts on patch releases; but perhaps I underestimate the problem on Linux.

We've wasted a lot of our time and developers' time on restricting API exports to frozen ones, plus some that are necessary to do useful extensions or other apps even though not frozen. It has been a mixed bag at best, and at worst a denial of service to would-be extenders. I'd rather have more extensions using more APIs and figure out which APIs were worth freezing, and which needed fixing, than try to get things right "in the cathedral".

This is not to say we should open the floodgates. Let's get our feet wet here, with Thebes. I'm in favor of taking some tiny risk here for pretty clear wins. So I rule in favor of Stuart.

Cc'ing Graydon, with whom I once discussed the whole torturous API freezing and export limiting process we've been through. I can't find a written record, but I recall he had some useful insights (which align with my position here, and may in fact be worth thinking about in a bigger, "floodgate" way).

/be

Comment 6

[Brendan Eich [:brendan]]

Thebes API review would be good, before 1.9 betas. I'm going to look for bugmail cc'ing me on that bug that Stuart's gonna file ;-).

/be

Comment 7

[Phil Ringnalda (:philor)]

This seems to have set Thunderbird/Windows on fire.

Comment 8

[(not currently active) Ted Mielczarek]

I'm guessing this is because this breaks in static builds, and Thunderbird is the only thing left doing them.

-#if defined(MOZ_ENABLE_LIBXUL) || defined(MOZ_STATIC_BUILD)

You ditched the MOZ_STATIC_BUILD part, which you really shouldn't have.

Comment 9

[(not currently active) Ted Mielczarek]

Attachment: fix static build bustage

I checked this in to fix the Thunderbird bustage.

Comment 10

[Stuart Parmenter]

Comment on attachment 272976 [details] [diff] [review]
fix static build bustage

this won't really work.  I guess we need to figure out how to go back to having our own shared library.  I'm not sure how to do that properly  so that we work with libxul and with static builds.. thoughts?

Comment 11

[(not currently active) Ted Mielczarek]

Do we need to support this in static builds?  We're not shipping Firefox in that configuration, and Thunderbird wants to go libxul as well (bug 377319).  Seems like a lot of extra trouble to go through for a configuration we're trying to drop.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Could this have caused the Linux Tp regression on July 18/19?

Comment 13

[Adam Guthrie]

(In reply to comment #12)

Yes, but according to Stuart, this is what it was at before libxul landed and it's going to stay this way.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is this the reason I'm now seeing a ton of compiler warnings like the following?

gkwidget.lib(nsDeviceContextSpecWin.obj) : warning LNK4217: locally defined symbol ??1gfxPDFSurface@@UAE@XZ (public: virtual __thiscall gfxPDFSurface::~gfxPDFSurface(void)) imported in function "public: virtual void * __thiscall gfxPDFSurface::`scalar deleting destructor'(unsigned int)" (??_GgfxPDFSurface@@UAEPAXI@Z)