Ticket #9 (closed defect: fixed)

Opened 8 months ago

Last modified 7 months ago

Post wrapping manual tweaking is needed to get gtkD to compile

Reported by: Pse Assigned to: JJR
Priority: major Milestone: Release 1.0
Component: Wrapper Version: TRUNK
Keywords: wrapper edit manual hand Cc:

Description

As it stands, gtkD needs lots of manual tweaking after running the wrapper. Lots of functions do not compile and need to be commented out, typedefs are missing, etc. I'll start listing the offending files below. It'd be nice if this could be directly handled by the wrapper.

Attachments

gtkd-tango.diff (1.4 kB) - added by Mike Wey on 12/27/07 08:55:31.
Manual edit of gtk/widget.d
gtkd-setrole.diff (1.3 kB) - added by Mike Wey on 12/27/07 08:56:52.
gtk_window_set_role is in the docs twice
manual_fixes.sh (0.8 kB) - added by Pse on 12/30/07 19:17:09.
Script to fix gtkD after a rewrap. Put it in /wrap and run it.

Change History

(follow-up: ↓ 3 ) 12/26/07 16:15:32 changed by Pse

gtkc/gdk.d: almost all gdk.X11 functions need to be commented out. Only XID function(GdkDrawable??* drawable)gdk_x11_drawable_get_xid; can be left in. These changes have to be mirrored in the Symbols section at the end of the file.

gdk/X11.d: Needs to mirror the changes made to gtkc/gdk.d.

(follow-up: ↓ 4 ) 12/26/07 17:21:24 changed by JJR

This is an important ticket. I've been going through the gtkD source and noticed many instances that need fixing.

  • wrappers are inserting class local imports
  • wrappers are inserting redundant imports
  • import system needs stream-lining

I manually repaired a number of these issues in the branches version. I was almost to the point of wondering if maintaining the wrapper was worth the effort because the questionable results. We'll see, I guess.

Concerning gdk.X11. Why does it have to be commented out? Is it not possible to insert a version statement within gtkc/gdk.d? I think this is how I did it with an old version of the freetype loader.

(in reply to: ↑ 1 ) 12/27/07 08:54:28 changed by Mike Wey

I've also noticed a few things that need manual editing.

a diff between Preview 7 and the generated code revealed a few manual edits in gtk/Widget.d for tango support. Also in the 2.12.3 'gtk_window_set_role' appears twice in the documentation. (Witch version is currently used ?)

I'll Attach them as diff's

12/27/07 08:55:31 changed by Mike Wey

  • attachment gtkd-tango.diff added.

Manual edit of gtk/widget.d

12/27/07 08:56:52 changed by Mike Wey

  • attachment gtkd-setrole.diff added.

gtk_window_set_role is in the docs twice

(in reply to: ↑ 2 ) 12/28/07 14:35:57 changed by Pse

Replying to JJR:

I was almost to the point of wondering if maintaining the wrapper was worth the effort because the questionable results. We'll see, I guess.

I've begun wondering this myself.

12/29/07 01:31:06 changed by Pse

  • version set to TRUNK.

12/29/07 11:42:41 changed by JJR

Okay, so what's the consensus about the wrappers? What I'm seeing is that the wrappers have done their service by getting the bulk of the Gtk+ interface converted to D. That was the hardest part, nor would that have been easy or realistic to do by hand. But now I think GtkD needs to be free of that so that cleanup and improvement can take place.

What we really need now is to keep track of the differences between the C Gtk+ updates, to find out which interfaces or types are being added to the more recent versions... we really shouldn't have to convert the whole package each time and then wrangle with all the problems that come from that. This way we just have to add (or remove, if symbols are deprecated) those changes from GtkD that have been updated in Gtk+. At this point, whether this is done manually or automatically should not be a big deal because I'm guessing the changes to Gtk+ aren't voluminous.

This will allow us to make specific additions and changes to GtkD as needed. This also allows us to improve those aspects of GtkD that would rarely get updated. We may also be able to apply specific scripts to GtkD to fix specific aspects of the project (like adding types of signal/slots or reworking constructors with exceptions).

I think it's time to move away from the wrappers... at least in that specific application of them and figure out a new way to manage updates incrementally that is more palatable to GtkD management.

Opinions?

(follow-up: ↓ 9 ) 12/29/07 12:52:14 changed by Pse

The problem right now is that I don't think gtkD has wrapped most of GTK+ yet. In fact, DUI remains listed in gtk.org as having "support" only for GTK-2.4 (but that's probably outdated, anyway). I think we should make the most out of the wrapper before moving away from it.

In any case, this is a major design decision and should be thoroughly discussed. It'd probably be wise to start a thread in the forums, or even arrange an online meeting (IRC?) to discuss this and other matters with those who are interested. I also think we should wait at least till we get pre-release 8 ready before going more in-depth about this. There are already too many things going on with gtkD and talking about big changes prematurely may end up being negative.

The wrapper really is a huge piece of work, and a lot of effort and thought has gone into it, we should make the most out of it while we can.

12/29/07 14:03:27 changed by JJR

Agreed.

Looking at my post again, I do see it looked like I was promoting a major change right away. Oops. :) My problem is that I have a fairly big distaste for wrapper systems because the output seems to be so unsightly (not to say they aren't extremely useful, though). I just wish we could have the code look clean and well organized.

All the doc comments weigh it down too in a rather ugly manner. If it's possible to wrap code, it seems to me that the same process should be able to separate the (excessive) documentation comments (that do little to help code readability, which is the original intention of code comments) from the wrapped code. To me the separation of the two just makes sense. Ok that's my little rant... now I feel better. :)

We'll keep this one on the table for now. Later we may want to take it into consideration.

Concerning IRC. If people are interested, it may be a good idea to start an IRC channel on Freenode at some point as Pse suggests?

(in reply to: ↑ 7 ) 12/29/07 14:05:44 changed by Mike Wey

Keeping the wrapper script around would probably depend on the time spend on fixing the generated code vs the time of keeping everything updated with the latest release.

for gtk we are missing 25 classes/interfaces. i got 7 of those in my local APILookupGtk.txt waiting for testing. with the wrapper it probably takes a lot less time to add those 25.

12/29/07 14:11:51 changed by JJR

Actually, I'm not completely depecrating wrapping/scripts, I'm advocating a modification of the process using "diffs" so-to-speak. It's time consuming and maybe somewhat error prone to re-wrap everything from the start when just changes can be addded/removed.

This of course still means redesign of the wrapping system. Just something to keep in mind as a future possibility.

12/29/07 14:22:44 changed by Pse

Oh, I know what you mean, JJR, and yes, it really is bothersome. Not easy to fix though.

To be honest, it's not like the generated code is completely broken and requires major tweaking to get it to compile. Our biggest problem right now are the definitions listed in those two files I mentioned in my first post. It seems the reason they don't compile is they use definitions from X11's headers (VisualID, Colormap, etc). In fact, there's a notice about this in APILookupGdk.txt, line 914. If we added the missing definitions to the right APILookup files they should work. Other than that, the wrapper should mostly work (minor edits here and there are necessary, I'm working on getting these sorted automatically).

12/29/07 15:12:06 changed by JJR

That's good to know that the wrapper output doesn't require major fixes.

One more thing I want to point out, though, is this (and I guess you already know this, so I'm likely not making much of a point): the fact that it's compiling doesn't mean that there aren't things being "overlooked" by the compiler. I manually went through many of these modules and found redundant imports and class-local imports spread all over the place. That's part of the problem that doesn't necessarily have any ill effects at the moment... not yet. (on the other hand, it might be a time-bomb).

Concerning some of the system specific gtk+ symbols... I also noticed that the loader tries to load some of the win32 specific gtk symbols on linux. Like we discussed above, I guess we have to determine if these OS specific symbols are necessary on each OS for use with GtkD. They either need to be removed, if they are not needed, or versioned, if they are needed. I guess this has to be added to the wrapper system as well.

Honestly, I haven't played with the wrapper yet. I looked at it several times, including some of its internals, and found it difficult to follow and understand... and thus completely unappealing to maintain or improve.

I'm glad you guys are here to fiddle with it. :)

12/29/07 16:53:17 changed by Pse

Duplicated declarations are now automatically omitted by the wrapper (r344). gtkd-setrole.diff is no longer needed. That leaves us with gtkc/gdk.d and gdk/X11.d.

12/29/07 16:59:07 changed by JJR

Good fix, pse... Looks like it wasn't as hard as I thought.

12/29/07 17:54:50 changed by Mike Wey

gtkd-tango.diff is no longer needed in r346. svn builds with Tango now.

12/30/07 19:17:09 changed by Pse

  • attachment manual_fixes.sh added.

Script to fix gtkD after a rewrap. Put it in /wrap and run it.

12/30/07 19:18:59 changed by Pse

I've attached a temporary fix for gdk/X11.d and gtkc/gdk.d. Run it after a rewrap.

12/31/07 02:17:30 changed by JJR

Okay, I've finally delved into the wrappers. Not pretty, but managed to figure some of it out. Thankfully, it's easy to run a rewrap (something I didn't realize), so things are not so doom and gloom with the wrappers as I made out above :).

My first update to the wrappers is in r362. Imports related to Signals are now added outside the class instead of inside. It looks like all the class-local imports have been successfully moved out of the classes. We're making some headway. :)

01/01/08 21:41:06 changed by kaarna

Heh. I strongly disagree with ever getting rid of the wrapper. The wrapper should just be made better. Gtkmm uses a wrapper program and the Java gtk+ bindings use one too (nowadays). The java version didn't use one earlier, and it really had an impact on their completeness and quality. A wrapper will quarantee that large design desicions can be made automatically to the whole project and in a consistent manner. The wrapper is gtkD.

I'm to blame for the X11 stuff. The wrapper worked fine before I needed the XID method. You could just easily comment out the lines that wrap the X11 functions from APIlookupGdk.txt. That's the way it was before. Or then you could just add the VisualID, Colormap etc. there correctly. That's propably all that's needed for the X11 functions to wrap correctly. There's nothing wrong with the wrapper there, it's just the wrapper script (APIlookupGdk.txt) that hasn't been correctly written on the X11 part.

I think using the wrapping program should be made easier, and that it should be made so that no manual editing whatsoever is needed. Then you could just run the wrapper and make changes to the wrapper and/or the wrapper scripts to make changes to gtkD. That's the way it's designed to be. Me and other's have just broken it, when not knowing how to use it (lack of documentation on how to make APIlookup scripts), and wanting to get stuff working quickly, rather than the way they're supposed to be.

Don't leave the wrapper! The wrapper is your friend. We just need to make it better, clearer, to compile with Tango, easier to use, better documented.

01/01/08 21:49:07 changed by Pse

Right now the wrapper mostly works out-of-the-box. I intend to add the missing X11 struct definitions eventually, so we don't need any external scripts after a wrap. It does need some love on the inside, though.

01/01/08 23:03:24 changed by JJR

Kaarna, I completely agree with you about the wrapper. My desire to abondon it initial was due to frustration: the wrapper has been poorly maintained and hardly doucmented at all. The code is rather like a deluge from a very prolific programmer that had little time to explain what he was doing. :)

The wrapper is a keeper. I've managed to figure out pieces of it, but I appreciate that we will need to rely on it in order to keep gtkD moving ahead. I just think it needs plenty of emphasis and tender loving care for the next while. We should treat it like it's our friend. :)

01/19/08 07:57:35 changed by Mike Wey

  • status changed from new to closed.
  • resolution set to fixed.

r409 adds some changes to APILookupGdk.txt so the manual tweaking isn't needed anymore. It does this by changing nocode to noprefix so no external declerations is added to gtkc/gdk.d for those funcions.