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 727329 - check: souphttpsrc: unknown type name ‘SoupStatus’
check: souphttpsrc: unknown type name ‘SoupStatus’
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal major
: 1.2.4
Assigned To: Reynaldo H. Verdejo Pinochet
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-30 07:50 UTC by Edward Hervey
Modified: 2014-05-09 15:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Workaround SoupKnownStatusCode rename in 2.44 (1.89 KB, patch)
2014-04-06 22:47 UTC, Reynaldo H. Verdejo Pinochet
reviewed Details | Review
Updated solution (1.89 KB, patch)
2014-04-07 17:39 UTC, Reynaldo H. Verdejo Pinochet
none Details | Review
Updated solution (1.10 KB, patch)
2014-04-07 21:06 UTC, Reynaldo H. Verdejo Pinochet
committed Details | Review
tests: fix compilation of souphttpsrc test for libsoup 2.40 (923 bytes, patch)
2014-05-07 12:25 UTC, Tim-Philipp Müller
none Details | Review
tests: fix compilation of souphttpsrc test for libsoup 2.40 (1.25 KB, patch)
2014-05-07 12:28 UTC, Tim-Philipp Müller
committed Details | Review

Description Edward Hervey 2014-03-30 07:50:49 UTC
the souphttpsrc fails:


elements/souphttpsrc.c: In function ‘do_get’:
elements/souphttpsrc.c:506:3: error: unknown type name ‘SoupStatus’
elements/souphttpsrc.c:533:18: error: ‘SoupStatus’ undeclared (first use in this function)
elements/souphttpsrc.c:533:18: note: each undeclared identifier is reported only once for each function it appears in
elements/souphttpsrc.c:533:30: error: expected ‘)’ before ‘SOUP_STATUS_OK’
elements/souphttpsrc.c:501:12: error: variable ‘send_error_doc’ set but not used [-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors
make[3]: *** [elements/elements_souphttpsrc-souphttpsrc.o] Error 1
make[3]: *** Waiting for unfinished jobs....

Looks like SoupStatus is a new symbol (yay, it's even better.. it's a renamed symbol, ALL THE API BREAKS o/).
Comment 1 Tim-Philipp Müller 2014-03-30 10:32:55 UTC
Sorry, could you provide more context, such as the libsoup version you're compiling against? I'm guessing you're using libsoup < 2.44 ?
Comment 2 Edward Hervey 2014-03-30 13:36:19 UTC
# pkg-config --modversion libsoup-2.4
2.40.3


We don't require >= 2.44 in the configure.ac afaics.
Comment 3 Reynaldo H. Verdejo Pinochet 2014-04-06 22:47:01 UTC
Created attachment 273675 [details] [review]
Workaround SoupKnownStatusCode rename in 2.44

Till there's an actual reason to require 2.44
(I don't see any obvious one now) I guess we
can get around the rename by aliasing it. Docs
claim this should be safe enough for SoupStatus.
Comment 4 Sebastian Dröge (slomo) 2014-04-07 07:47:36 UTC
Review of attachment 273675 [details] [review]:

::: tests/check/elements/souphttpsrc.c
@@ +42,3 @@
+#define MY_SOUP_STATUS_TYPE SoupKnownStatusCode
+#endif
+#endif

Why so complicated?

SOUP_CHECK_VERSION() should be there since forever so you can just use it, and just write then

#if SOUP_CHECK_VERSION()
#define ...
#else
#define ...
#endif

Alternatively as this is just a test, what about just storing it as int?
Comment 5 Tim-Philipp Müller 2014-04-07 09:07:00 UTC
Also, a simple single define SoupStatus whatever should do, no need for MY_SOUP_STATUS_TYPE
Comment 6 Reynaldo H. Verdejo Pinochet 2014-04-07 17:39:32 UTC
Created attachment 273733 [details] [review]
Updated solution

Sebastian: the test over SOUP_CHECK_VERSION()
was just out of been used to do it for third party
provided macros. Whatever the case, I don't rly mind
so I dropped them.

Tim: Would like to keep the MY_.. alias as its makes
clear this is being defined somewhere else and avoids
directly messing with the type. I don't particularly
fancy either option though so if you are strong about
this I can drop it too. Just let me know.
Comment 7 Tim-Philipp Müller 2014-04-07 17:52:52 UTC
Yes, please drop the custom type. We want the smallest, least intrusive fix to make it work with the old version. The /* XXX - drop this .. */ comment is also superfluous.
Comment 8 Reynaldo H. Verdejo Pinochet 2014-04-07 21:06:35 UTC
Created attachment 273746 [details] [review]
Updated solution

Tim: As requested.
Comment 9 Tim-Philipp Müller 2014-04-07 21:24:08 UTC
Comment on attachment 273746 [details] [review]
Updated solution

Thanks Reynaldo!
Comment 10 Reynaldo H. Verdejo Pinochet 2014-04-07 21:31:11 UTC
Comment on attachment 273746 [details] [review]
Updated solution

Cool, pushed. Not sure about the target milestone though so I left it at head.
Comment 11 Tim-Philipp Müller 2014-04-07 21:37:34 UTC
Next release off master is 1.3.1. If it applies to 1.2, please also cherry-pick into 1.2 branch and change milestone to 1.2.4 then, thanks!
Comment 12 Reynaldo H. Verdejo Pinochet 2014-04-08 00:14:32 UTC
pushed to 1.2, thanks.
Comment 13 Edward Hervey 2014-05-07 06:53:22 UTC
Problem is that soup-version.h was only added in 2.41 and we depend on 2.40

How about just making it

#ifndef SoupStatus
#define SoupStatus SoupKnownStatusCode
#endif
Comment 14 Tim-Philipp Müller 2014-05-07 08:23:24 UTC
That won't work, since it's an enum.
Comment 15 Sebastian Dröge (slomo) 2014-05-07 09:05:29 UTC
We could use AC_CHECK_TYPES() inside configure, this catches typedefs and #defines
Comment 16 Tim-Philipp Müller 2014-05-07 12:25:47 UTC
Created attachment 276065 [details] [review]
tests: fix compilation of souphttpsrc test for libsoup 2.40
Comment 17 Tim-Philipp Müller 2014-05-07 12:28:24 UTC
Created attachment 276066 [details] [review]
tests: fix compilation of souphttpsrc test for libsoup 2.40

Second attempt, need to make sure not to include the soup-version.h header as well I guess.
Comment 18 Nicolas Dufresne (ndufresne) 2014-05-07 13:43:00 UTC
Isn't it simplier to just bump to 2.41, not having proper version check seems like a fair rational to do so ?
Comment 19 Tim-Philipp Müller 2014-05-07 14:02:11 UTC
I think 2.41 is a dev version, so one would want to bump to 2.42, which is doable, but seems unnecessary just for the unit test, if it can be worked around easily.

Pushed this now, please re-open if there are still issues.

 commit cf94d498e68b4138d83ab321b42f1df6dd8f1426
 Author: Tim-Philipp Müller <tim@centricular.com>
 Date:   Wed May 7 13:23:50 2014 +0100

    tests: fix compilation of souphttpsrc test for libsoup 2.40
    
    SOUP_CHECK_VERSION was only added in 2.41, but we only
    depend on 2.40.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727329
Comment 20 Edward Hervey 2014-05-07 14:15:36 UTC
sorry :(


elements/souphttpsrc.c:33:56: error: missing binary operator before token "("
elements/souphttpsrc.c: In function ‘do_get’:
elements/souphttpsrc.c:504:3: error: unknown type name ‘SoupStatus’
elements/souphttpsrc.c:531:18: error: ‘SoupStatus’ undeclared (first use in this function)
elements/souphttpsrc.c:531:18: note: each undeclared identifier is reported only once for each function it appears in
elements/souphttpsrc.c:531:30: error: expected ‘)’ before ‘SOUP_STATUS_OK’
elements/souphttpsrc.c:499:12: error: variable ‘send_error_doc’ set but not used [-Werror=unused-but-set-variable]
mv -f pipelines/.deps/pipelines_wavpack-wavpack.Tpo pipelines/.deps/pipelines_wavpack-wavpack.Po
depbase=`echo elements/wavparse.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
Comment 21 Tim-Philipp Müller 2014-05-07 14:51:23 UTC
Hrm! Let's try this again then:

commit c3bd2bdcf493ba6c6dcd5bde4a67b8e0beeb21eb
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Wed May 7 15:49:39 2014 +0100

    tests: fix compilation of souphttpsrc test for libsoup 2.40 for real
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727329

Better? (Works for me)
Comment 22 Tim-Philipp Müller 2014-05-09 15:51:40 UTC
Assume it's fixed now. Please re-open if not :)