Closed Bug 471877 Opened 11 years ago Closed 11 years ago

Cleanup GTK includes

Categories

(Core :: Widget: Gtk, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: Swatinem, Assigned: Swatinem)

References

Details

Attachments

(3 files)

From http://live.gnome.org/GnomeGoals/CleanupGTKIncludes :
GTK+ is moving toward a model where it is only allowed to include the 'toplevel' headers. Only <glib.h>, <gdk/gdk.h>, <gdk-pixbuf/gdk-pixbuf.h> and <gtk/gtk.h> can be directly included.

I'm not sure if I caught all the includes. The libgnome-vfs system includes break when I try to compile with -DG_DISABLE_SINGLE_INCLUDES.
Attachment #355126 - Flags: superreview?(roc)
Attachment #355126 - Flags: review?(roc)
Attachment #355126 - Flags: superreview?(roc)
Attachment #355126 - Flags: superreview+
Attachment #355126 - Flags: review?(roc)
Attachment #355126 - Flags: review+
Keywords: checkin-needed
Comment on attachment 355126 [details] [diff] [review]
cleanup
[Checkin: Comment 1+3+4]

http://hg.mozilla.org/mozilla-central/rev/043671bf5dfc
Attachment #355126 - Attachment description: cleanup → cleanup [Checkin: Comment 1]
(In reply to comment #0)
> I'm not sure if I caught all the includes. The libgnome-vfs system includes
> break when I try to compile with -DG_DISABLE_SINGLE_INCLUDES.

I leave it to you to carry on or resolve this bug.
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.1b3
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1
Comment on attachment 355126 [details] [diff] [review]
cleanup
[Checkin: Comment 1+3+4]

For example:
{
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1230968572.1230968717.24315.gz
Linux mozilla-central build on 2009/01/02 23:42:52

/builds/moz2_slave/mozilla-central-linux/build/widget/src/gtk2/nsDeviceContextSpecG.h:51:30: error: gtk/gtkunixprint.h: No such file or directory
}

http://hg.mozilla.org/mozilla-central/rev/054ca03946b5
bustage fix (= revert nsDeviceContextSpecG.h)

<gtk/gtkunixprint.h> should be there per
http://svn.gnome.org/viewvc/gtk%2B/trunk/gtk/

Maybe the tinderboxes don't have the (patch-)expected gtk+ version ?
Attachment #355126 - Attachment description: cleanup [Checkin: Comment 1] → cleanup [Checkin: Comment 1+3]
Comment on attachment 355126 [details] [diff] [review]
cleanup
[Checkin: Comment 1+3+4]

(In reply to comment #3)

http://hg.mozilla.org/mozilla-central/rev/b7c83a7b2b24
bustage fix (= revert nsPrintDialogGTK.cpp and nsPrintSettingsGTK.h too)
Attachment #355126 - Attachment description: cleanup [Checkin: Comment 1+3] → cleanup [Checkin: Comment 1+3+4]
Looks like gtkunixprint.h is only there since 2.14. Sorry for the inconvenience.
Some more includes I found using mxr, as well as cleaning those includes from system-headers
Attachment #355826 - Flags: superreview?(roc)
Attachment #355826 - Flags: review?(roc)
Just to have the patch ready when gecko switches to gtk 2.14.
(In reply to comment #7)
> Just to have the patch ready when gecko switches to gtk 2.14.

Considering the fuss we had getting the minimum changed to 2.10, I don't see us upping it to 2.14 anytime soon.
Attachment #355826 - Flags: superreview?(roc)
Attachment #355826 - Flags: superreview+
Attachment #355826 - Flags: review?(roc)
Attachment #355826 - Flags: review+
Keywords: checkin-needed
Whiteboard: [needs GTK+ 2.14]
Comment on attachment 355827 [details] [diff] [review]
gtkunixprint (needs gtk 2.14)
[See bug 474599]

Per comment 8, I suggest to move this patch to a separate (future) bug about upgrading GTK+ (to 2.14).
(In reply to comment #9)
> (From update of attachment 355827 [details] [diff] [review])
> Per comment 8, I suggest to move this patch to a separate (future) bug about
> upgrading GTK+ (to 2.14).

I agree. We can land what can be landed right now and leave the other stuff to a new bug.
Comment on attachment 355826 [details] [diff] [review]
further cleanup + system-headers cleanup
[Checkin: Comment 11]


http://hg.mozilla.org/mozilla-central/rev/1804a811b099
Attachment #355826 - Attachment description: further cleanup + system-headers cleanup → further cleanup + system-headers cleanup [Checkin: Comment 11]
Attachment #355827 - Attachment description: gtkunixprint (needs gtk 2.14) → gtkunixprint (needs gtk 2.14) [See bug 474599]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs GTK+ 2.14]
Duplicate of this bug: 504318
You need to log in before you can comment on or make changes to this bug.