compose: Use GVariantDict, not manual hash table

There's a dedicated type for a mapping from "string to variant" in
`GVariantDict`.  Use it instead of rolling our own with `GHashTable`.
The advantage here is improved ergonomics from the C side; e.g.
we can directly pass C strings for values and have the variant
allocated internally.

But more importantly, this is prep for oxidizing things here because
we can pass `GVariantDict` to/from Rust easily, not so with
`GHashTable`.

Author: cgwalters

src/app/rpmostree-compose-builtin-tree.cxx

@@ -155,8 +155,8 @@ typedef struct
 {
   RpmOstreeContext *corectx;
   GFile *treefile_path;
-  GHashTable *metadata;
-  GHashTable *detached_metadata;
+  GVariantDict *metadata;
+  GVariantDict *detached_metadata;
   gboolean failed;
   GLnxTmpDir workdir_tmp;
   int workdir_dfd;
@@ -181,7 +181,8 @@ rpm_ostree_tree_compose_context_free (RpmOstreeTreeComposeContext *ctx)
 {
   g_clear_object (&ctx->corectx);
   g_clear_object (&ctx->treefile_path);
-  g_clear_pointer (&ctx->metadata, g_hash_table_unref);
+  g_clear_pointer (&ctx->metadata, g_variant_dict_unref);
+  g_clear_pointer (&ctx->detached_metadata, g_variant_dict_unref);
   ctx->treefile_rs.~optional ();
   /* Only close workdir_dfd if it's not owned by the tmpdir */
   if (!ctx->workdir_tmp.initialized)
@@ -515,17 +516,17 @@ install_packages (RpmOstreeTreeComposeContext *self, gboolean *out_unmodified,
 }
 
 static gboolean
-parse_metadata_keyvalue_strings (char **strings, GHashTable *metadata_hash, GError **error)
+parse_metadata_keyvalue_strings (char **strings, GVariantDict *metadata_hash, GError **error)
 {
   for (char **iter = strings; *iter; iter++)
     {
       const char *s = *iter;
       const char *eq = strchr (s, '=');
       if (!eq)
         return glnx_throw (error, "Missing '=' in KEY=VALUE metadata '%s'", s);
+      g_autofree char *k = g_strndup (s, eq - s);
 
-      g_hash_table_insert (metadata_hash, g_strndup (s, eq - s),
-                           g_variant_ref_sink (g_variant_new_string (eq + 1)));
+      g_variant_dict_insert (metadata_hash, k, "s", eq + 1);
     }
 
   return TRUE;
@@ -762,10 +763,8 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, const char *basear
 
   self->treefile = json_node_get_object (self->treefile_rootval);
 
-  self->metadata
-      = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify)g_variant_unref);
-  self->detached_metadata
-      = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify)g_variant_unref);
+  self->metadata = g_variant_dict_new (NULL);
+  self->detached_metadata = g_variant_dict_new (NULL);
 
   /* metadata from the treefile itself has the lowest priority */
   JsonNode *add_commit_metadata_node
@@ -805,8 +804,8 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, const char *basear
         {
           g_variant_builder_add (&builder, "s", s.c_str ());
         }
-      g_hash_table_insert (self->metadata, g_strdup ("ostree.container-cmd"),
-                           g_variant_ref_sink (g_variant_builder_end (&builder)));
+      g_variant_dict_insert_value (self->metadata, "ostree.container-cmd",
+                                   g_variant_builder_end (&builder));
     }
 
   auto layers = (*self->treefile_rs)->get_all_ostree_layers ();
@@ -836,8 +835,7 @@ inject_advisories (RpmOstreeTreeComposeContext *self, GCancellable *cancellable,
   g_autoptr (GVariant) advisories = rpmostree_advisories_variant (yum_sack, pkgs);
 
   if (advisories && g_variant_n_children (advisories) > 0)
-    g_hash_table_insert (self->metadata, g_strdup ("rpmostree.advisories"),
-                         g_steal_pointer (&advisories));
+    g_variant_dict_insert_value (self->metadata, "rpmostree.advisories", advisories);
 
   return TRUE;
 }
@@ -918,7 +916,7 @@ impl_install_tree (RpmOstreeTreeComposeContext *self, gboolean *out_changed,
   rust::String next_version;
   if (json_object_has_member (self->treefile, "automatic-version-prefix") &&
       /* let --add-metadata-string=version=... take precedence */
-      !g_hash_table_contains (self->metadata, OSTREE_COMMIT_META_KEY_VERSION))
+      !g_variant_dict_contains (self->metadata, OSTREE_COMMIT_META_KEY_VERSION))
     {
       CXX_TRY_VAR (ver_prefix, (*self->treefile_rs)->require_automatic_version_prefix (), error);
       auto ver_suffix = (*self->treefile_rs)->get_automatic_version_suffix ();
@@ -941,17 +939,15 @@ impl_install_tree (RpmOstreeTreeComposeContext *self, gboolean *out_changed,
                                                     last_version ?: ""),
                    error);
       next_version = std::move (next_versionv);
-      g_hash_table_insert (self->metadata, g_strdup (OSTREE_COMMIT_META_KEY_VERSION),
-                           g_variant_ref_sink (g_variant_new_string (next_version.c_str ())));
+      g_variant_dict_insert (self->metadata, OSTREE_COMMIT_META_KEY_VERSION, "s",
+                             next_version.c_str ());
     }
   else
     {
-      gpointer vp = g_hash_table_lookup (self->metadata, OSTREE_COMMIT_META_KEY_VERSION);
-      auto v = static_cast<GVariant *> (vp);
-      if (v)
+      const char *version = NULL;
+      if (g_variant_dict_lookup (self->metadata, "&s", OSTREE_COMMIT_META_KEY_VERSION, &version))
         {
-          g_assert (g_variant_is_of_type (v, G_VARIANT_TYPE_STRING));
-          next_version = rust::String (static_cast<char *> (g_variant_dup_string (v, NULL)));
+          next_version = rust::String (version);
         }
     }
 
@@ -991,24 +987,26 @@ impl_install_tree (RpmOstreeTreeComposeContext *self, gboolean *out_changed,
       = (*self->treefile_rs)->get_repo_metadata_target ();
   if (repomd_target == rpmostreecxx::RepoMetadataTarget::Inline)
     {
-      if (!g_hash_table_contains (self->metadata, "rpmostree.rpmmd-repos"))
+      if (!g_variant_dict_contains (self->metadata, "rpmostree.rpmmd-repos"))
         {
-          g_hash_table_insert (self->metadata, g_strdup ("rpmostree.rpmmd-repos"),
-                               rpmostree_context_get_rpmmd_repo_commit_metadata (self->corectx));
+          g_autoptr (GVariant) meta
+              = rpmostree_context_get_rpmmd_repo_commit_metadata (self->corectx);
+          g_variant_dict_insert_value (self->metadata, "rpmostree.rpmmd-repos", meta);
         }
     }
   else if (repomd_target == rpmostreecxx::RepoMetadataTarget::Detached)
     {
-      if (!g_hash_table_contains (self->detached_metadata, "rpmostree.rpmmd-repos"))
+      if (!g_variant_dict_contains (self->detached_metadata, "rpmostree.rpmmd-repos"))
         {
-          g_hash_table_insert (self->detached_metadata, g_strdup ("rpmostree.rpmmd-repos"),
-                               rpmostree_context_get_rpmmd_repo_commit_metadata (self->corectx));
+          g_autoptr (GVariant) meta
+              = rpmostree_context_get_rpmmd_repo_commit_metadata (self->corectx);
+          g_variant_dict_insert_value (self->detached_metadata, "rpmostree.rpmmd-repos", meta);
         }
     }
   else
     {
-      g_hash_table_remove (self->metadata, "rpmostree.rpmmd-repos");
-      g_hash_table_remove (self->detached_metadata, "rpmostree.rpmmd-repos");
+      g_variant_dict_remove (self->metadata, "rpmostree.rpmmd-repos");
+      g_variant_dict_remove (self->detached_metadata, "rpmostree.rpmmd-repos");
     }
 
   if (!inject_advisories (self, cancellable, error))
@@ -1057,8 +1055,7 @@ impl_install_tree (RpmOstreeTreeComposeContext *self, gboolean *out_changed,
   }
 
   /* Insert our input hash */
-  g_hash_table_replace (self->metadata, g_strdup ("rpmostree.inputhash"),
-                        g_variant_ref_sink (g_variant_new_string (new_inputhash)));
+  g_variant_dict_insert (self->metadata, "rpmostree.inputhash", "s", new_inputhash);
 
   *out_changed = TRUE;
   return TRUE;
@@ -1120,8 +1117,7 @@ impl_commit_tree (RpmOstreeTreeComposeContext *self, GCancellable *cancellable,
       GVariant *v = json_gvariant_deserialize (initramfs_args, "as", error);
       if (!v)
         return FALSE;
-      g_hash_table_insert (self->metadata, g_strdup ("rpmostree.initramfs-args"),
-                           g_variant_ref_sink (v));
+      g_variant_dict_insert_value (self->metadata, "rpmostree.initramfs-args", v);
     }
 
   /* Convert metadata hash tables to GVariants */

src/app/rpmostree-composeutil.cxx

@@ -64,7 +64,7 @@ rpmostree_composeutil_checksum (HyGoal goal, OstreeRepo *repo, const rpmostreecx
 }
 
 gboolean
-rpmostree_composeutil_read_json_metadata (JsonNode *root, GHashTable *metadata, GError **error)
+rpmostree_composeutil_read_json_metadata (JsonNode *root, GVariantDict *metadata, GError **error)
 {
   g_autoptr (GVariant) jsonmetav = json_gvariant_deserialize (root, "a{sv}", error);
   if (!jsonmetav)
@@ -76,7 +76,7 @@ rpmostree_composeutil_read_json_metadata (JsonNode *root, GHashTable *metadata,
     char *key;
     GVariant *value;
     while (g_variant_iter_loop (&viter, "{sv}", &key, &value))
-      g_hash_table_replace (metadata, g_strdup (key), g_variant_ref (value));
+      g_variant_dict_insert_value (metadata, key, value);
   }
 
   return TRUE;
@@ -86,7 +86,7 @@ rpmostree_composeutil_read_json_metadata (JsonNode *root, GHashTable *metadata,
  * to a hash table of a{sv}; suitable for further extension.
  */
 gboolean
-rpmostree_composeutil_read_json_metadata_from_file (const char *path, GHashTable *metadata,
+rpmostree_composeutil_read_json_metadata_from_file (const char *path, GVariantDict *metadata,
                                                     GError **error)
 {
   const char *errprefix = glnx_strjoina ("While parsing JSON file ", path);
@@ -101,19 +101,31 @@ rpmostree_composeutil_read_json_metadata_from_file (const char *path, GHashTable
 }
 
 static GVariantBuilder *
-metadata_conversion_start (GHashTable *metadata)
+variant_dict_to_builder (GVariantDict *metadata)
 {
-  GVariantBuilder *builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
-
-  GLNX_HASH_TABLE_FOREACH_KV (metadata, const char *, strkey, GVariant *, v)
-    g_variant_builder_add (builder, "{sv}", strkey, v);
+  bool found_value = false;
+  g_autoptr (GVariant) final_metadata = g_variant_ref_sink (g_variant_dict_end (metadata));
+  GVariantIter viter;
+  g_variant_iter_init (&viter, final_metadata);
+  char *key;
+  GVariant *value;
+  g_autoptr (GVariantBuilder) builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
+  while (g_variant_iter_loop (&viter, "{sv}", &key, &value))
+    {
+      found_value = true;
+      g_variant_builder_add (builder, "{sv}", key, value);
+    }
 
-  return builder;
+  if (found_value)
+    return util::move_nullify (builder);
+  return NULL;
 }
 
 static GVariant *
 metadata_conversion_end (GVariantBuilder *builder)
 {
+  if (!builder)
+    return NULL;
   g_autoptr (GVariant) ret = g_variant_ref_sink (g_variant_builder_end (builder));
   /* Canonicalize to big endian, like OSTree does. Without this, any numbers
    * we place in the metadata will be unreadable since clients won't know
@@ -126,9 +138,11 @@ metadata_conversion_end (GVariantBuilder *builder)
 
 /* Convert hash table of metadata into finalized GVariant */
 GVariant *
-rpmostree_composeutil_finalize_metadata (GHashTable *metadata, int rootfs_dfd, GError **error)
+rpmostree_composeutil_finalize_metadata (GVariantDict *metadata, int rootfs_dfd, GError **error)
 {
-  g_autoptr (GVariantBuilder) builder = metadata_conversion_start (metadata);
+  g_autoptr (GVariantBuilder) builder = variant_dict_to_builder (metadata);
+  if (!builder)
+    builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
 
   /* include list of packages in rpmdb; this is used client-side for easily previewing
    * pending updates. once we only support unified core composes, this can easily be much
@@ -143,13 +157,9 @@ rpmostree_composeutil_finalize_metadata (GHashTable *metadata, int rootfs_dfd, G
 
 /* Convert hash table of metadata into finalized GVariant */
 GVariant *
-rpmostree_composeutil_finalize_detached_metadata (GHashTable *detached_metadata)
+rpmostree_composeutil_finalize_detached_metadata (GVariantDict *detached_metadata)
 {
-  /* canonicalize empty detached metadata to NULL */
-  if (g_hash_table_size (detached_metadata) == 0)
-    return NULL;
-
-  g_autoptr (GVariantBuilder) builder = metadata_conversion_start (detached_metadata);
+  g_autoptr (GVariantBuilder) builder = variant_dict_to_builder (detached_metadata);
   return metadata_conversion_end (builder);
 }
 

src/app/rpmostree-composeutil.h

@@ -31,15 +31,16 @@ gboolean rpmostree_composeutil_checksum (HyGoal goal, OstreeRepo *repo,
                                          const rpmostreecxx::Treefile &tf, JsonObject *treefile,
                                          char **out_checksum, GError **error);
 
-gboolean rpmostree_composeutil_read_json_metadata (JsonNode *root, GHashTable *metadata,
+gboolean rpmostree_composeutil_read_json_metadata (JsonNode *root, GVariantDict *metadata,
                                                    GError **error);
-gboolean rpmostree_composeutil_read_json_metadata_from_file (const char *path, GHashTable *metadata,
+gboolean rpmostree_composeutil_read_json_metadata_from_file (const char *path,
+                                                             GVariantDict *metadata,
                                                              GError **error);
 
-GVariant *rpmostree_composeutil_finalize_metadata (GHashTable *metadata, int rootfs_dfd,
+GVariant *rpmostree_composeutil_finalize_metadata (GVariantDict *metadata, int rootfs_dfd,
                                                    GError **error);
 
-GVariant *rpmostree_composeutil_finalize_detached_metadata (GHashTable *detached_metadata);
+GVariant *rpmostree_composeutil_finalize_detached_metadata (GVariantDict *detached_metadata);
 
 gboolean rpmostree_composeutil_write_composejson (OstreeRepo *repo, const char *path,
                                                   const OstreeRepoTransactionStats *stats,