GNOME Bugzilla – Bug 737523
Extend relocation support to OS X and Linux
Last modified: 2014-10-22 19:00:37 UTC
Created attachment 287288 [details] [review] io: extend support for relocations to OS X and Linux For Windows platforms, gdk-pixbuf has support to replace the build-time directory of modules in loaders.cache to the runtime prefix considering these directories are almost always different. But the same happens with OS X applications, which are bundles that can be installed anywhere and Linux bundles. This patch extends the relocation support to these platforms too. With this patch the relocation is enabled for OS X and Windows by default, and disabled on Linux. Ideally, I would like to have a configure option --enable-relocations, but I don't know how to change the default value depending on the target platform in the m4 world.
Created attachment 287375 [details] [review] io: extend support for relocations to OS X and Linux Apply changes to get_localedir() in gdk-pixbuf-utils.c too
Created attachment 287938 [details] [review] io: Extend relocation support to OS X and Linux Add missing include
Review of attachment 287938 [details] [review]: ::: configure.ac @@ +1073,3 @@ +# cache with the installation prefix on this machine +# for relocatable packages such as Windows and OS X +# applicationsapplications and linux bundles That doesn't seem to enable it for Linux platform, does it? ::: gdk-pixbuf/gdk-pixbuf-io.c @@ +211,3 @@ + toplevel = g_build_path (G_DIR_SEPARATOR_S, bin_dir, "..", NULL); + g_free (bin_dir); +#elif defined (G_OS_UNIX) You need to check for Linux specifically here, I doubt this works on, say, FreeBSD. @@ +220,3 @@ + g_free (bin_dir); +#else +#error "Relocations not supported for this platform" This needs not to fail on other platforms, so you probably want to wrap _gdk_pixbuf_get_toplevel() in #ifdef GDK_PIXBUF_RELOCATABLE. ::: gdk-pixbuf/gdk-pixbuf-util.c @@ +360,2 @@ /* In gdk-pixbuf-io.c */ + extern char *_gdk_pixbuf_get_toplevel (void); That's pretty ugly and should be fixed in a preliminary patch. Move the definition to "gkd-pixbuf-private.h", which isn't installed. @@ +370,3 @@ g_free (temp); +#else + retval = temp; I'd do: #ifdef G_OS_WIN32 { char *retval; retval = g_win32_locale_filename_from_utf8 (temp); g_free (temp); return retval; } #else return temp; #endif
(In reply to comment #3) > Review of attachment 287938 [details] [review]: > > ::: configure.ac > @@ +1073,3 @@ > +# cache with the installation prefix on this machine > +# for relocatable packages such as Windows and OS X > +# applicationsapplications and linux bundles > > That doesn't seem to enable it for Linux platform, does it? I don't think it should be enabled for Linux platforms by default as it wouldn't work for applications installed in /usr/local but linking against libgdkpixbuf from /usr/lib If one is building an AppKit bundle, the build system can enable it adding -DGDK_PIXBUF_RELOCATABLE to the CFLAGS.
(In reply to comment #4) > (In reply to comment #3) > > Review of attachment 287938 [details] [review] [details]: > > > > ::: configure.ac > > @@ +1073,3 @@ > > +# cache with the installation prefix on this machine > > +# for relocatable packages such as Windows and OS X > > +# applicationsapplications and linux bundles > > > > That doesn't seem to enable it for Linux platform, does it? > > I don't think it should be enabled for Linux platforms by default as it > wouldn't work for applications installed in /usr/local but linking against > libgdkpixbuf from /usr/lib > If one is building an AppKit bundle, the build system can enable it adding > -DGDK_PIXBUF_RELOCATABLE to the CFLAGS. Right, I misunderstood that. Can you update the patch though?
Created attachment 289160 [details] [review] lib: add gdk_pixbuf_win32_get_toplevel to the private API
Created attachment 289161 [details] [review] io: extend support for relotations to OS X and Linux
G_OS_LINUX does not exists though...
Created attachment 289163 [details] [review] io: extend support for relotations to OS X and Linux
Review of attachment 289160 [details] [review]: Looks good.
Review of attachment 289163 [details] [review]: Looks good as well.
Attachment 289160 [details] pushed as ae7716c - lib: add gdk_pixbuf_win32_get_toplevel to the private API Attachment 289163 [details] pushed as a55aa6e - io: extend support for relotations to OS X and Linux