376790 - remove support for non-cairo GFX ports from configure.in

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect)

Description

[David Baron :dbaron:]

Now that non-cairo GFX ports no longer build, we should remove support for them from configure.in.  Having options that don't work anymore can confuse people trying to build Mozilla, and we shouldn't have code in the tree that we don't maintain.  There's probably some associated code that can be removed along with just the code that accepts the options (or sets default values to them).

(We might also want to remove the "cairo-" from the names for the cairo options, since they'll all be cairo.)

Comment 1

[Peter Weilbacher]

This bug report is rather unspecific. What the summary suggests has already been done in bug 372151, other code removal is the topic of blockers of bug 376791.

If the removal or "cairo-" from the toolkit names is really wanted, this should be more specific (I don't really see much of a value in that, though). If not let's dupe this...

Comment 2

[David Baron :dbaron:]

Well, many of the options associated with those ports haven't been removed yet, such as --enable-xft, --enable-pango, --disable-postscript, MOZ_ENABLE_COREXFONTS, --disable-xprint, and various code that tests for their results.

Comment 3

[Peter Weilbacher]

I'm willing to try this one, removing stuff is fun. :-)

Don't find xprint any more, so that was probably removed in the meantime. What about MOZ_EXTRA_X11CONVERTERS that depends on MOZ_ENABLE_COREXFONTS? I see lots of #ifdefs for that in intl. I guess disable-postscript shouldn't be touched, in case embedders don't want PS? So that leaves to remove MOZ_ENABLE_COREXFONTS and subsume MOZ_ENABLE_PANGO and MOZ_ENABLE_XFT under MOZ_ENABLE_GTK2. Am I missing something?

As BeOS still exists in widget and other places I don't want to tackle that in this bug.

Comment 4

[tqh]

The BeOS port has a problem as it doesn't have a working Cairo or any of it's needed libraries. If we had them working we could continue working on trunk, but right now that isn't possible. 

Sidenote:
Cairo, Pango and so on doesn't really fit in on BeOS. Having well defined interfaces so we could adapt to native libraries (like gfx and widget did) would be much preferred by us. Unfortunatly now that Cairo is used in a lot of places directly it's very hard to do any other way. Finding someone to maintain and build Cairo is probably hard as the code is a bit messy and subpar compared to the native 'Interface Kit' and it will probably only be used by Mozilla.

Comment 5

[Peter Weilbacher]

Attachment: patch

This should do (most of) what David had in mind. I didn't try to touch MOZ_EXTRA_X11CONVERTERS yet.

Comment 6

[Peter Weilbacher]

Comment on attachment 297349 [details] [diff] [review]
patch

A little cleanup (that works as far as the try server let's me test it).

Comment 7

[Peter Weilbacher]

Patch checked into trunk.

I filed bug 413613 as a follow-up on perhaps dealing with MOZ_EXTRA_X11CONVERTERS.

Comment 8

[David Baron :dbaron:]

Doesn't doing what you did with MOZ_EXTRA_X11CONVERTERS mean you're now suddenly enabling a bunch of code that's been disabled for a long time?  (Since MOZ_ENABLE_COREXFONTS has been off by default for quite a while.)   If that's the case, I think that bug needs to be fixed or this one should be backed out.

Comment 9

[David Baron :dbaron:]

... or just never defining MOZ_EXTRA_X11CONVERTERS (but leaving the AC_SUBST), and leaving the removal until later.

Comment 10

[Peter Weilbacher]

A pity that such comments often come up only after the checkin...

In any case, what I removed was
   if test "$MOZ_X11"
   then
       MOZ_ENABLE_COREXFONTS=${MOZ_ENABLE_COREXFONTS-1}
   else
       MOZ_ENABLE_COREXFONTS=
   fi
   if test "$MOZ_ENABLE_COREXFONTS"
   then
       AC_DEFINE(MOZ_ENABLE_COREXFONTS)
   fi
   AC_SUBST(MOZ_ENABLE_COREXFONTS)

I may have misunderstood this before but I have now tested in bash and ash that the MOZ_X11 case actually results in MOZ_ENABLE_COREXFONTS=1 (for the case that the variable never existed before, it is only set to nothing in the wince case), so nothing changed.

Comment 11

[Peter Weilbacher]

Also, looking at the codesize graph of the "Linux fx-linux-tbox Dep Nightly" tinderbox I see a small jump around before the patch went in but not afterwards (it went in on 2008-01-23 00:27 and the jump is before midnight).

Comment 12

[David Baron :dbaron:]

Oh, I see -- that was really a regression from when cairo was turned on, since the gtk2 section of configure.in had

    MOZ_ENABLE_COREXFONTS=${MOZ_ENABLE_COREXFONTS-}

which made it default to off for --enable-default-toolkit=gtk2, but that was never copied to the cairo-gtk2 section.