GNOME Bugzilla – Bug 492077
Build issues on Windows/MSVC
Last modified: 2007-11-01 23:52:46 UTC
Please describe the problem: GStreamer doesn't build properly with Windows/MSVC (MSVS2005) and GLib 2.14.x. There are also some symbols missing in the .def file that need to be exported. Steps to reproduce: 1. 2. 3. Actual results: Expected results: Does this happen every time? Other information:
Created attachment 98238 [details] [review] Debug category should be exported, not imported, when building plugins.
Created attachment 98239 [details] [review] Some additional symbols need to be exported.
Created attachment 98241 [details] [review] dirent needs a couple of casts to build warning-free with MSVC.
Created attachment 98242 [details] [review] Fixed a couple of missing includes and a C99 issue (MSVC doesn't like C99).
Created attachment 98243 [details] [review] GLib doesn't have a pipe() macro anymore starting with 2.14.0.
Thanks for the patches. I had to do the lib*.def ones manually, since most of the missing symbols were already added a few weeks ago (and your patch changed the alphabetical order of things too for some reason). Hope I didn't miss anything, please double-check. I have changed the guard around the unistd.h include to #ifdef HAVE_UNISTD_H, that seems more correct to me. Could you expand on patch #98238 a bit for non-win32 like me? Who defines GST_PLUGIN_EXPORTS? I can't find a reference to that anywhere within GStreamer. Also, what does the change actually do? Is it really needed? (It seems to have worked fine so far without on MSVC)
Sorry about the hassle with the .def stuff, my bad for not getting around to pushing the patches earlier. The change of alphabetical order was clearly not intentional, I realize that this must've been done rather hastily. I'm planning on going over this properly with the same code built with autotools on Linux, run nm, sort the symbols and compare to an export dump on Windows. There's probably a few left out, the ones added by the patch were discovered because some plugin relied on it, and given that we only build a subset of the plugins there could be a few still left out. Maybe this is something that could be integrated with the continuous build system, after building it could dump the symbols and compare to the .def files. Great catch regarding the HAVE_UNISTD_H fix, that's definitely more correct. Patch #98238 is in retrospect a bit confusing and needs some explaining. When exporting symbols you need to explicitly say which should be exported, or you won't get any exported at all. There are two ways to do this. 1. You have an entry like the following in the header-file: int __declspec(dllexport) gst_foo_do_bar (void); (Obviously the _user_ of the library needs "dllimport" instead of "dllexport", so you want to use a macro for the whole __declspec(dllimport/dllexport).) 2. Tell the linker to use a .def file that you create with a list of symbols, like: gst_foo_do_bar A common convention is to have a define called "LIBRARY_NAME_EXPORTS" that you only define when building LIBRARY_NAME, and in the header-file you do: #ifndef __GNUC__ # define __DLL_IMPORT__ __declspec(dllimport) # define __DLL_EXPORT__ __declspec(dllexport) #else # define __DLL_IMPORT__ __attribute__((dllimport)) extern # define __DLL_EXPORT__ __attribute__((dllexport)) extern #endif #if defined(__WIN32__) || defined(_WIN32) || defined(WIN32) # ifdef LIBRARY_NAME_EXPORTS # define LIBRARY_NAME_API __DLL_EXPORT__ # else # define LIBRARY_NAME_API __DLL_IMPORT__ # endif #else # define LIBRARY_NAME_API extern #endif And use the LIBRARY_NAME_API like this: int LIBRARY_NAME_API gst_foo_do_bar (void); GStreamer has, as you can see from the patch, a mixture of these two (but the majority is of the latter kind). The upside to the .def approach is that you don't need to change all the header-files if you're porting something to Windows, whilst the flip side is obviously that you need to maintain the .def files. The patch changes the macro GST_DEBUG_CATEGORY_EXTERN so that it declares the symbol as exported when used by a plugin (built with GST_PLUGIN_EXPORTS defined) to export a debug category, and imported otherwise. Without this patch the linker generates warnings about conflicting linkage, but presumably it figures out what your intention is and does the right thing. When building GStreamer on Windows we have a bunch of MSVS2005 project files of which those for plugins define GST_PLUGIN_EXPORTS. I didn't add this to the project files in the win32 subdirectory because I wouldn't be able to test these changes. The reason that we're not using these project files is another (rather long) story, but basically it's because we have a different approach. MSVS has the notion of a solution containing projects, where each project is typically a program or a library. The solution also knows about dependencies between the projects. In our team we have one big solution that contains projects for everything from GLib to GStreamer (as this is the most comfortable to work with for a Windows developer, given that dependencies etc. are taken care of), and by using something called "property sheets" we can share common things among the projects. All projects inherit the property sheet that we've called "Common" which sets output directories, common compiler flags etc., and the projects that are GStreamer plugins also inherit "GStreamerPlugin", which overrides the output directory to be $(OutDir)/lib/gstreamer-0.10 instead of the default $(OutDir)/bin (set by the Common property sheet), adds this GST_PLUGIN_EXPORTS define, etc. We also have a python tool that parses a given Makefile.am and installs header-files in $(OutDir)/include, so that we don't have to duplicate the list of header-files in our build system. So bottom line is that we end up with just the list of files in the project files, they end up being really slim thanks to property sheets, and with one toplevel solution instead of one solution per component, so that means we have very little in common with the projects in the win32 subdir. I'm hoping that this is something that can be made suitable for upstream eventually, but that's a different issue (something that I'm planning on trying to address in http://projects.collabora.co.uk/~oleavr/OABuild/). Sorry about the lengthy explanation. Coming from a Linux-background I have to say that the more I learn about Windows the more it beats me that it is not a very developer-friendly platform. :)
After further discussion on IRC: Nov 01 17:52:45 <__tim> oleavr: can I bug you again about the export declaration for a second? Nov 01 17:54:20 <__tim> oleavr: why does it need to be exported at all? exported debug category in plugins usually just means that other .c files within the same plugin should be able to use the symbol, but it doesn't need to be visible to code outside the plugin afaics Nov 01 17:55:12 <oleavr> __tim: ahh.. that explain a few things, I was wondering what the dllimport was doing there in the first place Nov 01 17:56:16 <oleavr> __tim: let me try removing it here and see if MSVC goes on a pony killing spree :) Nov 01 19:49:00 <oleavr> __tim: I'm puzzled about the whole __declspec(dllimport) for the GST_DEBUG_CATEGORY_EXTERN, can't see why it was there in the first place, it should clearly be extern for all cases.. and I just verified this with MSVC, builds just fine without any warnings Nov 01 19:50:44 <oleavr> __tim: so it'd rock if you could fix it by removing the MSVC special-casing and mark the remaining patch on #492077 as obsolete :) Nov 01 19:56:34 <__tim> oleavr: okay, I'll have another look later; maybe there's a hint in the history why it was added in the first place :) Nov 01 20:24:49 <oleavr>__tim: tracked it down: http://webcvs.freedesktop.org/gstreamer/gst-plugins-good/gst/avi/gstavidemux.c?r1=1.181&r2=1.182 http://webcvs.freedesktop.org/gstreamer/gst-plugins-good/gst/avi/gstavidemux.c?r1=1.186&r2=1.187 http://webcvs.freedesktop.org/gstreamer/gstreamer/gst/gstinfo.h?r1=1.94&r2=1.95 http://webcvs.freedesktop.org/gstreamer/gstreamer/gst/gstinfo.h?r1=1.99&r2=1.100 http://webcvs.freedesktop.org/gstreamer/gstreamer/gst/gstinfo.h?r1=1.100&r2=1.101 looks to me like the initial fix couldn't possibly have been tested given that it has a syntax error (_declspec instead of __declspec) Nov 01 20:26:22 <__tim> oh THAT's where it comes from, it was just copy'n'pasted from a plugin Nov 01 20:26:25 <__tim> excellent 2007-11-01 Tim-Philipp Müller <tim at centricular dot net> * gst/gstinfo.h: (GST_DEBUG_CATEGORY_EXTERN): Remove __declspec(dllimport) for MSVC that was copied over into core from a plugin, obviously without ever having been tested (note the single underscore in _declspec in the initial commit), and that doesn't really make sense. See #492077.