Merge pull request #277 from sgallagher/mbs_fixes

Assorted fixes for MBS

Author: sgallagher

modulemd/v2/include/modulemd-2.0/modulemd-module-stream.h

@@ -107,14 +107,14 @@ modulemd_module_stream_new (guint64 mdversion,
  * modulemd_module_stream_read_file:
  * @path: (in): The path to a YAML document containing a module stream
  * definition.
+ * @strict: (in): Whether the parser should return failure if it encounters an
+ * unknown mapping key or if it should ignore it.
  * @module_name: (in) (nullable): An optional module name to override the
  * document on disk. Mostly useful in cases where the name is being
  * auto-detected from git.
  * @module_stream: (in) (nullable): An optional module stream name to override
  * the document on disk. Mostly useful in cases where the name is being
  * auto-detected from git.
- * @strict: (in): Whether the parser should return failure if it encounters an
- * unknown mapping key or if it should ignore it.
  * @error: (out): A #GError that will return the reason for a failed read.
  *
  * Create a #ModulemdModuleStream object from a YAML file.
@@ -139,14 +139,14 @@ modulemd_module_stream_read_file (const gchar *path,
  * modulemd_module_stream_read_string:
  * @yaml_string: (in): A YAML document string containing a module stream
  * definition.
+ * @strict: (in): Whether the parser should return failure if it encounters an
+ * unknown mapping key or if it should ignore it.
  * @module_name: (in) (nullable): An optional module name to override the
  * document on disk. Mostly useful in cases where the name is being
  * auto-detected from git.
  * @module_stream: (in) (nullable): An optional module stream name to override
  * the document on disk. Mostly useful in cases where the name is being
  * auto-detected from git.
- * @strict: (in): Whether the parser should return failure if it encounters an
- * unknown mapping key or if it should ignore it.
  * @error: (out): A #GError that will return the reason for a failed read.
  *
  * Create a #ModulemdModuleStream object from a YAML string.
@@ -171,14 +171,14 @@ modulemd_module_stream_read_string (const gchar *yaml_string,
  * modulemd_module_stream_read_stream: (skip)
  * @stream: (in): A YAML document as a FILE * containing a module stream
  * definition.
+ * @strict: (in): Whether the parser should return failure if it encounters an
+ * unknown mapping key or if it should ignore it.
  * @module_name: (in) (nullable): An optional module name to override the
  * document on disk. Mostly useful in cases where the name is being
  * auto-detected from git.
  * @module_stream: (in) (nullable): An optional module stream name to override
  * the document on disk. Mostly useful in cases where the name is being
  * auto-detected from git.
- * @strict: (in): Whether the parser should return failure if it encounters an
- * unknown mapping key or if it should ignore it.
  * @error: (out): A #GError that will return the reason for a failed read.
  *
  * Create a #ModulemdModuleStream object from a YAML file.

modulemd/v2/modulemd-module-stream-v1.c

@@ -861,7 +861,7 @@ modulemd_module_stream_v1_set_xmd (ModulemdModuleStreamV1 *self, GVariant *xmd)
   g_return_if_fail (MODULEMD_IS_MODULE_STREAM_V1 (self));
 
   g_clear_pointer (&self->xmd, g_variant_unref);
-  self->xmd = xmd;
+  self->xmd = modulemd_variant_deep_copy (xmd);
 }
 
 GVariant *
@@ -1069,7 +1069,7 @@ modulemd_module_stream_v1_copy (ModulemdModuleStream *self,
     copy, v1_self, servicelevels, modulemd_module_stream_v1_add_servicelevel);
 
   if (v1_self->xmd != NULL)
-    modulemd_module_stream_v1_set_xmd (copy, g_variant_ref (v1_self->xmd));
+    modulemd_module_stream_v1_set_xmd (copy, v1_self->xmd);
 
   return MODULEMD_MODULE_STREAM (g_steal_pointer (&copy));
 }
@@ -1421,7 +1421,7 @@ modulemd_module_stream_v1_parse_yaml (ModulemdSubdocumentInfo *subdoc,
                   return NULL;
                 }
               modulemd_module_stream_v1_set_xmd (modulestream, xmd);
-              xmd = NULL;
+              g_clear_pointer (&xmd, g_variant_unref);
             }
 
           /* Dependencies */

modulemd/v2/modulemd-util.c

@@ -271,15 +271,25 @@ modulemd_hash_table_unref (void *table)
   g_hash_table_unref ((GHashTable *)table);
 }
 
+
+static void
+destroy_variant_data (gpointer data)
+{
+  g_free (data);
+}
+
+
 GVariant *
 modulemd_variant_deep_copy (GVariant *variant)
 {
   const GVariantType *data_type = g_variant_get_type (variant);
   gsize data_size = g_variant_get_size (variant);
-  gconstpointer data = g_variant_get_data (variant);
+  gpointer data = g_malloc0 (data_size);
+
+  g_variant_store (variant, data);
 
-  return g_variant_ref_sink (
-    g_variant_new_from_data (data_type, data, data_size, TRUE, NULL, NULL));
+  return g_variant_ref_sink (g_variant_new_from_data (
+    data_type, data, data_size, FALSE, destroy_variant_data, data));
 }
 
 

modulemd/v2/modulemd-yaml-util.c

@@ -957,7 +957,8 @@ modulemd_yaml_emit_variant (yaml_emitter_t *emitter,
       g_set_error (error,
                    MODULEMD_YAML_ERROR,
                    MODULEMD_YAML_ERROR_EMIT,
-                   "Unhandled variant type");
+                   "Unhandled variant type: \"%s\"",
+                   g_variant_get_type_string (variant));
       return FALSE;
     }
   return TRUE;

modulemd/v2/tests/ModulemdTests/modulestream.py

@@ -528,10 +528,10 @@ def test_xmd(self):
 
             xmd_copy = stream.get_xmd()
             assert xmd_copy
-            assert 'outer_key' in xmd
-            assert 'scalar' in xmd['outer_key']
-            assert 'inner_key' in xmd['outer_key'][1]
-            assert xmd['outer_key'][1]['inner_key'] == 'another_scalar'
+            assert 'outer_key' in xmd_copy
+            assert 'scalar' in xmd_copy['outer_key']
+            assert 'inner_key' in xmd_copy['outer_key'][1]
+            assert xmd_copy['outer_key'][1]['inner_key'] == 'another_scalar'
 
     def test_upgrade(self):
         v1_stream = Modulemd.ModuleStreamV1.new("SuperModule", "latest")
@@ -1129,6 +1129,38 @@ def test_validate_buildafter(self):
                 "%s/modulemd/v2/tests/test_data/buildafter/invalid_key.yaml" %
                 (os.getenv('MESON_SOURCE_ROOT')), True)
 
+    def test_unicode_desc(self):
+        # Test a valid module stream with unicode in the description
+        stream = Modulemd.ModuleStream.read_file(
+            "%s/modulemd/v2/tests/test_data/stream_unicode.yaml" %
+            (os.getenv('MESON_SOURCE_ROOT')), True, '', '')
+
+        self.assertIsNotNone(stream)
+        self.assertTrue(stream.validate())
+
+    def test_xmd_issue_274(self):
+        # Test a valid module stream with unicode in the description
+        stream = Modulemd.ModuleStream.read_file(
+            "%s/modulemd/v2/tests/test_data/stream_unicode.yaml" %
+            (os.getenv('MESON_SOURCE_ROOT')), True, '', '')
+
+        # In this bug, we were getting a traceback attemping to call
+        # get_xmd() more than once on the same stream. There were subtle
+        # memory issues at play here.
+        if '_overrides_module' in dir(Modulemd):
+            # The XMD python tests can only be run against the installed lib
+            # because the overrides that translate between python and GVariant
+            # must be installed in /usr/lib/python*/site-packages/gi/overrides
+            # or they are not included when importing Modulemd
+
+            xmd = stream.get_xmd()
+            mbs_xmd = stream.get_xmd()['mbs']
+            mbs_xmd2 = stream.get_xmd()['mbs']
+
+        else:
+            stream.get_xmd()
+            stream.get_xmd()
+
 
 if __name__ == '__main__':
     unittest.main()

modulemd/v2/tests/test-modulemd-modulestream.c

@@ -911,6 +911,52 @@ module_stream_v2_test_rpm_map (ModuleStreamFixture *fixture,
   g_assert_true (modulemd_rpm_map_entry_equals (entry, retrieved_entry));
 }
 
+static void
+module_stream_v2_test_unicode_desc (void)
+{
+  g_autoptr (ModulemdModuleStream) stream = NULL;
+  g_autofree gchar *path = NULL;
+  g_autoptr (GError) error = NULL;
+
+  /* Test a module stream with unicode in description */
+  path = g_strdup_printf ("%s/modulemd/v2/tests/test_data/stream_unicode.yaml",
+                          g_getenv ("MESON_SOURCE_ROOT"));
+  g_assert_nonnull (path);
+
+  stream = modulemd_module_stream_read_file (path, TRUE, NULL, NULL, &error);
+  g_assert_nonnull (stream);
+  g_assert_no_error (error);
+}
+
+
+static void
+module_stream_v2_test_xmd_issue_274 (void)
+{
+  g_autoptr (ModulemdModuleStream) stream = NULL;
+  g_autofree gchar *path = NULL;
+  g_autoptr (GError) error = NULL;
+  GVariant *xmd1 = NULL;
+  GVariant *xmd2 = NULL;
+
+  path = g_strdup_printf ("%s/modulemd/v2/tests/test_data/stream_unicode.yaml",
+                          g_getenv ("MESON_SOURCE_ROOT"));
+  g_assert_nonnull (path);
+
+  stream = modulemd_module_stream_read_file (path, TRUE, NULL, NULL, &error);
+  g_assert_nonnull (stream);
+  g_assert_no_error (error);
+  g_assert_cmpint (modulemd_module_stream_get_mdversion (stream),
+                   ==,
+                   MD_MODULESTREAM_VERSION_ONE);
+
+  xmd1 =
+    modulemd_module_stream_v1_get_xmd (MODULEMD_MODULE_STREAM_V1 (stream));
+  xmd2 =
+    modulemd_module_stream_v1_get_xmd (MODULEMD_MODULE_STREAM_V1 (stream));
+
+  g_assert_true (xmd1 == xmd2);
+}
+
 
 int
 main (int argc, char *argv[])
@@ -1012,6 +1058,11 @@ main (int argc, char *argv[])
               module_stream_v2_test_rpm_map,
               NULL);
 
+  g_test_add_func ("/modulemd/v2/modulestream/v2/unicode/description",
+                   module_stream_v2_test_unicode_desc);
+
+  g_test_add_func ("/modulemd/v2/modulestream/v2/xmd/issue274",
+                   module_stream_v2_test_xmd_issue_274);
 
   return g_test_run ();
 }

modulemd/v2/tests/test_data/stream_unicode.yaml

@@ -0,0 +1,26 @@
+document: modulemd
+version: 1
+data:
+  description: Fedora 28 traditional base ’
+  name: platform
+  license:
+    module: [MIT]
+  profiles:
+    buildroot:
+      rpms: [bash, bzip2, coreutils, cpio, diffutils, fedora-release, findutils, gawk,
+        gcc, gcc-c++, grep, gzip, info, make, patch, redhat-rpm-config, rpm-build,
+        sed, shadow-utils, tar, unzip, util-linux, which, xz]
+    srpm-buildroot:
+      rpms: [bash, fedora-release, fedpkg-minimal, gnupg2, redhat-rpm-config, rpm-build,
+        shadow-utils]
+  stream: f28
+  summary: Fedora 28 traditional base
+  version: 3
+  context: 00000000
+  xmd:
+    mbs:
+      buildrequires: {}
+      commit: virtual
+      requires: {}
+      mse: true
+      koji_tag: module-f28-build