Hi Hans, Hans Breuer <hans@xxxxxxxxxx> writes: > >> Second question: > >> - to make Gimp compile again I needed to include some headers > >> in other headers again (see attached patch). Yes, I've read the > >> docs, which say not to do so, but is it really wanted to get > >> a header order dependency ? > > > >Please send the patch included in your email, not as zip archive. > >Makes it easier to read and allows me to comment on the diff in > >a reply. I have looked into it and parts of it look good, others > >need a different solution imho. > > > Here it comes. Missing a ChangeLog entry, which I I would write before > really commiting. Something like: Thanks for patch. I have some comments: > from-cvs/gimp/app/appenv.h my-gtk/gimp/app/appenv.h > --- > from-cvs/gimp/app/appenv.h Sun Feb 04 14:26:00 2001 > +++ > my-gtk/gimp/app/appenv.h Sun Feb 04 17:51:08 2001 > @@ -19,6 +19,7 @@ > > #ifndef __APPENV_H__ > #define __APPENV_H__ > > +#include <glib.h> /* gboolean > */ glib.h should never be included in any header file. > diff --exclude-from=c:\util\tool\diff.ign -u -r > from-cvs/gimp/app/cursorutil.h my-gtk/gimp/app/cursorutil.h > --- > from-cvs/gimp/app/cursorutil.h Thu Jan 11 23:33:48 2001 > +++ > my-gtk/gimp/app/cursorutil.h Sun Feb 04 17:32:38 2001 > @@ -19,6 +19,9 @@ > > #ifndef __CURSORUTIL_H__ > #define __CURSORUTIL_H__ > > +#include > <gdk/gdktypes.h> > +#include <gdk/gdkcursor.h> > +#include <gtk/gtkwidget.h> please don't include any of these in a header. > diff --exclude-from=c:\util\tool\diff.ign -u -r > from-cvs/gimp/app/datafiles.h my-gtk/gimp/app/datafiles.h > --- > from-cvs/gimp/app/datafiles.h Sun Feb 04 14:26:40 2001 > +++ > my-gtk/gimp/app/datafiles.h Sun Feb 04 18:42:08 2001 > @@ -22,6 +22,7 @@ > > #ifndef __DATAFILES_H__ > #define __DATAFILES_H__ > > +#include <time.h> This is perfectly ok since the "time_t" data type is used in the header and we don't want to include time.h from any c file which includes datafile.h > diff --exclude-from=c:\util\tool\diff.ign -u -r > from-cvs/gimp/app/dialog_handler.h my-gtk/gimp/app/dialog_handler.h > --- > from-cvs/gimp/app/dialog_handler.h Sun Feb 04 18:50:02 2001 > +++ > my-gtk/gimp/app/dialog_handler.h Sun Feb 04 19:16:42 2001 > @@ -20,8 +20,10 > @@ > #ifndef __DIALOG_HANDLER_H__ > #define __DIALOG_HANDLER_H__ > > > - > +#include <gmodule.h> This looks also ok to me. > diff --exclude-from=c:\util\tool\diff.ign -u -r > from-cvs/gimp/app/draw_core.h my-gtk/gimp/app/draw_core.h > --- > from-cvs/gimp/app/draw_core.h Thu Jan 11 23:34:00 2001 > +++ > my-gtk/gimp/app/draw_core.h Sun Feb 04 17:30:02 2001 > @@ -19,6 +19,8 @@ > > #ifndef __DRAW_CORE_H__ > #define __DRAW_CORE_H__ > > +#include > <gdk/gdktypes.h> > +#include "apptypes.h" This is absolutely forbidden :-) > diff --exclude-from=c:\util\tool\diff.ign -u -r > from-cvs/gimp/app/gdisplay.h my-gtk/gimp/app/gdisplay.h > --- > from-cvs/gimp/app/gdisplay.h Sun Feb 04 14:26:48 2001 > +++ > my-gtk/gimp/app/gdisplay.h Sun Feb 04 18:05:36 2001 > @@ -19,6 +19,8 @@ > > #ifndef __GDISPLAY_H__ > #define __GDISPLAY_H__ > > +#include > <gtk/gtkwidget.h> Again, gtk is included from every c file and should not be included in headers. > my-gtk/gimp/app/gimpdrawable.h > --- from-cvs/gimp/app/gimpdrawable.h Sun Feb > 04 14:27:10 2001 > +++ my-gtk/gimp/app/gimpdrawable.h Sun Feb 04 18:04:02 > 2001 > @@ -21,7 +21,7 @@ > > > #include "gimpobject.h" > - > +#include > "apptypes.h" Don't include apptypes.h here. > from-cvs/gimp/app/pdb/fileops_cmds.c Sun Jan 21 22:58:16 2001 > +++ > my-gtk/gimp/app/pdb/fileops_cmds.c Sun Feb 04 19:04:24 2001 > @@ -27,8 +27,11 > @@ > #include <unistd.h> > #endif > > - > #include <gtk/gtk.h> > + > +#ifdef > G_OS_WIN32 > +#include <process.h> /* For _getpid() */ > +#endif The files in the "pdb" dir are all autogenerated from <top_srcdir>/tools/pdbgen/pdb/. Please change it there. > --- from-cvs/gimp/app/plug_in.h Sun Feb 04 > 14:28:38 2001 > +++ my-gtk/gimp/app/plug_in.h Sun Feb 04 17:00:58 2001 > @@ > -21,7 +21,7 @@ > > > #include "pdb/procedural_db.h" > - > +#include <time.h> /* > time_t */ The time.h include is ok here, the procedural_db.h belongs to the c file. All the Win32 include reordering you've done in the *.c files are of course perfectly ok. Please, just remove the includes (except time.h) from the header files and feel free to commit the fixes. It may look a bit strange to create a header-file-order dependency by not allowing includes in headers, but this only applies to stuff like "include system stuff before gimp stuff". The gimp includes itself can be ordered arbitratily. The benefit we get from this is that we can grep the sources to see which functions are used from where. We don't need to include something to get access to data types because they live in the "inter-module" namespace established by apptypes.h ciao, --Mitch
