Add function to initialize locale directory localedir

[jim-easterbrook]

The function is called from gp_abilities_list_new() so existing code
will carry on working as before. Non-standard installations of
libgphoto2 can call the initialisation function with their choice of
locale dir. The function includes code to ensure it's only run once.

[ndim]

I like the idea. What other aspects of non-standard installations are there, and should those be dealt with separately or in a coordinated way? I vaguely remember iolib and camlib locations being candidates for a similar non-standard install setup function.

[jim-easterbrook]

I've handled iolibs and camlibs locations by setting environment variables, but initialisation functions would be a nicer way of doing it. That would be a different PR though.

[ndim]

Different PR maybe, but it would be nice if similar functions were named similarly, called similarly, and behaved similarly. If we are extending the API, we should do it in a halfway consistent manner, and this PR is as good a place to start with that as any.

The PR commit e6747c4 adds a function to libgphoto2

int gp_initialise_locale(const char *localedir);

This name uses "initialise" with letter "s". From a quick grep, we mostly use the "initialize" spelling using the letter "z" in our comments, but for function names the "z" vs "s" does not matter because our function names just use "init". "init" is a very common abbreviation, it is faster to type, takes less space, and does not need looking up whether it is written with "s" vs "z".

gp_init_locale() hmm... does it initialize "locale" or set the "localedir"?

We already have the following two functions which are a bit related, both as to when and how they are called, and that they are also dealing with translated messages:

const char* gp_message_codeset(const char *codeset);
const char* gp_port_message_codeset(const char *codeset);

where gp_message_codeset() calls gp_port_message_codeset() internally. So in an analog manner, adding a gp_init_localedir() function must be coupled with adding a gp_port_init_localedir() function which is called from within gp_init_localedir(), and from gp_port_info_list_new().

Naming this consistently could be

int gp_init_localedir(const char *localedir);
int gp_port_init_localedir(const char *localedir);

if it is just for initialization or

int gp_set_localedir(const char *localedir);
int gp_port_set_localedir(const char *localedir);

if it is meant to be called any time to set the localedir used by all code running subsequently.

However, now we have two of functions with closely related functionality (both …_localedir() and …_message_codeset() deal with message translations and related settings), but the function names do not suggest they are related at all. This may or may not be a problem.

Anyway, libgphoto2 lacks a comprehensive library initialization function ("gp_init", "gp_library_init", like "libusb_init"), to which such initialization parameters could be resonably passed. So we need another way to do that kind of setup, while not breaking the existing ABI and API.

Is it conceivable that someone needs to change the localedir, camlibdir, iolibdir after they have started doing actual things with libgphoto2? I do not think so, but my thinking is not always comprehensive. For a setting once model, the following set of functions and function names would make sense to me:

gp_init_camlibdir(const char *camlibdir)
gp_init_iolibdir(const char *iolibdir)
gp_init_localedir(const char *localedir)
gp_init_message_codeset(const char *codeset)
   (unfortunately, this is already called gp_message_codeset(), but well...)

All gp_init_*() functions would be specified to be called before actually using libgphoto2 (e.g. getting a GPContext or a Camera object).

(Note1: If we integrate libgphoto2_port into libgphoto2, we could remove the distinction between iolibdir and camlibdir and just have one common driverdir with a value like ${libdir}/libgphoto2/${VERSION} with two subdirs camlibs/ and iolibs/. In that case, a single corresponding init function gp_init_driverdir(const char *driverdir) would be enough.)

(Note2: If we can support statically adding (dlpreload) all drivers to the respective, we do not need camlibdir and iolibdir any more. There is no way around localedir, though.)

When adding API functions, we also need to increment LIBGPHOTO2*_AGE and LIBGPHOTO2*_CURRENT in configure.ac and libgphoto2_port/configure.ac, respectively.

BTW, the gp_initialise_locale implementation (e6747c4) always sets locale_initialized and always returns GP_OK, even if bindtextdomain() fails.

[jim-easterbrook]

I agree with all your points. Returning GP_OK was just for consistency with other functions, and to allow for future changes or additions which might fail. Is there an easy way to detect bindtextdomain() failure?

[ndim]

I read

https://www.freebsd.org/cgi/man.cgi?query=bindtextdomain&apropos=0
https://www.gnu.org/software/libc/manual/html_node/Locating-gettext-catalog.html

as bindtextdomain() failing IFF it returns NULL, and being successful otherwise.

[ndim]

I am not sure it make sense to

locale_initialised = 1;

in the error case.

I have already pushed the update

[jim-easterbrook]

I am not sure it make sense to

locale_initialised = 1;

in the error case.

I agree.

[ndim]

BTW, why the locale_initialized flag at all? gp_message_codeset() does not have such a flag, after all.

[jim-easterbrook]

The locale_initialized flag is to ensure bindtextdomain doesn't get called again when gp_abilities_list_new() calls gp_init_localedir with the default locale directory. This would undo the non-default directory already set by calling gp_init_localedir directly.

Maybe gp_message_codeset() needs something similar. I haven't looked into it. Everything should be UTF-8 nowadays.

[ndim]

Argh. Sorry for bringing this up. The relevant difference is that inside the gettext code, the codeset has a useful default without us calling any initialization function at all, while the localedir must be set (at least) once. So the two functions are fine flag-wise as they are.

So we either have a gp_init_localedir() with a flag to make it run only once, and have the calling software component call it 0 or 1 time, or we introduce a mandatory gp_library_init() with a localedir parameter which can be given as NULL for the default to be used. A new mandatory function call breaks API and ABI though and must therefore be discarded.

Apropos NULL... what does gp_init_localedir() do when called with a NULL value as its localedir argument?

I will have to check that when I am back in front of a PC.

[jim-easterbrook]

Calling gp_init_localedir with localedir value NULL is safe. bindtextdomain just returns the current locale directory, which will be the libintl.h default if it hasn't been set.

[ndim]

Argh. Sorry for bringing this up. The relevant difference is that inside the gettext code, the codeset has a useful default without us calling any initialization function at all, while the localedir must be set (at least) once.

https://www.gnu.org/software/libc/manual/html_node/Locating-gettext-catalog.html

So we either have a gp_init_localedir() with a flag to make it run only once, and have the calling software component call it 0 or 1 time, or we introduce a mandatory gp_library_init() with a localedir parameter which can be given as NULL for the default to be used.

Apropos NULL... what does gp_init_localedir() do when called with a NULL value as its localedir argument?

I will have to check that when I am back in front of a PC.

[ndim]

@msmeissn I have a few questions for you

• Are you OK with introducing setup functions to be run once only, and only before the normal library initialization happens? At this time, library initialization happens implicitly when calling gp_abilities_list_new() or gp_port_info_list_new(), but we might want to introduce at some time in the future a gp_library_init or gp_init function to initialize the library. If that were to happen, perhaps gp_preinit_localedir would make more sense than gp_init_localedir does.
• Is the gp_init_localedir() gp_port_init_localedir() function name (and possibly later gp_init_camlibdir etc.) OK with you?
• I presume LIBGPHOTO2_5_0 is the correct symbol version label to put the gp_port_init_localedir symbol under in libgphoto2_port.ver?

[msmeissn]

these are weird?

[msmeissn]

ah, but i see its all commented out.

[msmeissn]

looks good to me

[ndim]

I have just squashed my barrage of commits, rebased onto master, and merged the result into master.

Thank you very much for the original PR and the ensuing productive cooperation!