GNOME Bugzilla – Bug 576595
Please add gobject-introspection support
Last modified: 2010-05-03 21:29:33 UTC
libsoup should build and install the .gir and .typelib files generated and used by gobject-introspection.
Created attachment 131277 [details] [review] Add gobject-introspection support - Detect gobject-introspection (g-i) in configure, using the macro from introspection.m4 as installed by g-i; this adds g-i as a autogen.sh-time dependency, and as an *optional* build dependency - Integrate the generation of .gir and .typelib files into the build build process - Add the annotations from gir-repository to the source code
as discussed on IRC, I've updated the patch to copy introspection.m4 into the sources rather than requiring an installed copy. What was your intent with regards to libsoup stable vs unstable? Should this go into 2.26.1? I feel like if we're going to do that we should make it default to not building introspection, and require an explicit --enable-introspection, just to make sure distros don't accidentally end up shipping introspection files in packages.
(In reply to comment #2) > as discussed on IRC, I've updated the patch to copy introspection.m4 into the > sources rather than requiring an installed copy. > Fine. I've also done a rebase against current git HEAD, and included introspection.m4 in the patch. > What was your intent with regards to libsoup stable vs unstable? Should this go > into 2.26.1? I feel like if we're going to do that we should make it default to > not building introspection, and require an explicit --enable-introspection, > just to make sure distros don't accidentally end up shipping introspection > files in packages. > I'd be OK if this would only end up in the development releases.
Created attachment 133038 [details] [review] Enable the build of gobject-introspection support Rebased, and include introspection.m4.
Created attachment 133039 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable
Created attachment 135808 [details] [review] Enable the build of gobject-introspection support Rebased against dfdde36f28e3092c6ac20ecae3774903be92a153; added SoupGNOME typelib, several tweaks.
Created attachment 135809 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable Rebased.
Created attachment 136157 [details] [review] Some generic build fixes I've split out some generic build fixes from the g-i patch, so they can be applied on their own merits.
Created attachment 136159 [details] [review] Enable the build of gobject-introspection support Rebased against ad981db8305fe9a19c9a2b1e16675f512fa0cac7, slight annotation updates, generic build fixes split out, build robustness improvements.
OK, I haven't looked in detail at the annotations, but we can continue to adjust those after this lands. m4/introspection.m4 needs to be disted in the tarball, right? (Should we also dist the m4 files libtool and gtk-doc are dumping in there?) Need to update .gitignore for m4/ and for the new generated files too Where is annotation-glossary.xml supposed to come from? It's not finding that for me.
(In reply to comment #10) > OK, I haven't looked in detail at the annotations, but we can continue to > adjust those after this lands. > OK. > m4/introspection.m4 needs to be disted in the tarball, right? (Should we also > dist the m4 files libtool and gtk-doc are dumping in there?) > Yes, I missed this. > Need to update .gitignore for m4/ and for the new generated files too > Will do. > Where is annotation-glossary.xml supposed to come from? It's not finding that > for me. > The development version of gtk-doc creates it -- does the build fail when using a stable release of gtk-doc?
(In reply to comment #11) > (In reply to comment #10) > > m4/introspection.m4 needs to be disted in the tarball, right? (Should we also > > dist the m4 files libtool and gtk-doc are dumping in there?) > > > Yes, I missed this. > In fact, I didn't -- it seems automake does that, well, automagically: % tar tf libsoup-2.27.2.tar.gz | grep '\.m4$' libsoup-2.27.2/m4/ltoptions.m4 libsoup-2.27.2/m4/libtool.m4 libsoup-2.27.2/m4/gtk-doc.m4 libsoup-2.27.2/m4/introspection.m4 libsoup-2.27.2/m4/ltsugar.m4 libsoup-2.27.2/m4/ltversion.m4 libsoup-2.27.2/m4/lt~obsolete.m4 libsoup-2.27.2/aclocal.m4 That's with automake 1.10.2.
committed acinclude.m4 removal for bug 589033
Review of attachment 136159 [details] [review]: This patch should be updated to follow the conventions listed here: http://live.gnome.org/GObjectIntrospection/AutotoolsIntegration
btw, i'm planning to commit this soon after gtk-doc 1.12 is released (which is supposed to be later this month). gtk-doc 1.11 messes up the docs when there are annotations.
ping, as gtk-doc 1.12 is released now :)
Created attachment 151600 [details] [review] Enable the build of gobject-introspection support This version of the patch is updated according to the wiki page gobject-instrospection page, and rebased against 2.29.5 (01141aaf699ad0df...).
Created attachment 154104 [details] [review] Bug 576595 - Please add gobject-introspection support - Detect gobject-introspection (g-i) in configure, using the M4 macro from the newly-added introspection.m4. This adds gobject-introspection as a new *optional* build dependency. - Integrate the generation of .gir and .typelib files into the build build process. - Add the annotations from gir-repository to the source code. - Include annotation glossary in "Reference Manual" main file. - Remove acinclude.m4, which just hosted a stale copy of GTK_DOC_CHECK, which in turn broke "make distcheck" when used in combination with the gtk-doc.make installed by newer gtkdocize. - Fix #include in soup-path-map.h.
Rebased the patch for master (.in->.ac change). o committed @151600 - Enable the build of gobject-introspection support - none
Created attachment 154122 [details] [review] Enable the build of gobject-introspection support My take on a rebase (against 433fd0c645a6e9aad..); Diego missed the following change to configure.ac, I think: --- a/configure.ac +++ b/configure.ac @@ -206,6 +206,10 @@ dnl *** gtk-doc *** dnl *************** GTK_DOC_CHECK([1.0]) +dnl ***************************** +dnl *** gobject-introspection *** +dnl ***************************** +GOBJECT_INTROSPECTION_CHECK([0.6.7]) dnl ************************************* dnl *** Warnings to show if using GCC ***
It's a shame this doesn't appear to have made it into the 2.30 release, since it currently means that from a packager's point of view, whilst most of the 2.30 packages accept --enable-introspection, libsoup requires the additional gir-repository package. Can I ask what's still left to do?
Created attachment 158139 [details] [review] Enable the build of gobject-introspection support
Created attachment 158140 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable Rebase against 50e363db02 + g-i.diff.
Comment on attachment 158140 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable >- Annotate `soup_message_body_append_buffer' with "Rename to: > soup_message_body_append", as it is not bindable. forcing callers to create a SoupBuffer first is a nuisance. It would be better to add soup_message_body_append_take() and rename that, and leave append_buffer() as-is. >+ * @data: (type pointer): the data why is this not "(type guchar)"? (or uint8 or whatever) >+ * convinience and simplifying language bindings. typo ("convenience") The char*/guint8*/pointer thing is one of the problems I haven't figured out yet; the buffers are technically guint8*, but most of the time you're going to want to treat them as char*. Often UTF-8 char*, but not necessarily...
Comment on attachment 158139 [details] [review] Enable the build of gobject-introspection support >Subject: [PATCH] Bug 576595 - Please add gobject-introspection support can you change that to just "add gobject-introspection support"? (No "Please" and no bug number). And then add the bugzilla URL to the end of the commit, like the other patch. >- Remove acinclude.m4, which just hosted a stale copy of > GTK_DOC_CHECK, which in turn broke "make distcheck" when used in > combination with the gtk-doc.make installed by newer gtkdocize. > >- Fix #include in soup-path-map.h. remove these from the commit message since they were committed a while back. >+ <xi:include href="xml/annotation-glossary.xml"> >+ <xi:fallback /> >+ </xi:include> put this at the very end, after the index >+gi_soup_files = $(addprefix $(srcdir)/,\ >+ $(filter-out soup.h soup-marshal.% soup-enum-types.%,\ >+ $(soup_headers) $(libsoup_2_4_la_SOURCES))) \ >+ soup-enum-types.h the soup-enum-types.h needs to be inside the addprefix. As it is now it fails "make distcheck". >- * @cancellable: a #GCancellable object, or %NULL >+ * @cancellable: (allow-none): a #GCancellable object, or %NULL GCancellables are automatically allow-none now, so you don't need to mark them >+ * Return value: (element-type utf8): the list of paths, which must be freed with > * soup_auth_free_protection_space(). change "must" to "can", and annotate soup_auth_free_protection_space() with "(skip)" > * soup_cookies_free: >- * @cookies: a #GSList of #SoupCookie >+ * @cookies: (element-type SoupCookie): a #GSList of #SoupCookie likewise, you can just annotate this (skip) since it's just a C convenience method >+ * @callback: (allow-none): a #SoupSessionCallback which will be called after the > * message completes or when an unrecoverable error occurs. should have "(scope async)" as well. as should the callbacks to soup_address_resolve_async() and soup_socket_connect_async().
Created attachment 159102 [details] [review] Enable the build of gobject-introspection support I think I applied all of Dan's suggestions for this patch.
Comment on attachment 159102 [details] [review] Enable the build of gobject-introspection support >+Soup_2_4_gir_FILES = \ >+ $(addprefix $(srcdir)/, $(gi_soup_files)) \ >+ $(foreach f,$(gi_built_soup_files), \ >+ $(if $(shell test -f $(addprefix $(srcdir)/,$(f)) && echo yes), \ >+ $(addprefix $(srcdir)/,$(f)), \ >+ $(f))) lovely After seeing this, I thought, "this can't possibly be right", and after poking around some, I figured out the right fix (bug 616425), but that will require a new gobject-introspection release, so we still need your hack for now.
Created attachment 159599 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable Updated regarding to Dan's suggestions.
Comment on attachment 159599 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable > * SoupBuffer: >- * @data: the data >+ * @data: (type pointer): the data I don't think you explained this one. It should be (type uint8) shouldn't it? >+ * This function is exactly equivalent to #soup_buffer_new() with >+ * #SOUP_MEMORY_TAKE as first argument; it exists mainly for "#" is used only before type names. So just remove it before "soup_buffer_new()" (which gets recognized as a function because of the "()"s) and use "%" before SOUP_MEMORY_TAKE. >+ * Return value: the new #SoupBuffer. >+ * Rename to: soup_buffer_new add a blank line (well, blank-except-for-an-asterisk) and a "Since: 2.32" between those two lines. >+soup_buffer_new_take (gconstpointer data, gsize length) @data is not const, it's (transfer full). And if this is primarily for bindings, then you should make the parameter types "correct", rather than making them generic and fixing them up in the annotations. So "guchar *data", and remove the now-redundant parts of its annotation. >+ * Appends @length bytes from @data to @body according to @use. remove reference to "@use" >+ * This function is exactly equivalent to #soup_message_body_apppend() >+ * with #SOUP_MEMORY_TAKE as second argument; it exists mainly for same as above >+ * Rename to: soup_message_body_append put "Since: 2.32" before >+ gpointer data, gsize length) as above, "guchar *data" > * operation that doesn't actually require copying the data in > * @buffer.) >+ * > **/ no need to add that blank line there
(In reply to comment #29) > (From update of attachment 159599 [details] [review]) > > * SoupBuffer: > >- * @data: the data > >+ * @data: (type pointer): the data > > I don't think you explained this one. It should be (type uint8) shouldn't it? > No, it /should/ be (array length=length) (element-type uint8), but the gobject-introspection toolchain cannot handle that (for struct members, as opposed to parameters) yet. So `(type pointer)' is the best that's currently possible -- if not for that annotation, it would be mistaken for a string, which is clearly wrong. > "#" is used only before type names. So just remove it before > "soup_buffer_new()" (which gets recognized as a function because of the "()"s) > and use "%" before SOUP_MEMORY_TAKE. > > [...] > I have adjusted the patch according to your comments now. Thanks for the quick review.
Created attachment 159633 [details] [review] Make `soup_buffer_new' and `soup_message_body_append' bindable Applied suggestions from comment #29.
Created attachment 160093 [details] [review] Add some helper functions to aid language bindings I now added `soup_buffer_get_data', as discussed on IRC.
Comment on attachment 160093 [details] [review] Add some helper functions to aid language bindings great, just one suggestion: > /** >+ * soup_buffer_get_data: >+ * @buffer: >+ * @data: (out) (array length=length) (transfer none): the pointer >+ * to the buffer data is stored here >+ * @length: (out): the length of the buffer data is stored here >+ * >+ * Provides access to the buffer data. This function primarily exists >+ * for use by language bindings. >+ */ How about: This function exists for use by language bindings, because it's not currently possible to get the right effect by annotating the fields of #SoupBuffer. Also, "Since: 2.32"
(In reply to comment #33) > >+ * @buffer: oh, and "@buffer: a #SoupBuffer"
Created attachment 160227 [details] [review] Add some helper functions to aid language bindings Adjusted docs for `soup_buffer_get_data' as suggested by Dan.
As I just pushed the last patch, this bug has been dealt with.