After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 576595 - Please add gobject-introspection support
Please add gobject-introspection support
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: Misc
unspecified
Other All
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on: 615550
Blocks: 585444
 
 
Reported: 2009-03-24 17:20 UTC by Andreas Rottmann
Modified: 2010-05-03 21:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add gobject-introspection support (19.19 KB, patch)
2009-03-24 17:20 UTC, Andreas Rottmann
none Details | Review
Enable the build of gobject-introspection support (22.71 KB, patch)
2009-04-21 13:29 UTC, Andreas Rottmann
none Details | Review
Make `soup_buffer_new' and `soup_message_body_append' bindable (2.42 KB, patch)
2009-04-21 13:30 UTC, Andreas Rottmann
none Details | Review
Enable the build of gobject-introspection support (25.75 KB, patch)
2009-06-02 13:50 UTC, Andreas Rottmann
none Details | Review
Make `soup_buffer_new' and `soup_message_body_append' bindable (2.42 KB, patch)
2009-06-02 13:51 UTC, Andreas Rottmann
none Details | Review
Some generic build fixes (2.43 KB, patch)
2009-06-08 18:00 UTC, Andreas Rottmann
committed Details | Review
Enable the build of gobject-introspection support (27.04 KB, patch)
2009-06-08 18:04 UTC, Andreas Rottmann
needs-work Details | Review
Enable the build of gobject-introspection support (27.64 KB, patch)
2010-01-17 15:07 UTC, Andreas Rottmann
none Details | Review
Bug 576595 - Please add gobject-introspection support (26.99 KB, patch)
2010-02-18 04:43 UTC, Diego Escalante Urrelo (not reading bugmail)
none Details | Review
Enable the build of gobject-introspection support (27.36 KB, patch)
2010-02-18 11:57 UTC, Andreas Rottmann
none Details | Review
Enable the build of gobject-introspection support (27.36 KB, patch)
2010-04-07 16:53 UTC, Andreas Rottmann
needs-work Details | Review
Make `soup_buffer_new' and `soup_message_body_append' bindable (2.61 KB, patch)
2010-04-07 16:55 UTC, Andreas Rottmann
needs-work Details | Review
Enable the build of gobject-introspection support (27.70 KB, patch)
2010-04-19 19:13 UTC, Andreas Rottmann
accepted-commit_now Details | Review
Make `soup_buffer_new' and `soup_message_body_append' bindable (3.97 KB, patch)
2010-04-26 14:03 UTC, Andreas Rottmann
none Details | Review
Make `soup_buffer_new' and `soup_message_body_append' bindable (3.62 KB, patch)
2010-04-26 19:07 UTC, Andreas Rottmann
none Details | Review
Add some helper functions to aid language bindings (4.80 KB, patch)
2010-05-01 11:14 UTC, Andreas Rottmann
accepted-commit_now Details | Review
Add some helper functions to aid language bindings (4.89 KB, patch)
2010-05-03 20:46 UTC, Andreas Rottmann
none Details | Review

Description Andreas Rottmann 2009-03-24 17:20:06 UTC
libsoup should build and install the .gir and .typelib files generated
and used by gobject-introspection.
Comment 1 Andreas Rottmann 2009-03-24 17:20:10 UTC
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
Comment 2 Dan Winship 2009-03-28 12:58:10 UTC
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.
Comment 3 Andreas Rottmann 2009-04-21 13:26:31 UTC
(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.
Comment 4 Andreas Rottmann 2009-04-21 13:29:31 UTC
Created attachment 133038 [details] [review]
Enable the build of gobject-introspection support

Rebased, and include introspection.m4.
Comment 5 Andreas Rottmann 2009-04-21 13:30:57 UTC
Created attachment 133039 [details] [review]
Make `soup_buffer_new' and `soup_message_body_append' bindable
Comment 6 Andreas Rottmann 2009-06-02 13:50:12 UTC
Created attachment 135808 [details] [review]
Enable the build of gobject-introspection support

Rebased against dfdde36f28e3092c6ac20ecae3774903be92a153; added SoupGNOME typelib, several tweaks.
Comment 7 Andreas Rottmann 2009-06-02 13:51:29 UTC
Created attachment 135809 [details] [review]
Make `soup_buffer_new' and `soup_message_body_append' bindable

Rebased.
Comment 8 Andreas Rottmann 2009-06-08 18:00:24 UTC
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.
Comment 9 Andreas Rottmann 2009-06-08 18:04:03 UTC
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.
Comment 10 Dan Winship 2009-06-23 23:36:46 UTC
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.
Comment 11 Andreas Rottmann 2009-06-27 11:49:39 UTC
(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?
Comment 12 Andreas Rottmann 2009-06-27 11:58:50 UTC
(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.
Comment 13 Dan Winship 2009-07-23 19:54:06 UTC
committed acinclude.m4 removal for bug 589033
Comment 14 Johan (not receiving bugmail) Dahlin 2009-12-17 13:27:39 UTC
Review of attachment 136159 [details] [review]:

This patch should be updated to follow the conventions listed here:
http://live.gnome.org/GObjectIntrospection/AutotoolsIntegration
Comment 15 Dan Winship 2009-12-17 14:45:43 UTC
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.
Comment 16 Javier Jardón (IRC: jjardon) 2009-12-22 01:05:24 UTC
ping, as gtk-doc 1.12 is released now :)
Comment 17 Andreas Rottmann 2010-01-17 15:07:19 UTC
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...).
Comment 18 Diego Escalante Urrelo (not reading bugmail) 2010-02-18 04:43:17 UTC
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.
Comment 19 Diego Escalante Urrelo (not reading bugmail) 2010-02-18 04:44:05 UTC
Rebased the patch for master (.in->.ac change).


o committed @151600 - Enable the build of gobject-introspection support - none
Comment 20 Andreas Rottmann 2010-02-18 11:57:20 UTC
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 ***
Comment 21 Mike Auty 2010-04-05 02:49:41 UTC
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?
Comment 22 Andreas Rottmann 2010-04-07 16:53:40 UTC
Created attachment 158139 [details] [review]
Enable the build of gobject-introspection support
Comment 23 Andreas Rottmann 2010-04-07 16:55:00 UTC
Created attachment 158140 [details] [review]
Make `soup_buffer_new' and `soup_message_body_append' bindable

Rebase against 50e363db02 + g-i.diff.
Comment 24 Dan Winship 2010-04-11 14:03:51 UTC
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 25 Dan Winship 2010-04-11 14:40:30 UTC
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().
Comment 26 Andreas Rottmann 2010-04-19 19:13:43 UTC
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 27 Dan Winship 2010-04-21 19:30:35 UTC
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.
Comment 28 Andreas Rottmann 2010-04-26 14:03:10 UTC
Created attachment 159599 [details] [review]
Make `soup_buffer_new' and `soup_message_body_append' bindable

Updated regarding to Dan's suggestions.
Comment 29 Dan Winship 2010-04-26 14:14:40 UTC
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
Comment 30 Andreas Rottmann 2010-04-26 19:03:22 UTC
(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.
Comment 31 Andreas Rottmann 2010-04-26 19:07:56 UTC
Created attachment 159633 [details] [review]
Make `soup_buffer_new' and `soup_message_body_append' bindable

Applied suggestions from comment #29.
Comment 32 Andreas Rottmann 2010-05-01 11:14:16 UTC
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 33 Dan Winship 2010-05-03 20:24:14 UTC
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"
Comment 34 Dan Winship 2010-05-03 20:24:40 UTC
(In reply to comment #33)
> >+ * @buffer:

oh, and "@buffer: a #SoupBuffer"
Comment 35 Andreas Rottmann 2010-05-03 20:46:21 UTC
Created attachment 160227 [details] [review]
Add some helper functions to aid language bindings

Adjusted docs for `soup_buffer_get_data' as suggested by Dan.
Comment 36 Andreas Rottmann 2010-05-03 21:29:33 UTC
As I just pushed the last patch, this bug has been dealt with.