GNOME Bugzilla – Bug 342564
Configure options causing compilation to fail
Last modified: 2007-05-09 16:33:19 UTC
Please describe the problem: Using any of the following configure options causes the build to fail: --disable-parse --disable-trace --disable-alloc-trace --disable-enumtypes Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information: I do not know whether the correct solution to this problem is to remove the options, or fix the code. I was just trying to build the libraries as small as possible, and thus started by disabling just about everything I could...
*** Bug 344016 has been marked as a duplicate of this bug. ***
also see partial patch in Bug #334016
parse, trace and alloc-trace has been fixed. disabling enumtypes fails: ../gst/.libs/libgstreamer-0.10.so: undefined reference to `gst_state_get_type' ../gst/.libs/libgstreamer-0.10.so: undefined reference to `gst_pad_direction_get_type' ../gst/.libs/libgstreamer-0.10.so: undefined reference to `gst_format_get_type' ../gst/.libs/libgstreamer-0.10.so: undefined reference to `gst_seek_flags_get_type' ../gst/.libs/libgstreamer-0.10.so: undefined reference to `gst_seek_type_get_type' ../gst/.libs/libgstreamer-0.10.so: undefined reference to `gst_pad_presence_get_type'
Created attachment 68869 [details] [review] patch to fix compilation with --disable-index --disable-index saves (9604 + 6996 = 16600) bytes 9604 on libgstreamer and 6996 by ommiting the indexers.
> Created an attachment (id=68869) [edit] > patch to fix compilation with --disable-index What's with all the unrelated stuff in the patch (added casts, const-ification)? (On a sidenote, patches in unified diff format are often easier to read (echo "diff -u -p" >> ~/.cvsrc)). If it works fine for you with --disable-index and with --enable-index, why not just commit it?
oh sh*t wrong patch, I haven't committed so far, as I can write acces cvs from work and wanted to retest at home before committing
What would you do when one configures with --disable-enumtypes: a.) also disable all code that uses these enums (see below) b.) leave explicitly used enumtypes enabled c.) remove this --disable-enumtypes from configure d.) define enumtypes with empty desc/blurb strings (ugly) gst_state_get_type: (2 occurences) gst_message_new_state_changed gst_message_parse_state_changed gst_format_get_type: (19 occurences) gst_event_ gst_message_ gst_query_ gst_seek_flags_get_type: (1 occurence) gst_event_new_seek gst_seek_type_get_type: (2 occurences) gst_event_new_seek gst_pad_presence_get_type: (1 occurence) gstregistryxml:load_pad_template
--disable-trace is broken again gst_launch_0.10-gst-launch.o: In function `main':/home/xyz/gstreamer-0.10.9/tools/gst-launch.c:591: undefined reference to `gst_alloc_trace_print_live' :/home/xyz/gstreamer-0.10.9/tools/gst-launch.c:720: undefined reference to `gst_alloc_trace_print_live' collect2: ld returned 1 exit status make[3]: *** [gst-launch-0.10] Error 1
shouldn't --disable-net also exclude: gstreamer/libs/gst/net/ gst-plugins-base/gst-libs/gst/netbuffer
I'm away from work, so I cannot test myself, but is --disable-trace still broken after http://webcvs.freedesktop.org/gstreamer/gstreamer/gst/gsttrace.h?r1=1.26&r2=1.27 was applied (as reported in #347756)?
*** Bug 352246 has been marked as a duplicate of this bug. ***
Created attachment 73040 [details] [review] Condional include of gstparse.h in gst.h if --disable-parse is used The use of --disable-parse for gstreamer breaks building of gst-plugins-base. The reason is that gstparse.h is no longer installed when --disable-parse is used, but gst.h still uncondionally includes gstparse.h. The attached patch (gst.h.patch) should fix this.
IMO, gst_parse_launch() should still exist as a symbol even when --disable-parse is used -- it's easier to be ABI compatible. The function should return NULL (or threaten the programmer with kittenicide). It's cleaner than having #ifdef's in headers.
Created attachment 73658 [details] [review] Allow --disable-parse to work again by providing dummy defines in gstparse.h This latest patch to fix --disable-parse adds dummy defines to gstparse.h when --disable-parse is used. This is exactly the same way as --disable-trace is handled.
what is the idea behind using dummy #defines instead of making the api unavailable. Personally I want my build to fail if API is not available. Then I can add configure checks that require gstreamer to be build with the feature I actually use in my app. if we have the defines in place, it goes unnoticed that the API does not work. What about just ifdef'ing the functionbody in the .c file: GstElement* gst_parse_launchv(const gchar **argv, GError **error) { #ifndef GST_DISABLE_PARSE ... original code ... #else GST_WARNING ("disabled API called"); return NULL; #endif } This way we reduce code size and keep ABI intact. Although I would prefer the current solution, if parse is disabled I don't want to have that ABI part in the resulting lib.
Created attachment 76641 [details] [review] Maintain a consistent ABI when --disable-parse is used The idea behind the #defines was to maintain the API even when --disable-parse is used. This is the same as how it is done for a number of the other --disable options. However, I fully agree with you that we should also maintain the ABI. That way it does not matter whether the version of GStreamer you build your application against has been configured with --disable-parse or not, the application will be able to use the functionality when it exists. So, I have attached a new patch that should hopefully be acceptable as it maintains both the API and ABI when --disable-parse is used. One might consider adding a GST_PARSE_ERROR_UNSUPPORTED as well to be able to tell the real reason why a pipeline could not be constructed.
right, running the app with G_DEBUG="fatal_warnings" will abort on the first stub API usage. thus one can safly test that nothing like that is used (if you can run all your code).
GST_WARNING()s do not affect the glib debug system.
so what about using a g_warning() instead of the GST_WARNING(). The patch looks good and imho should be applied then.
> so what about using a g_warning() instead of the GST_WARNING(). Since we now have an appropriate error code, it should just return NULL and a GST_CORE_ERROR_DISABLED error IMHO. > The patch looks good and imho should be applied then. The main reason (besides the missing error code) I haven't applied it yet was your "if parse is disabled I don't want to have that ABI part in the resulting lib" objection actually ;)
I just want a reliable way for the developer to check for it. GST_CORE_ERROR_DISABLED is fine for that. So no more objections for my side.
Committed version that sets the GError properly when --disable-parse-launch is used; also added a small unit test. 2007-05-09 Tim-Philipp Müller <tim at centricular dot net> Based on patch by: Peter Kjellerstedt <pkj at axis com> * gst/Makefile.am: * gst/gstparse.c: (gst_parse_launchv), (gst_parse_launch): * gst/gstparse.h: * gst/gstutils.c: (gst_parse_bin_from_description): * gst/gstutils.h: Maintain API and ABI when --disable-parse is used. Now that we have an appropriate error code, we can just return NULL and the appropriate error when gst_parse_launch() is used despite it having been disabled (#342564). * tests/check/Makefile.am: * tests/check/pipelines/.cvsignore: * tests/check/pipelines/parse-disabled.c: Make sure these functions exist and return NULL plus a GError when --disable-parse is used.