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

2018-11-17 Thread 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 
---
  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, 
);
+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, );
+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

___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo


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

2018-11-15 Thread Fabiano Fidêncio
Daniel,

On Tue, 2018-10-09 at 16:34 +0200, 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.

We have discussed this one during the "BoF" that happened during KVM
Forum.

Is this code fine by you? Considering it is, would be possible to have
something like latest.libosinfo.org (or similar) where we could fetch
the info from?

[snip]

Best Regards,
-- 
Fabiano Fidêncio

___
Libosinfo mailing list
Libosinfo@redhat.com
https://www.redhat.com/mailman/listinfo/libosinfo


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

2018-10-09 Thread Fabiano Fidêncio
--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.

Signed-off-by: Fabiano Fidêncio 
---
 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, 
);
+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;
+}
+
+stream = g_file_read(file, NULL, );
+if (err != NULL) {
+g_printerr("Failed to read the "VERSION_FILE" file: %s\n",
+   err->message);
+goto cleanup;
+}
+
+count = g_file_info_get_size(info);
+*version = g_malloc0(count*sizeof(gchar));
+if (*version == NULL)
+goto cleanup;
+
+if (!g_input_stream_read_all(G_INPUT_STREAM(stream),
+ (void *)*version,
+ count,
+ NULL,
+ NULL,
+ )) {
+g_printerr("Failed get the "VERSION_FILE" file content: %s\n",
+   err->message);
+goto cleanup;
+}
+
+ret = TRUE;
+
+ cleanup:
+g_object_unref(file);
+if (info != NULL)
+g_object_unref(info);
+if (stream != NULL)
+g_object_unref(stream);
+if (err != NULL)
+g_error_free(err);
+if (!ret)
+g_free(*version);
+
+return ret;
+}
+
+static gboolean osinfo_db_get_latest_info(gchar **version,
+  gchar **url)
+{
+GFile *uri = NULL;
+gchar **set = NULL;
+gchar *content = NULL;
+GError *err = NULL;
+gboolean ret = FALSE;
+
+uri = g_file_new_for_uri(LATEST_URI);
+if (uri == NULL)
+return FALSE;
+
+if (!g_file_load_contents(uri, NULL, , NULL, NULL, )) {
+g_printerr("Could not load the content of "LATEST_URI": %s\n",
+   err->message);
+goto cleanup;
+}
+
+/* As the osinfo-db-latest.txt file has the following format:
+ * ```
+ * 20180920
+ * https://releases.pagure.org/libosinfo/osinfo-db-20180920.tar.xz
+ * ```
+ *
+ * We can safely split the content by "\n" that the first element
+ * of the resulting array will be the version and the second will
+ * be the url. */
+set = g_strsplit(content, "\n", 0);
+if (set == NULL)
+goto cleanup;
+
+*version = g_strdup(set[0]);
+if (*version == NULL)
+goto cleanup;
+
+*url = g_strdup(set[1]);
+if (*url == NULL)
+goto cleanup;
+
+ret = TRUE;
+
+ cleanup:
+g_object_unref(uri);
+if (set != NULL)
+g_strfreev(set);
+if (ret != TRUE) {
+g_free(*version);
+g_free(*url);
+}
+
+return ret;
+}
+
 static int osinfo_db_import_extract(GFile *target,
 const char *source,
 gboolean verbose)
@@ -280,6 +404,8 @@ gint main(gint argc, gchar **argv)
 gboolean user = FALSE;
 gboolean local = FALSE;
 gboolean system = FALSE;
+gboolean latest = FALSE;
+gchar *latest_url = NULL;
 const gchar *root = "";
 const gchar *archive = NULL;
 const gchar *custom = NULL;
@@ -298,6 +424,8 @@ gint main(gint argc, gchar **argv)
 N_("Import into custom directory"), NULL, },
   { "root", 0, 0, G_OPTION_ARG_STRING, ,
 N_("Installation root directory"), NULL, },
+  { "latest", 0, 0,