GIMP is already relocatable on the Win32 platform. It would be trival to extend
this to other platforms. One would just have to do some minor changes to
libgimpbase/gimpenv.c.

This is a duplicate of bug #139580, which was closed as WONTFIX after a long
discussion on the developers mailing list.  If you would like to reopen the
question, please do so there (and it would help to read the relevant material
from the mailing list, if you can find it).

*** This bug has been marked as a duplicate of 138580 ***

Reopening since this is certainly not a duplicate of this bug.

Huh?  Is it not a duplicate because it deals with a different issue, or not a
duplicate because bug #138580 contained a patch that was ultimately rejected?

Because bug #138580 is summarized as "gweather is not localized" which doesn't
seem to be related.

I think you are refering to bug #139580. That bug should not have been closed in
the first place and the patch was also not ultimately rejected. The author was
told that it needs work before it can be accepted and that we would prefer a
solution that is not Linux specific. That doesn't mean that the change is
rejected, just that it needs to undergo some changes before it will be
considered for inclusion. Unfortunately noone took the time to do these changes yet.

It looks like the GLib version of binreloc has a much better chance to be
accepted for inclusion in libgimpbase than the original patch, see
http://autopackage.org/docs/binreloc/doxygen/glib-binreloc.html

Aargh.  Clumsy fingers on my part.

Created attachment 54362 [details] [review]
New proposed patch

Here's a new patch based on the glib version of BinReloc. Patched against Gimp
2.3.5.

AFAIK, the patch introduces memory leaks. The return values of
gbr_find_data_dir() and friends need to be freed. Also I wonder if this code
will do the right thing if being called from a plug-in because reloc hasn't been
initialized then. The GIMP specific binreloc wrapper code would also preferably
live in the gimp namespace.

Created attachment 54366 [details] [review]
Updates patch

This updates patch fixes the memory leaks you mentioned.

> Also I wonder if this code will do the right thing if being called from a
> plug-in because reloc hasn't been initialized then.

BinReloc is initialized right after calling gimp_init_malloc(). Or do you mean
something else?

> The GIMP specific binreloc wrapper code would also preferably
> live in the gimp namespace.

You mean that the functions should be called gimp_something()?

I wonder if it wouldn't be better to stay with the code from your first patch
and to make the gbr_find_dir() functions return a const string. The value
shouldn't change and can be computed on the first call and stored in a local
static variable. The functions can then be marked as G_GNUC_CONST.

The binreloc initialization is only called by the gimp core, not by plug-ins
(which are separate processes). Since plug-ins are also using the functions in
gimpenv.[ch], they would probably have to use the gbr_init_lib() function.

Yes, gimp_binreloc_ would for example be a good function prefix.

Oh, and any functions that are not needed in the public API but are only used
within libgimpbase, should be prefixed with an underscore and marked as
G_GNUC_INTERNAL. None of the binreloc functions need to be in the public API. We
just need a gimp_env_init() function that is called from the core (app/main.c)
and from the gimp_main() function in libgimp/gimp.c which is called from all
plug-ins.

Created attachment 54369 [details] [review]
Changed gbr_ to _gimp_binreloc_; marked most functions G_GNUC_INTERNAL

And I used a different way to ensure that BinReloc is initialized. The
functions in gimpenv.c checks whether it has tried to initialize BinReloc
before. If not, it will initialize BinReloc.

I've changed some parts of your patch to that the reloc API is completely hidden
and will commit these changes as soon as I get a chance to test them.

OK. I haven't found problems in my own tests. Here's a convenient command:

(apply patch)
(rerun autoconf/automake tools to regenerate build environment)
make clean
CFLAGS=-pipe ./configure
make install DESTDIR=$HOME/gimptest
~/gimptest/usr/local/bin/gimp-2.3

I haven't tested everything yet (localization for example), but in general this
seems to work. Committed to CVS:

2005-11-06  Sven Neumann  <sven@gimp.org>

	Added support for binary relocation by means of binreloc, largely
	based on a patch by Hongli Lai:

	* m4macros/Makefile.am
	* m4macros/binreloc.m4: new file providing a macro to check for
	binreloc support.

	* acinclude.m4
	* configure.in: use the macro.

	* libgimpbase/Makefile.am
	* libgimpbase/gimpreloc.[ch]: new files providing binreloc support
	on Linux.

	* libgimpbase/gimpenv.[ch]: use binreloc, provide a function to
	initialize the environment machinery.

	* libgimpbase/gimpbase.def: updated.

	* app/Makefile.am: fiddle with the LDFLAGS for binreloc.

	* app/main.c (main): gimp_env_init(FALSE).

	* libgimp/gimp.c (gimp_main): gimp_env_init(TRUE).

I am not yet closing this bug as FIXED because I think we should default to not
build with binreloc support for 2.4. It's a good thing to have for people who
want to intentionally build a relocatable binary and it should be documented in
INSTALL. But I think in general it bears the risk of doing the wrong thing. For
now it is enabled so that it gets some testing.

In the meantime the default has been changed to --disable-binreloc and the
feature has been documented in INSTALL. Closing as FIXED.

It would be nice if we could move the linux specific code to a new file
gimpreloc-linux.c and move the Win32 specific code in gimpenv.c into
gimpreloc-win32.c. gimpenv.c could then be completely platform agnostic and at
some point someone could add support for binary relocatibility on other
platforms. Especially on Mac OS X this would be very useful.