Hey,
On Fri, Jul 29, 2016 at 11:21:26AM +0100, Daniel P. Berrange wrote:
> The new database locations are
>
> - System location
>
> This is determined by the env variable
>
> $OSINFO_SYSTEM_DIR
>
> If not set, then defaults to /usr/share/osinfo
>
> This location is intended for use by operating system
> distributors to install the initial data set via a
> package management system like RPM or Deb
>
> - Local location
>
> This is determined by the env variable
>
> $OSINFO_LOCAL_DIR
>
> If not set, then defaults to /etc/osinfo
>
> This location is intended for use by local system
> administrators to install custom local data that
> should be available to all users on a host
>
> - User location
>
> This is determined by the env variable
>
> $OSINFO_USER_DIR
>
> If not set, then defaults to $XDG_CONFIG_HOME/osinfo
>
> If that is not set, then defaults to $HOME/.config/osinfo
I wanted to ask why XDG_CONFIG_HOME and not XDG_DATA_HOME, but it seems
it was already this way before the split, better to keep things as close
as possible.
>
> This location is intended for use by unprivileged users
> wishing to install local data for use by their applications
>
> Adapt to use those, but include temporary support for looking
> at the legacy local & user directory locations, and the
> OSINFO_DATA_DIR env variable for back-compat.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> osinfo/Makefile.am | 1 +
> osinfo/osinfo_loader.c | 84 ++++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/osinfo/Makefile.am b/osinfo/Makefile.am
> index ddece61..c915b57 100644
> --- a/osinfo/Makefile.am
> +++ b/osinfo/Makefile.am
> @@ -36,6 +36,7 @@ libosinfo_1_0_la_CFLAGS = \
> $(GOBJECT_CFLAGS) \
> $(GLIB_CFLAGS) \
> $(GIO_CFLAGS) \
> + -DDATA_DIR='"$(datadir)"' \
DATA_DIR is then used to lookup the osinfo database. This means that the
database and the library have to be built to use the same prefix. I'm
wondering if it would make sense to have a .pc file together with the
database to help with locating the system database. Probably not worth
the complication at this point.
> -DPKG_DATA_DIR='"$(pkgdatadir)"' \
> -DSYS_CONF_DIR='"$(sysconfdir)"' \
> -DLOCALEDIR="\"$(datadir)/locale\"" \
> diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> index 73843cf..33ff174 100644
> --- a/osinfo/osinfo_loader.c
> +++ b/osinfo/osinfo_loader.c
> @@ -2280,23 +2280,61 @@ static GFile *osinfo_loader_get_usb_path(void)
> static GFile *osinfo_loader_get_system_path(void)
> {
> GFile *file;
> - gchar *dbdir;
> - const gchar *path = g_getenv("OSINFO_DATA_DIR");
> - if (!path)
> - path = PKG_DATA_DIR;
> + const gchar *path;
> +
> + path = g_getenv("OSINFO_DATA_DIR");
> + if (path) {
> + char *dbpath;
> + static gboolean warned = FALSE;
> + if (!warned) {
> + g_printerr("$OSINFO_DATA_DIR is deprecated, please "
> + "use $OSINFO_SYSTEM_DIR intead. Support "
'instead'. I think this should be translatable.
> + "for $OSINFO_DATA_DIR will be removed "
> + "in a future release");
> + warned = TRUE;
> + }
> +
> + dbpath = g_strdup_printf("%s/db", path);
> + file = g_file_new_for_path(path);
> + g_free(dbpath);
> + } else {
> + path = g_getenv("OSINFO_SYSTEM_DIR");
> + if (!path)
> + path = DATA_DIR "/osinfo";
>
> - dbdir = g_strdup_printf("%s/db", path);
> - file = g_file_new_for_path(dbdir);
> + file = g_file_new_for_path(path);
> + }
> g_object_set_data(G_OBJECT(file), "data-format",
> GINT_TO_POINTER(OSINFO_DATA_FORMAT_NATIVE));
> - g_free(dbdir);
> return file;
> }
>
> static GFile *osinfo_loader_get_local_path(void)
> {
> GFile *file;
> - file = g_file_new_for_path(SYS_CONF_DIR "/libosinfo/db");
> + const gchar *path = g_getenv("OSINFO_LOCAL_DIR");
> +
> + if (!path) {
> + path = SYS_CONF_DIR "/osinfo";
> +
> + if (access(path, R_OK) == -1) {
This could also use g_file_test(path, G_FILE_TEST_IS_DIR)
> + const gchar *oldpath = SYS_CONF_DIR "/libosinfo/db";
> + if (access(oldpath, R_OK) != -1) {
> + static gboolean warned = FALSE;
> +
> + if (!warned) {
> + g_printerr("%s is deprecated, please use %s instead. "
> + "Support %s will be removed in a future "
Support *for* %s will be ... ? This should be marked for translation
too.
> + "release",
> + oldpath, path, oldpath);
> + warned = TRUE;
> + }
> + path = oldpath;
> + }
> + }
> + }
> +
> + file = g_file_new_for_path(path);
> g_object_set_data(G_OBJECT(file), "data-format",
> GINT_TO_POINTER(OSINFO_DATA_FORMAT_NATIVE));
> return file;
> @@ -2305,14 +2343,36 @@ static GFile *osinfo_loader_get_local_path(void)
> static GFile *osinfo_loader_get_user_path(void)
> {
> GFile *file;
> - gchar *dbdir;
> + const gchar *path = g_getenv("OSINFO_USER_DIR");
> const gchar *configdir = g_get_user_config_dir();
>
> - dbdir = g_strdup_printf("%s/libosinfo/db", configdir);
> - file = g_file_new_for_path(dbdir);
> + if (path) {
> + file = g_file_new_for_path(path);
> + } else {
> + gchar *dbdir = g_strdup_printf("%s/osinfo", configdir);
> + if (access(dbdir, R_OK) == -1) {
> + static gboolean warned = FALSE;
> + gchar *olddir = g_strdup_printf("%s/libosinfo/db", configdir);
> + if (access(olddir, R_OK) != -1) {
> + if (!warned) {
> + g_printerr("%s is deprecated, please use %s instead. "
> + "Support %s will be removed in a future "
> + "release",
> + olddir, dbdir, olddir);
Same comments as above. Apart from these minor things, looks good to me.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
