[Libosinfo] [osinfo-db-tools PATCH 1/1] import: Introduce "--latest" option

[Cole Robinson]

On 10/09/2018 10:34 AM, Fabiano Fidêncio wrote:
> --latest option checks whether there's a new osinfo-db available from
> the official libosinfo's release website, downloads and install it.
> 
> The download and installation is only then when the version available in
> libosinfo's release website is newer than the version installed in the
> (specified location in) system.
> 

I'd like to see a bit more details in the commit message, like the file 
format of the latest.txt file, and the URL we are using (which I realize 
will change before commit)

> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>   tools/osinfo-db-import.c | 155 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 155 insertions(+)
> 
> diff --git a/tools/osinfo-db-import.c b/tools/osinfo-db-import.c
> index c0b4931..79a0031 100644
> --- a/tools/osinfo-db-import.c
> +++ b/tools/osinfo-db-import.c
> @@ -31,6 +31,9 @@
>   
>   #include "osinfo-db-util.h"
>   
> +#define LATEST_URI "https://fidencio.fedorapeople.org/osinfo-db-latest.txt"
> +#define VERSION_FILE "VERSION"
> +
>   const char *argv0;
>   
>   static int osinfo_db_import_create_reg(GFile *file,
> @@ -184,6 +187,127 @@ osinfo_db_import_download_file(const gchar *url,
>       return ret;
>   }
>   
> +static gboolean osinfo_db_get_installed_version(GFile *dir,
> +                                                gchar **version)
> +{
> +    GFile *file = NULL;
> +    GFileInfo *info = NULL;
> +    GFileInputStream *stream = NULL;
> +    goffset count;
> +    GError *err = NULL;
> +    gboolean ret = FALSE;
> +
> +    file = g_file_get_child(dir, VERSION_FILE);
> +    if (file == NULL)
> +        return FALSE;
> +
> +    info = g_file_query_info(file, "standard", G_FILE_QUERY_INFO_NONE, NULL, &err);
> +    if (err != NULL) {
> +        /* In the case the file was not found, it just means that there's no
> +         * osinfo-db installed in the specified, directory. Let's just return
> +         * TRUE and proceed normally from here. */
> +        if (err->code == G_IO_ERROR_NOT_FOUND) {
> +            ret = TRUE;
> +            goto cleanup;
> +        }
> +        g_printerr("Failed to query info for the "VERSION_FILE" file: %s\n",
> +                   err->message);
> +        goto cleanup;
> +    }
> +

Is there a benefit to using string concatenation here? To me it looks 
less readable. Also placing 'file' after the filename makes it less 
readable too IMO: I'd have done:

         g_printerr("Failed to query info for file %s: %s\n",
                    VERSION_FILE, err->message);


> +    stream = g_file_read(file, NULL, &err);
> +    if (err != NULL) {
> +        g_printerr("Failed to read the "VERSION_FILE" file: %s\n",
> +                   err->message);

Similar.

The rest looks fine to me but my glib/gio fu is weak.

But I agree that we should get a libosinfo.org redirect set up for this 
before pushing, that gives us future flexibility.

Thanks,
Cole