Closed Bug 388663 Opened 16 years ago Closed 16 years ago

Export thebes symbols


(Core :: Graphics, defect)

Windows XP
Not set





(Reporter: pavlov, Assigned: pavlov)




(2 files)

Thebes was built such that it could be used extensions and shouldn't be hidden away.
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.

Attached patch fixSplinter Review
Attachment #272853 - Flags: review?(vladimir)
Benjamin: I don't think we're going to come to an agreement on this issue.
Well, since you and I both claim module ownership over the decision, I guess we have no choice but to appeal to 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).

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 ;-).

Closed: 16 years ago
Resolution: --- → FIXED
This seems to have set Thunderbird/Windows on fire.
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.
I checked this in to fix the Thunderbird bustage.
Attachment #272976 - Flags: review?(pavlov)
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?
Attachment #272976 - Flags: review?(pavlov) → review-
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.
Could this have caused the Linux Tp regression on July 18/19?
(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.
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)
Depends on: 402853
You need to log in before you can comment on or make changes to this bug.