Created attachment 13525 [details] [review]
minor changes for registering boxed types

I'd love to have python bindings for libgnomeprintui. But the patch
needs a bit of work.

-SUBDIRS = gpa transports modules
+SUBDIRS = gpa . transports modules

Why do you want to build "." before transports & modules? You don't
need to.

+GType gnome_glyphlist_get_type(void)
+{
+	static GType type = 0;

Is not the coding style of gnome-print. Should be:
Gtype
gnome_glyphlist_get_type (void)
{
 ....

+	if (type == 0)
+		type = g_boxed_type_register_static
+			("GnomeGlyphList",


Should be:
                       
+	if (type == 0) {
                      ^^^^

This:

+ * I changed GnomePrintConfig form being a boxed type to a GObject,
+ * because app developers are going to g_object_unref anyways  Move the

comment goes away since it has already been moved to the .h file.


For:

+static GnomePrintUnit*
+gnome_print_unit_copy (GnomePrintUnit *unit)
+{
+	  /* units cannot be changed and are statically allocated, so
+	   * no copying is necessary */
+	return unit;
+}

Shouldn't the patch chanegd from:

+		type = g_boxed_type_register_static
+			("GnomePrintUnit",
+			 (GBoxedCopyFunc) gnome_print_unit_copy,
+			 (GBoxedFreeFunc) gnome_print_unit_free);
+	return type;

To:

+		type = g_boxed_type_register_static
+			("GnomePrintUnit",
+			 (GBoxedCopyFunc) NULL,
+			 (GBoxedFreeFunc) NULL);
+	return type;

?

What is G_GNUC_CONST here used for?

+GType                 gnome_print_unit_get_type (void) G_GNUC_CONST;


The part inside drivers is not needed, since drivers is not built
anymore. 

You don't need this:
 
-libgnomeprint_file_la_LIBADD =
+libgnomeprint_file_la_LIBADD = ../libgnomeprint-2-2.la
 
(and the other modules)

I will get to the patch during this week to fix the issues, but if you
need it before I can get to it, you can provide an updated patch.

thanks!
 

About these changes:

-SUBDIRS = gpa transports modules
+SUBDIRS = gpa . transports modules

-libgnomeprint_file_la_LIBADD =
+libgnomeprint_file_la_LIBADD = ../libgnomeprint-2-2.la

I *do* need these changes. You see, Python loads all modules in a way
that doesn't make symbols from such modules global, ie. available for
future modules. See the flag RTLD_GLOBAL in dlopen(1). Also see
jdahlin's comments in bug #102226, particularly this one:
   "3) Remove the linker hacks in __init__.py, It's the wrong way of
solving the problem. Make sure libgnomeprint[ui] (both the libraries
and bindings) are linked against all libraries they use symbols from
instead."

  Regarding all other issues, I don't disagree with you. If you don't
mind fixing the remaining problems in this patch, I don't mind waiting
another week. If, however, you don't have enough time, I can do it
myself, no problem.

I'm not convinced about the change to link the modules with the .la. 

For example, I don't see the pango modules linking to pango's .la
library. Also, there aren't any symbols inside the modules that you
need to write bindings for, or that the python app should need to call
directly.

Created attachment 13548 [details] [review]
what went into cvs, Jan 13th

I fixed all of the issues except the one we are still discussing.

Also...

Have you talked to James Henstridge about this? I know he was going
some python bindings for libgnomeprint/ui some time ago.

http://bugzilla.gnome.org/show_bug.cgi?id=93271

Created attachment 13550 [details] [review]
second part of the fix

Fixed in cvs, let me know if you need anything else.

Looks like there is one minor problem with what went into CVS.  The
registration for the GnomePrintUnit type passes NULL as the copy/free
functions, which is an error.  If that type does not require copying
or freeing, then dummy copy/free funcs are still needed.  Something like:
  GnomePrintUnit *
  unit_copy(GnomePrintUnit *unit)
  {
      return unit;
  }
  void
  unit_free(GnomePrintUnit *unit)
  {
      /* nothing */
  }

Created attachment 13568 [details] [review]
fix

Fixed in cvs.