API1: Don't fail merges when default streams differ

Instead of failing the merge on a default stream conflict, we will
instead treat it as having no default set. This is safe because if
the module is not yet installed, then this just means its packages
aren't visible and it needs to be selected explicitly.

If some packages from it are already installed, then the module
stream is already set, and thus changing the default won't matter.

Resolves: rhbz#1666871

Signed-off-by: Stephen Gallagher <[email protected]>

Author: sgallagher

modulemd/v1/include/modulemd-1.0/private/modulemd-private.h

@@ -26,6 +26,8 @@ enum
 
 #define MD_VERSION_LATEST MD_VERSION_2
 
+#define DEFAULT_MERGE_CONFLICT "__merge_conflict__"
+
 ModulemdModule *
 modulemd_module_new_from_modulestream (ModulemdModuleStream *stream);
 

modulemd/v1/modulemd-defaults.c

@@ -14,6 +14,7 @@
 #include "modulemd.h"
 #include "modulemd-defaults.h"
 #include "modulemd-simpleset.h"
+#include "private/modulemd-private.h"
 #include "private/modulemd-yaml.h"
 
 
@@ -148,6 +149,15 @@ modulemd_defaults_peek_default_stream (ModulemdDefaults *self)
 {
   g_return_val_if_fail (self, NULL);
 
+  if (self->default_stream &&
+      g_str_equal (self->default_stream, DEFAULT_MERGE_CONFLICT))
+    {
+      /* During an index merge, we determined that this was in conflict
+       * with another set of ModulemdDefaults for the same module. If we
+       * see this, treat it as no default stream when querying for it.
+       */
+      return NULL;
+    }
   return self->default_stream;
 }
 
@@ -157,6 +167,16 @@ modulemd_defaults_dup_default_stream (ModulemdDefaults *self)
 {
   g_return_val_if_fail (self, NULL);
 
+  if (self->default_stream &&
+      g_str_equal (self->default_stream, DEFAULT_MERGE_CONFLICT))
+    {
+      /* During an index merge, we determined that this was in conflict
+       * with another set of ModulemdDefaults for the same module. If we
+       * see this, treat it as no default stream when querying for it.
+       */
+      return NULL;
+    }
+
   return g_strdup (self->default_stream);
 }
 
@@ -755,24 +775,21 @@ modulemd_defaults_merge (ModulemdDefaults *first,
    * Merge them as best we can.
    */
 
+  defaults = modulemd_defaults_copy (first);
+
   /* First check for incompatibilities with the streams */
-  if (g_strcmp0 (modulemd_defaults_peek_default_stream (first),
-                 modulemd_defaults_peek_default_stream (second)))
+  if (g_strcmp0 (first->default_stream, second->default_stream))
     {
       /* Default streams don't match and override is not set.
        * Return an error
        */
-      g_set_error (
-        error,
-        MODULEMD_DEFAULTS_ERROR,
-        MODULEMD_DEFAULTS_ERROR_CONFLICTING_STREAMS,
-        "Conflicting default streams when merging defaults for module %s",
-        modulemd_defaults_peek_module_name (first));
-      return NULL;
+      /* They have conflicting default streams */
+      g_info ("Module stream mismatch in merge: %s != %s",
+              first->default_stream,
+              second->default_stream);
+      modulemd_defaults_set_default_stream (defaults, DEFAULT_MERGE_CONFLICT);
     }
 
-  defaults = modulemd_defaults_copy (first);
-
   /* Merge the profile defaults */
   profile_defaults = modulemd_defaults_peek_profile_defaults (defaults);
 

modulemd/v1/tests/test-modulemd-defaults.c

@@ -604,6 +604,7 @@ modulemd_defaults_test_prioritizer (DefaultsFixture *fixture,
   g_autofree gchar *yaml_override_path = NULL;
   g_autoptr (GPtrArray) base_objects = NULL;
   g_autoptr (GPtrArray) override_objects = NULL;
+  g_autoptr (GPtrArray) override_nodejs_objects = NULL;
   g_autoptr (GPtrArray) merged_objects = NULL;
   g_autoptr (ModulemdPrioritizer) prioritizer = NULL;
   g_autoptr (GError) error = NULL;
@@ -620,6 +621,18 @@ modulemd_defaults_test_prioritizer (DefaultsFixture *fixture,
   g_assert_nonnull (base_objects);
   g_assert_cmpint (base_objects->len, ==, 7);
 
+
+  yaml_override_path =
+    g_strdup_printf ("%s/test_data/defaults/overriding-nodejs.yaml",
+                     g_getenv ("MESON_SOURCE_ROOT"));
+  g_assert_nonnull (yaml_override_path);
+
+  override_nodejs_objects =
+    modulemd_objects_from_file (yaml_override_path, &error);
+  g_clear_pointer (&yaml_override_path, g_free);
+  g_assert_nonnull (override_nodejs_objects);
+  g_assert_cmpint (override_nodejs_objects->len, ==, 1);
+
   yaml_override_path = g_strdup_printf (
     "%s/test_data/defaults/overriding.yaml", g_getenv ("MESON_SOURCE_ROOT"));
   g_assert_nonnull (yaml_override_path);
@@ -661,6 +674,48 @@ modulemd_defaults_test_prioritizer (DefaultsFixture *fixture,
   g_assert_true (result);
 
 
+  /*
+   * Test that importing the nodejs overrides at the same priority level fails.
+   *
+   * This YAML has a conflicting default stream which should be ignored and set
+   * to "no default stream".
+   */
+
+  result = modulemd_prioritizer_add (
+    prioritizer, override_nodejs_objects, prio, &error);
+  g_assert_true (result);
+
+  merged_objects = modulemd_prioritizer_resolve (prioritizer, &error);
+  g_assert_nonnull (merged_objects);
+  g_assert_cmpint (merged_objects->len, ==, 3);
+
+  for (gint i = 0; i < merged_objects->len; i++)
+    {
+      if (MODULEMD_IS_DEFAULTS (g_ptr_array_index (merged_objects, i)))
+        {
+          defaults = g_ptr_array_index (merged_objects, i);
+          if (!g_strcmp0 (modulemd_defaults_peek_module_name (defaults),
+                          "nodejs"))
+            {
+              g_assert_null (modulemd_defaults_peek_default_stream (defaults));
+            }
+        }
+    }
+
+  g_clear_pointer (&merged_objects, g_ptr_array_unref);
+
+
+  /* Start over and test profile conflicts */
+  g_clear_pointer (&prioritizer, g_object_unref);
+  prioritizer = modulemd_prioritizer_new ();
+  result = modulemd_prioritizer_add (prioritizer, base_objects, prio, &error);
+  if (!result)
+    {
+      fprintf (stderr, "Merge error: %s", error->message);
+    }
+  g_assert_true (result);
+
+
   /*
    * Test that importing the overrides at the same priority level fails.
    *
@@ -673,7 +728,7 @@ modulemd_defaults_test_prioritizer (DefaultsFixture *fixture,
   g_assert_cmpstr (
     g_quark_to_string (error->domain), ==, "modulemd-defaults-error-quark");
   g_assert_cmpint (
-    error->code, ==, MODULEMD_DEFAULTS_ERROR_CONFLICTING_STREAMS);
+    error->code, ==, MODULEMD_DEFAULTS_ERROR_CONFLICTING_PROFILES);
   g_clear_pointer (&error, g_error_free);
 
   /* The object's internal state is undefined after an error, so delete it */
@@ -989,7 +1044,7 @@ modulemd_defaults_test_index_prioritizer (DefaultsFixture *fixture,
   g_assert_cmpstr (
     g_quark_to_string (error->domain), ==, "modulemd-defaults-error-quark");
   g_assert_cmpint (
-    error->code, ==, MODULEMD_DEFAULTS_ERROR_CONFLICTING_STREAMS);
+    error->code, ==, MODULEMD_DEFAULTS_ERROR_CONFLICTING_PROFILES);
   g_clear_pointer (&error, g_error_free);
 
   /* The object's internal state is undefined after an error, so delete it */
@@ -1140,6 +1195,11 @@ modulemd_regressions_issue44 (DefaultsFixture *fixture,
 
   /* Add another almost-identical document, except with a conflicting default
    * stream set.
+   *
+   * NOTE: when this was written (for issue 44 on GitHub), this was meant to be
+   * a hard error. As of 1.8.1 we expect this to just result in having no
+   * default stream for this module. This test has been slightly modified so
+   * that the expected result is now a pass.
    */
   yaml_conflicting_path = g_strdup_printf (
     "%s/test_data/defaults/issue44-2.yaml", g_getenv ("MESON_SOURCE_ROOT"));
@@ -1151,7 +1211,7 @@ modulemd_regressions_issue44 (DefaultsFixture *fixture,
 
   result =
     modulemd_prioritizer_add (prioritizer, conflicting_objects, 0, &error);
-  g_assert_false (result);
+  g_assert_true (result);
 }
 
 

test_data/defaults/overriding-nodejs.yaml

@@ -0,0 +1,10 @@
+---
+document: modulemd-defaults
+version: 1
+data:
+    module: nodejs
+    stream: '9.0'
+    profiles:
+        '6.0': [default]
+        '8.0': [super]
+        '9.0': [supermegaultra]