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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
  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

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux