GNOME Bugzilla – Bug 776081
windows: rework loaders cache relocation support
Last modified: 2018-04-24 10:37:12 UTC
Created attachment 341937 [details] [review] windows: rework loaders cache relocation support Relocation works by recognizing paths in the loaders cache which start with the built time prefix and extract the relative path from that. This leads to the following problem when updating the cache: In case the package is build on another machine one has to either match the build directory layout or adjust the cache by hand for the resulting cache to stay relocatable. This commonly occurs with msys2 where mostly pre-build packages are used which are built on another machine and the cache gets generated at install time. Another case is updating the cache in a separate deployment environment. This patch takes the package installation directory as a base and writes relative paths into the cache when relocation is enabled. When loading the cache a relative path is made absolute by prepending the package base again.
This is now included in msys2: https://github.com/Alexpux/MINGW-packages/pull/2029
Created attachment 371168 [details] [review] windows: rework loaders cache relocation support rebased on master
Review of attachment 371168 [details] [review]: Patch looks good to me, see the nitpicks to fix. I'll give you the final ok once this is fixed. ::: gdk-pixbuf/gdk-pixbuf-io.c @@ +521,3 @@ have_error = TRUE; } +#ifdef GDK_PIXBUF_RELOCATABLE I wonder if we really need the ifdef here, I think I prefer that you ifdef inside build_module_path ::: gdk-pixbuf/queryloaders.c @@ +119,3 @@ +#ifdef GDK_PIXBUF_RELOCATABLE + +/* Based on gdk_pixbuf_get_toplevel () */ should we put that method in an utils file and share it? @@ +130,3 @@ +#elif defined(OS_DARWIN) + char pathbuf[PATH_MAX + 1]; + uint32_t bufsize = sizeof(pathbuf); check all the methods here and add space before ( @@ +154,3 @@ +/* Returns the relative path or NULL; transfer full */ +static gchar * +get_relative_path(const gchar *parent, const gchar *descendant) space before ( and split params
Created attachment 371221 [details] [review] windows: rework loaders cache relocation support
(In reply to Ignacio Casal Quinteiro (nacho) from comment #3) > Review of attachment 371168 [details] [review] [review]: > > Patch looks good to me, see the nitpicks to fix. I'll give you the final ok > once this is fixed. Thanks. > ::: gdk-pixbuf/gdk-pixbuf-io.c > @@ +521,3 @@ > have_error = TRUE; > } > +#ifdef GDK_PIXBUF_RELOCATABLE > > I wonder if we really need the ifdef here, I think I prefer that you ifdef > inside build_module_path good idea, done > ::: gdk-pixbuf/queryloaders.c > @@ +119,3 @@ > +#ifdef GDK_PIXBUF_RELOCATABLE > + > +/* Based on gdk_pixbuf_get_toplevel () */ > > should we put that method in an utils file and share it? I'd like to keep this simple if you don't mind. gdk_pixbuf_get_toplevel uses the module handle as a base so more changes would be required there. > @@ +130,3 @@ > +#elif defined(OS_DARWIN) > + char pathbuf[PATH_MAX + 1]; > + uint32_t bufsize = sizeof(pathbuf); > > check all the methods here and add space before ( done > @@ +154,3 @@ > +/* Returns the relative path or NULL; transfer full */ > +static gchar * > +get_relative_path(const gchar *parent, const gchar *descendant) > > space before ( and split params done
Review of attachment 371221 [details] [review]: Fix the nitpick and feel free to push it. Thanks for working on this! ::: gdk-pixbuf/gdk-pixbuf-io.c @@ +354,3 @@ { +#ifdef GDK_PIXBUF_RELOCATABLE + if (g_path_is_absolute (path)) I think the { should be inline to keep consistency with the rest of the code.
Thanks for the reviews!