Enable component 'buildorder' for modulemd-packager v2 and v3 documents

Resolves: fedora-modularity#539

Signed-off-by: Merlin Mathesius <[email protected]>

modulemd/include/private/modulemd-component-rpm-private.h

@@ -33,9 +33,6 @@
  * @name: (in): A string with the name of the component.
  * @strict: (in): Whether the parser should return failure if it encounters an
  * unknown mapping key or if it should ignore it.
- * @only_packager: (in): Whether the parser should return failure if it
- * encounters a mapping key that is not intended for packager use. Ignored
- * if @strict is #FALSE.
  * @error: (out): A #GError that will return the reason for parsing error.
  *
  * Returns: (transfer full): A newly-allocated #ModulemdComponentRpm object
@@ -48,7 +45,6 @@ ModulemdComponentRpm *
 modulemd_component_rpm_parse_yaml (yaml_parser_t *parser,
                                    const gchar *name,
                                    gboolean strict,
-                                   gboolean only_packager,
                                    GError **error);
 
 

modulemd/modulemd-component-rpm.c

@@ -649,7 +649,6 @@ ModulemdComponentRpm *
 modulemd_component_rpm_parse_yaml (yaml_parser_t *parser,
                                    const gchar *name,
                                    gboolean strict,
-                                   gboolean only_packager,
                                    GError **error)
 {
   MODULEMD_INIT_TRACE ();
@@ -857,8 +856,7 @@ modulemd_component_rpm_parse_yaml (yaml_parser_t *parser,
                 }
             }
           else if (g_str_equal ((const gchar *)event.data.scalar.value,
-                                "buildorder") &&
-                   !only_packager)
+                                "buildorder"))
             {
               buildorder = modulemd_yaml_parse_int64 (parser, &nested_error);
               if (buildorder == 0 && nested_error != NULL)

modulemd/modulemd-module-stream-v1.c

@@ -2420,7 +2420,6 @@ modulemd_module_stream_v1_parse_rpm_components (
             parser,
             (const gchar *)event.data.scalar.value,
             strict,
-            FALSE,
             &nested_error);
           if (!component)
             {

modulemd/modulemd-module-stream-v2.c

@@ -1778,7 +1778,6 @@ modulemd_module_stream_v2_parse_components (
   yaml_parser_t *parser,
   ModulemdModuleStreamV2 *modulestream,
   gboolean strict,
-  gboolean only_packager,
   GError **error);
 
 static gboolean
@@ -2064,11 +2063,8 @@ modulemd_module_stream_v2_parse_yaml (ModulemdSubdocumentInfo *subdoc,
           else if (g_str_equal ((const gchar *)event.data.scalar.value,
                                 "components"))
             {
-              if (!modulemd_module_stream_v2_parse_components (&parser,
-                                                               modulestream,
-                                                               strict,
-                                                               only_packager,
-                                                               &nested_error))
+              if (!modulemd_module_stream_v2_parse_components (
+                    &parser, modulestream, strict, &nested_error))
                 {
                   g_propagate_error (error, g_steal_pointer (&nested_error));
                   return NULL;
@@ -2527,7 +2523,6 @@ modulemd_module_stream_v2_parse_rpm_components (
   yaml_parser_t *parser,
   ModulemdModuleStreamV2 *modulestream,
   gboolean strict,
-  gboolean only_packager,
   GError **error);
 static gboolean
 modulemd_module_stream_v2_parse_module_components (
@@ -2542,7 +2537,6 @@ modulemd_module_stream_v2_parse_components (
   yaml_parser_t *parser,
   ModulemdModuleStreamV2 *modulestream,
   gboolean strict,
-  gboolean only_packager,
   GError **error)
 {
   MODULEMD_INIT_TRACE ();
@@ -2576,11 +2570,7 @@ modulemd_module_stream_v2_parse_components (
           if (g_str_equal ((const gchar *)event.data.scalar.value, "rpms"))
             {
               if (!modulemd_module_stream_v2_parse_rpm_components (
-                    parser,
-                    modulestream,
-                    strict,
-                    only_packager,
-                    &nested_error))
+                    parser, modulestream, strict, &nested_error))
                 {
                   g_propagate_error (error, g_steal_pointer (&nested_error));
                   return FALSE;
@@ -2629,7 +2619,6 @@ modulemd_module_stream_v2_parse_rpm_components (
   yaml_parser_t *parser,
   ModulemdModuleStreamV2 *modulestream,
   gboolean strict,
-  gboolean only_packager,
   GError **error)
 {
   MODULEMD_INIT_TRACE ();
@@ -2664,7 +2653,6 @@ modulemd_module_stream_v2_parse_rpm_components (
             parser,
             (const gchar *)event.data.scalar.value,
             strict,
-            only_packager,
             &nested_error);
           if (!component)
             {

modulemd/modulemd-packager-v3.c

@@ -1663,7 +1663,6 @@ modulemd_packager_v3_parse_rpm_components (yaml_parser_t *parser,
             parser,
             (const gchar *)event.data.scalar.value,
             strict,
-            FALSE,
             &nested_error);
           if (!component)
             {

modulemd/tests/ModulemdTests/modulepackager.py

@@ -128,41 +128,6 @@ def test_read_extra_values(self):
             )
         )
 
-        # Handle buildorder as a special case as well
-        minimal_plus_extra = deepcopy(minimal_valid)
-        minimal_plus_extra["data"]["components"] = {
-            "rpms": {
-                "foo": {
-                    "rationale": "A dependency of bar that must be built first",
-                    "ref": "master",
-                },
-                "bar": {
-                    "rationale": "The important package",
-                    "ref": "master",
-                    "buildorder": 10,
-                },
-            }
-        }
-
-        logging.debug(
-            "YAML: {}".format(json.dumps(minimal_plus_extra, indent=2))
-        )
-
-        # This code takes advantage of the fact that JSON is a proper
-        # subset of YAML, so we can avoid requiring pyYAML for the tests.
-        with self.assertRaises(gi.repository.GLib.GError) as cm:
-            stream = Modulemd.ModuleStream.read_string(
-                json.dumps(minimal_plus_extra), True
-            )
-
-        gerror = cm.exception
-        self.assertTrue(
-            gerror.matches(
-                domain=Modulemd.yaml_error_quark(),
-                code=Modulemd.YamlError.UNKNOWN_ATTR,
-            )
-        )
-
     def test_fail_v1(self):
         minimal_v1 = {
             "document": "modulemd-packager",

modulemd/tests/test-modulemd-component-rpm.c

@@ -419,7 +419,7 @@ component_rpm_test_parse_yaml (void)
   g_assert_cmpint (event.type, ==, YAML_SCALAR_EVENT);
   yaml_event_delete (&event);
 
-  r = modulemd_component_rpm_parse_yaml (&parser, "bar", TRUE, FALSE, &error);
+  r = modulemd_component_rpm_parse_yaml (&parser, "bar", TRUE, &error);
   g_assert_nonnull (r);
   g_assert_true (MODULEMD_IS_COMPONENT_RPM (r));
   g_assert_cmpstr (
@@ -479,7 +479,7 @@ component_rpm_test_parse_packager_yaml (void)
   g_assert_cmpint (event.type, ==, YAML_SCALAR_EVENT);
   yaml_event_delete (&event);
 
-  r = modulemd_component_rpm_parse_yaml (&parser, "bar", TRUE, TRUE, &error);
+  r = modulemd_component_rpm_parse_yaml (&parser, "bar", TRUE, &error);
   g_assert_no_error (error);
   g_assert_nonnull (r);
   g_assert_true (MODULEMD_IS_COMPONENT_RPM (r));
@@ -516,39 +516,6 @@ component_rpm_test_parse_packager_yaml (void)
 }
 
 
-static void
-component_rpm_test_parse_buildorder_packager_yaml (void)
-{
-  g_autoptr (ModulemdComponentRpm) r = NULL;
-  g_autoptr (GError) error = NULL;
-  int result;
-  MMD_INIT_YAML_EVENT (event);
-  MMD_INIT_YAML_PARSER (parser);
-  g_autofree gchar *yaml_path = NULL;
-  g_autoptr (FILE) yaml_stream = NULL;
-
-  /* First test a valid component RPM */
-  yaml_path =
-    g_strdup_printf ("%s/component-rpm/cr.yaml", g_getenv ("TEST_DATA_PATH"));
-  g_assert_nonnull (yaml_path);
-
-  yaml_stream = g_fopen (yaml_path, "rbe");
-  g_assert_nonnull (yaml_stream);
-
-  yaml_parser_set_input_file (&parser, yaml_stream);
-
-  parser_skip_headers (&parser);
-  result = yaml_parser_parse (&parser, &event);
-  g_assert_cmpint (result, ==, 1);
-  g_assert_cmpint (event.type, ==, YAML_SCALAR_EVENT);
-  yaml_event_delete (&event);
-
-  r = modulemd_component_rpm_parse_yaml (&parser, "bar", TRUE, TRUE, &error);
-  g_assert_error (error, MODULEMD_YAML_ERROR, MMD_YAML_ERROR_UNKNOWN_ATTR);
-  g_assert_null (r);
-}
-
-
 static void
 component_rpm_test_override_name (void)
 {
@@ -616,9 +583,6 @@ main (int argc, char *argv[])
   g_test_add_func ("/modulemd/v2/component/rpm/yaml/parse/packager",
                    component_rpm_test_parse_packager_yaml);
 
-  g_test_add_func ("/modulemd/v2/component/rpm/yaml/parse/packager/buildorder",
-                   component_rpm_test_parse_buildorder_packager_yaml);
-
   g_test_add_func ("/modulemd/v2/component/rpm/override_name",
                    component_rpm_test_override_name);
 

modulemd/tests/test-modulemd-packager-v3.c

@@ -377,23 +377,21 @@ validate_yaml (ModulemdPackagerV3 *packager)
     "        repository: https://pagure.io/bar.git\n"
     "        cache: https://example.com/cache\n"
     "        ref: 26ca0c0\n"
-    "        buildafter:\n"
-    "        - baz\n"
     "      baz:\n"
     "        rationale: Demonstrate updating the buildroot contents.\n"
     "        buildroot: true\n"
     "        srpm-buildroot: true\n"
     "      xxx:\n"
     "        rationale: xxx demonstrates arches and multilib.\n"
-    "        buildafter:\n"
-    "        - bar\n"
+    "        buildorder: 10\n"
     "        arches: [i686, x86_64]\n"
     "        multilib: [x86_64]\n"
     "    modules:\n"
     "      includedmodule:\n"
     "        rationale: Included in the stack, just because.\n"
     "        repository: https://pagure.io/includedmodule.git\n"
     "        ref: somecoolbranchname\n"
+    "        buildorder: 100\n"
     "...\n");
 
   /* re-parse the YAML string and verify everything is still there */

modulemd/tests/test_data/upgrades/packager_v3_to_index.yaml

@@ -78,23 +78,21 @@ data:
         repository: https://pagure.io/bar.git
         cache: https://example.com/cache
         ref: 26ca0c0
-        buildafter:
-        - baz
       baz:
         rationale: Demonstrate updating the buildroot contents.
         buildroot: true
         srpm-buildroot: true
       xxx:
         rationale: xxx demonstrates arches and multilib.
-        buildafter:
-        - bar
+        buildorder: 10
         arches: [i686, x86_64]
         multilib: [x86_64]
     modules:
       includedmodule:
         rationale: Included in the stack, just because.
         repository: https://pagure.io/includedmodule.git
         ref: somecoolbranchname
+        buildorder: 100
 ...
 ---
 document: modulemd
@@ -154,21 +152,19 @@ data:
         repository: https://pagure.io/bar.git
         cache: https://example.com/cache
         ref: 26ca0c0
-        buildafter:
-        - baz
       baz:
         rationale: Demonstrate updating the buildroot contents.
         buildroot: true
         srpm-buildroot: true
       xxx:
         rationale: xxx demonstrates arches and multilib.
-        buildafter:
-        - bar
+        buildorder: 10
         arches: [i686, x86_64]
         multilib: [x86_64]
     modules:
       includedmodule:
         rationale: Included in the stack, just because.
         repository: https://pagure.io/includedmodule.git
         ref: somecoolbranchname
+        buildorder: 100
 ...

modulemd/tests/test_data/upgrades/packager_v3_to_stream_v2.yaml

@@ -81,21 +81,19 @@ data:
         repository: https://pagure.io/bar.git
         cache: https://example.com/cache
         ref: 26ca0c0
-        buildafter:
-        - baz
       baz:
         rationale: Demonstrate updating the buildroot contents.
         buildroot: true
         srpm-buildroot: true
       xxx:
         rationale: xxx demonstrates arches and multilib.
-        buildafter:
-        - bar
+        buildorder: 10
         arches: [i686, x86_64]
         multilib: [x86_64]
     modules:
       includedmodule:
         rationale: Included in the stack, just because.
         repository: https://pagure.io/includedmodule.git
         ref: somecoolbranchname
+        buildorder: 100
 ...

yaml_specs/modulemd_packager_v2.yaml

@@ -331,13 +331,18 @@ data:
                 # must be be ordered later than some other entries in this map.
                 # The values of this array come from the keys of this map and
                 # not the real component name to enable bootstrapping.
+                # Use of both buildafter and buildorder in the same document is
+                # prohibited, as they will conflict.
                 #
                 # In this example, 'baz' makes some changes to the default
                 # buildroot that 'bar' must use.
                 #
+                # Note: The use of buildafter is not currently supported by the
+                # Fedora module build system.
+                #
                 # Type: AUTOMATIC
-                buildafter:
-                    - baz
+                # buildafter:
+                #     - baz
 
                 # buildonly:
                 # Use the "buildonly" value to indicate that all artifacts
@@ -416,8 +421,26 @@ data:
                 # present in order to build correctly, so we inform the build
                 # system not to build this until 'bar' has completed
                 # successfully.
-                buildafter:
-                    - bar
+                # buildafter:
+                #     - bar
+
+                # buildorder:
+                # Build order group
+                # When building, components are sorted by build order tag
+                # and built in batches grouped by their buildorder value.
+                # Built batches are then re-tagged into the buildroot.
+                # Multiple components can have the same buildorder index
+                # to map them into build groups.
+                # Defaults to zero.
+                # Integer, negative values are allowed.
+                # In this example, bar and baz are built first in no particular
+                # order, then tagged into the buildroot, then, finally, xxx is
+                # built.
+                # Use of both buildafter and buildorder in the same document is
+                # prohibited, as they will conflict.
+                #
+                # Type: OPTIONAL
+                buildorder: 10
 
         # modules:
         # Module content of this module
@@ -451,3 +474,9 @@ data:
                 #
                 # Type: AUTOMATIC
                 ref: somecoolbranchname
+
+                # buildorder:
+                # See the rpms buildorder.
+                #
+                # Type: AUTOMATIC
+                buildorder: 100