GNOME Bugzilla – Bug 726762
Fix Import of Clutter Version Constants
Last modified: 2014-03-20 16:35:11 UTC
Created attachment 272459 [details] Sample program causing the link problem Hi, In commit 3b21999, a change is done for CLUTTER_VAR to be defined via _CLUTTER_EXTERN. Unfortunately on Visual Studio builds, this will cause the following problem when building a program (a sample is attached here) that uses these constants: a.obj : error LNK2019: unresolved external symbol clutter_major_version referenced in function main a.obj : error LNK2019: unresolved external symbol clutter_minor_version referenced in function main a.obj : error LNK2019: unresolved external symbol clutter_micro_version referenced in function main Meaning that the programs using these three items will fail to link. Variables and constants, unlike functions, need to be decorated with __declspec (dllimport), so that programs using them will import them and acquire the correct values for them.
Created attachment 272460 [details] [review] Revert the change to define CLUTTER_VAR using _CLUTTER_EXTERN Hi, This patch does the revert of 3b219994, to define CLUTTER_VAR in the former method. This is similar to what is currently done in GLib, GLIB_VAR, so that variables and constants can be exported and imported correctly, on Visual Studio builds and builds using a MinGW-built Clutter with Visual Studio. With blessings, thank you!
Created attachment 272462 [details] [review] Fix Import of Version Constants on Visual Studio Hi, Please forget my previous patch, as it seems that there are other things to consider about using _CLUTTER_EXTERN... Coming to think of it, it's probably a better idea to do this instead to fix this. We can still continue to define CLUTTER_VAR as _CLUTTER_EXTERN, as done in commit 3b21999, but do so when we are either building the main Clutter library (with any compiler) and/or when we are not using Visual Studio to build against Clutter. When we are building against Clutter on Visual Studio, then define CLUTTER_VAR like what we use to do (extern __declspec (dllimport)) so that these constants will link and import properly. With blessings, thank you!
Review of attachment 272462 [details] [review]: ah, this is totally my fault, and I apologize: I should have checked what GLib does before stomping all over it. ::: clutter/clutter-version.h.in @@ +262,1 @@ #define CLUTTER_VAR _CLUTTER_EXTERN the: #define CLUTTER_VAR _CLUTTER_EXTERN can be replaced with: #define CLUTTER_VAR extern
Hello Emmanuele, (In reply to comment #3) > #define CLUTTER_VAR _CLUTTER_EXTERN > > can be replaced with: > > #define CLUTTER_VAR extern Unfortunately I don't think it can be replaced with extern, as _CLUTTER_EXTERN is also used when CLUTTER_COMPILATION is defined, including on Visual C++, so that the constants are decorated with __declspec (dllexport) extern during the build of the main Clutter library, making those constants export from the DLL. Please let me know if otherwise. With blessings, thank you!
GLib uses (simplified without the GLIB_STATIC_COMPILATION case): #ifdef G_PLATFORM_WIN32 # ifdef GLIB_COMPILATION # ifdef DLL_EXPORT # define GLIB_VAR __declspec(dllexport) # else # define GLIB_VAR extern # endif # else # define GLIB_VAR extern __declspec(dllimport) # endif #else # define GLIB_VAR _GLIB_EXTERN #endif so we want something like: #ifdef CLUTTER_WINDOWING_WIN32 # ifdef CLUTTER_COMPILATION # ifdef DLL_EXPORT # define CLUTTER_VAR __declspec(dllexport) # else # define CLUTTER_VAR extern # endif # else # define CLUTTER_VAR __declspec(dllimport) # endif #else # define CLUTTER_VAR _CLUTTER_EXTERN #endif does this look good to you?
Created attachment 272490 [details] [review] Refine how CLUTTER_VAR is defined Hello Emmanuele, I think it's good, I just added DLL_EXPORT to the Visual Studio build defines so that the correct thing is done during the build of the Clutter DLL. Not sure about the MinGW part, but anyways... With blessings, thanks!
Review of attachment 272490 [details] [review]: looks good.
Hi, Apparently I did my patch against my previous one (as the one in comment #2), so I had the same updates to the original code pushed as 2b3fac8b. Will close this bug shortly. With blessings, thank you!