Created attachment 69065 [details] [review]
use hidden visibility if compiling with gcc >= 3.4

Ideally you would have a configure test that discovered whether this compiler feature was supported. Then you'd use the resulting define (from your config.h) to conditionally define a macro for this. glibmm does this kind of thing often for other compiler features.

Created attachment 69108 [details] [review]
define HAVE_GNUC_VISIBILITY

Defining something like HAVE_GNUC_VISIBILITY is an improvement, since new tests don't have to test the gcc version. I don't like to use configure for such simple tests, but if it is required I can create a new patch.

Comment on attachment 69108 [details] [review]
define HAVE_GNUC_VISIBILITY

>+#define HIDDEN __attribute__ ((visibility ("hidden")))

This is missing proper namespacing.  Please change the macro name to PYGTK_HIDDEN.
It would be nice also to push this macro upstream to glib, for glib 2.14.

Other problems:

hidden.h is not installed.  Yet, codegen unconditionally generates code that requires inclusion of this header.  I bet gnome-python-* will no longer build with this patch.
I suggest merging hidden.h into gtk/pygtk.h, or into pygobject.h (in pygobject), or install hidden.h and include it from pygtk.h.

Created attachment 69118 [details] [review]
define PYGOBJECT_HIDDEN

Created attachment 69119 [details] [review]
use PYGOBJECT_HIDDEN

(In reply to comment #4)
> It would be nice also to push this macro upstream to glib, for glib 2.14.
I will ask them to make G_HAVE_GNUC_VISIBILITY visible to user code.

G_HAVE_GNUC_VISIBILITY is already used in gtk+ 2.10..

PYGOBJECT_HIDDEN should be PYG_HIDDEN; that's for consistency, we've used PYG_ prefix in pygobject since forever...

Created attachment 69120 [details] [review]
define PYG_HIDDEN

Created attachment 69121 [details] [review]
use PYG_HIDDEN

(In reply to comment #8)
> G_HAVE_GNUC_VISIBILITY is already used in gtk+ 2.10..
I think that it is used internally only. I have gtk+ 2.10 installed and "grep -r G_HAVE_GNUC_VISIBILITY" in /usr/include returns

/usr/include/gtk-2.0/gdk/gdkalias.h:#ifdef G_HAVE_GNUC_VISIBILITY
/usr/include/gtk-2.0/gdk/gdkalias.h:#endif /* G_HAVE_GNUC_VISIBILITY *

http://developer.gnome.org/doc/API/2.0/glib/glib-Miscellaneous-Macros.html#G-HAVE-GNUC-VISIBILITY:CAPS
http://developer.gnome.org/doc/API/2.0/glib/glib-Miscellaneous-Macros.html#G-GNUC-INTERNAL:CAPS

Apparently "since 2.6", so...

PS: these macros are in /usr/lib/glib-2.0/include/glibconfig.h, that's why you can't find them grepping /usr/include.

Created attachment 69130 [details] [review]
alternative patch that uses G_GNUC_INTERNAL

Comment on attachment 69130 [details] [review]
alternative patch that uses G_GNUC_INTERNAL

>Index: atk.override
>===================================================================
>RCS file: /cvs/gnome/pygtk/atk.override,v
>retrieving revision 1.15
>diff -u -r1.15 atk.override
>--- atk.override	6 Jul 2006 17:44:55 -0000	1.15
>+++ atk.override	18 Jul 2006 16:55:15 -0000
>@@ -23,6 +23,7 @@
> headers
> #define NO_IMPORT_PYGOBJECT
> #include "pygobject.h"
>+#include <glibconfig.h>

Please drop the <glibconfig.h> includes everywhere; you don't need them.  After that, it's ready for commit.

> Please drop the <glibconfig.h> includes everywhere; you don't need them.  After
> that, it's ready for commit.

Actually we do. I added them because the build was failing.

(In reply to comment #16)
> > Please drop the <glibconfig.h> includes everywhere; you don't need them.  After
> > that, it's ready for commit.
> 
> Actually we do. I added them because the build was failing.
> 

Weird... :|  glib/gtypes.h includes it, and glib/gtypes.h is needed everywhere...

In fact, the patch compiles fine here in HEAD if I remove the <glibconfig.h> #includes; what glib version do you have?

I have just tried the patch without the includes and it works. I was having some problems with the *.c not being regenerated automatically. I though that the problem was the missing includes.

Reopening to hold any extra patches floating around...

Created attachment 69474 [details] [review]
make pyglade_xml_new static

Created attachment 69475 [details] [review]
make pygdk_event_handler_marshal static

Created attachment 69476 [details] [review]
make PyGContainerIter_Type static

Created attachment 69477 [details] [review]
make pygtk_tree_model_row_iter_getiter static

Created attachment 69478 [details] [review]
make pygtk_tree_model_row_iter_next static

Created attachment 69479 [details] [review]
make _wrap_gdk_event_tp_getattr static

Created attachment 69480 [details] [review]
make _wrap_gdk_gc_tp_getattro static

Created attachment 69481 [details] [review]
make pygtk_make_pixbuf_format_dict static

Created attachment 69482 [details] [review]
make pygtk_find_char_pred static

Created attachment 69484 [details] [review]
make PyGtkDeprecationWarning static

Is it really necessary to put every single change in its own patch?

No, but I did it to make it easier for you to accept or reject it. I have them in a local branch with svn. I can build larger patches if necessary.

(In reply to comment #32)
> No, but I did it to make it easier for you to accept or reject it. I have them
> in a local branch with svn. I can build larger patches if necessary.
> 

Yes, please, it'll make it easier to apply them. If you feel that the changes are unrelated, group some of them together. Sending one change per patch provides a bit of overhead, for both you and the review/applier.

Created attachment 69494 [details] [review]
make some symbols static

*** Bug 348286 has been marked as a duplicate of this bug. ***

*** Bug 348229 has been marked as a duplicate of this bug. ***

Committed to CVS, thanks!

Checking in ChangeLog;
/cvs/gnome/pygtk/ChangeLog,v  <--  ChangeLog
new revision: 1.1581; previous revision: 1.1580
done
Checking in atkrectangle.override;
/cvs/gnome/pygtk/atkrectangle.override,v  <--  atkrectangle.override
new revision: 1.2; previous revision: 1.1
done
Checking in gtk/gdk.override;
/cvs/gnome/pygtk/gtk/gdk.override,v  <--  gdk.override
new revision: 1.149; previous revision: 1.148
done
Checking in gtk/gdkevent.override;
/cvs/gnome/pygtk/gtk/gdkevent.override,v  <--  gdkevent.override
new revision: 1.4; previous revision: 1.3
done
Checking in gtk/gdkgc.override;
/cvs/gnome/pygtk/gtk/gdkgc.override,v  <--  gdkgc.override
new revision: 1.2; previous revision: 1.1
done
Checking in gtk/gdkpixbuf.override;
/cvs/gnome/pygtk/gtk/gdkpixbuf.override,v  <--  gdkpixbuf.override
new revision: 1.2; previous revision: 1.1
done
Checking in gtk/gtk-types.c;
/cvs/gnome/pygtk/gtk/gtk-types.c,v  <--  gtk-types.c
new revision: 1.58; previous revision: 1.57
done
Checking in gtk/gtkcontainer.override;
/cvs/gnome/pygtk/gtk/gtkcontainer.override,v  <--  gtkcontainer.override
new revision: 1.17; previous revision: 1.16
done
Checking in gtk/gtkmodule.c;
/cvs/gnome/pygtk/gtk/gtkmodule.c,v  <--  gtkmodule.c
new revision: 1.74; previous revision: 1.73
done
Checking in gtk/gtktextview.override;
/cvs/gnome/pygtk/gtk/gtktextview.override,v  <--  gtktextview.override
new revision: 1.19; previous revision: 1.18
done
Checking in gtk/libglade.override;
/cvs/gnome/pygtk/gtk/libglade.override,v  <--  libglade.override
new revision: 1.31; previous revision: 1.30
done

Created attachment 69964 [details] [review]
hide some symbols

pygtk is not under active development anymore and had its last code changes
in 2013. Its codebase has been archived:
https://gitlab.gnome.org/Archive/pygtk/commits/master

PyGObject at https://gitlab.gnome.org/GNOME/pygobject is its successor. See https://pygobject.readthedocs.io/en/latest/guide/porting.html for porting info.

Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect
reality. Feel free to open a task in GNOME Gitlab if the issue described in this task still applies to a recent version of PyGObject. Thanks!