diff --git a/0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch b/0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch new file mode 100644 index 0000000..61ce19f --- /dev/null +++ b/0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch @@ -0,0 +1,395 @@ +From c0bee1d95d628f12fa0e1e61eaad28b24d4416bc Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Wed, 18 Sep 2019 10:26:25 -0400 +Subject: [PATCH] Improvements to ModuleIndex.update_from_defaults_dir() + +- Import each file in the directory as a merge rather than an an overwrite so we +can detect conflicts. +- Modify the meaning of the 'strict' argument to mean that it should fail if the +merge would result in a conflict in the default stream setting for a module. +- Reduce the opportunities for a failure to corrupt the index. + +Fixes: https://github.com/fedora-modularity/libmodulemd/issues/366 + +Signed-off-by: Stephen Gallagher +--- + .../modulemd-2.0/modulemd-module-index.h | 2 +- + .../private/modulemd-rpm-map-entry-private.h | 4 +- + modulemd/modulemd-module-index.c | 122 ++++++++++++------ + modulemd/tests/test-modulemd-moduleindex.c | 69 +++++++++- + .../tests/test_data/bad_defaults/meson-2.yaml | 7 + + .../tests/test_data/bad_defaults/meson.yaml | 7 + + .../tests/test_data/bad_defaults/ninja.yaml | 7 + + .../tests/test_data/bad_defaults/nodejs.yaml | 9 ++ + 8 files changed, 183 insertions(+), 44 deletions(-) + create mode 100644 modulemd/tests/test_data/bad_defaults/meson-2.yaml + create mode 100644 modulemd/tests/test_data/bad_defaults/meson.yaml + create mode 100644 modulemd/tests/test_data/bad_defaults/ninja.yaml + create mode 100644 modulemd/tests/test_data/bad_defaults/nodejs.yaml + +diff --git a/modulemd/include/modulemd-2.0/modulemd-module-index.h b/modulemd/include/modulemd-2.0/modulemd-module-index.h +index 483a82f1e0292232181a39a78089b6d715c57805..d899f8223aecdac13b3fab0ad849350bffa09c65 100644 +--- a/modulemd/include/modulemd-2.0/modulemd-module-index.h ++++ b/modulemd/include/modulemd-2.0/modulemd-module-index.h +@@ -251,7 +251,7 @@ modulemd_module_index_update_from_custom (ModulemdModuleIndex *self, + * @self: This #ModulemdModuleIndex object. + * @path: (in): The path to a directory containing defaults documents. + * @strict: (in): Whether the parser should return failure if it encounters an +- * unknown mapping key or if it should ignore it. ++ * unknown mapping key or a conflict in module default streams. + * @overrides_path: (in) (nullable): If non-NULL, the path to a directory + * containing defaults documents that should override those in @path. + * @error: (out): A #GError indicating why this function failed. +diff --git a/modulemd/include/private/modulemd-rpm-map-entry-private.h b/modulemd/include/private/modulemd-rpm-map-entry-private.h +index a9cff19929037e3eaa0f14a999587af1e3b8f7bc..3f9945821dea63ba722736036e9f4daf803401be 100644 +--- a/modulemd/include/private/modulemd-rpm-map-entry-private.h ++++ b/modulemd/include/private/modulemd-rpm-map-entry-private.h +@@ -37,8 +37,8 @@ + */ + ModulemdRpmMapEntry * + modulemd_rpm_map_entry_parse_yaml (yaml_parser_t *parser, +- gboolean strict, +- GError **error); ++ gboolean strict, ++ GError **error); + + + /** +diff --git a/modulemd/modulemd-module-index.c b/modulemd/modulemd-module-index.c +index af0c8e96291d9b3e1534cfeac80dbc8abe262330..ab81bf421b38554e813dec6e3767ac9c8a2baf35 100644 +--- a/modulemd/modulemd-module-index.c ++++ b/modulemd/modulemd-module-index.c +@@ -671,76 +671,124 @@ modulemd_module_index_update_from_custom (ModulemdModuleIndex *self, + } + + +-gboolean +-modulemd_module_index_update_from_defaults_directory ( +- ModulemdModuleIndex *self, +- const gchar *path, +- gboolean strict, +- const gchar *overrides_path, +- GError **error) ++/* ++ * modules_from_directory: ++ * @path: A directory containing one or more modulemd YAML documents ++ * @file_suffix: A file suffix to limit the files to be read. Pass "" if you ++ * need to read all files. ++ * @strict: Whether to fail on unknown fields ++ * @strict_default_streams: Whether to fail on default stream merges. ++ * @error: Error return value ++ */ ++static ModulemdModuleIndex * ++modules_from_directory (const gchar *path, ++ const gchar *file_suffix, ++ gboolean strict, ++ gboolean strict_default_streams, ++ GError **error) + { + const gchar *filename = NULL; ++ g_autoptr (GDir) dir = NULL; + g_autofree gchar *filepath = NULL; +- g_autoptr (GDir) defdir = NULL; +- g_autoptr (GDir) overridedir = NULL; ++ g_autoptr (ModulemdModuleIndex) index = NULL; ++ g_autoptr (ModulemdModuleIndex) intermediate = NULL; + g_autoptr (GPtrArray) failures = NULL; +- g_autoptr (ModulemdModuleIndex) override_idx = NULL; ++ g_autoptr (GError) nested_error = NULL; + +- /* Read the regular path first */ +- defdir = g_dir_open (path, 0, error); +- if (!defdir) ++ index = modulemd_module_index_new (); ++ ++ /* Open the directory */ ++ dir = g_dir_open (path, 0, &nested_error); ++ if (!dir) + { ++ g_propagate_error (error, g_steal_pointer (&nested_error)); + return FALSE; + } + +- while ((filename = g_dir_read_name (defdir)) != NULL) ++ while ((filename = g_dir_read_name (dir)) != NULL) + { +- if (g_str_has_suffix (filename, MMD_YAML_SUFFIX)) ++ if (g_str_has_suffix (filename, file_suffix)) + { ++ intermediate = modulemd_module_index_new (); ++ + filepath = g_build_path ("/", path, filename, NULL); +- g_debug ("Reading defaults from %s", filepath); ++ g_debug ("Reading modulemd from %s", filepath); + if (!modulemd_module_index_update_from_file ( +- self, filepath, strict, &failures, error)) ++ intermediate, filepath, strict, &failures, error)) + { + return FALSE; + } ++ ++ if (!modulemd_module_index_merge (intermediate, ++ index, ++ FALSE, ++ strict_default_streams, ++ &nested_error)) ++ { ++ g_propagate_error (error, g_steal_pointer (&nested_error)); ++ return FALSE; ++ } ++ + g_clear_pointer (&failures, g_ptr_array_unref); + g_clear_pointer (&filepath, g_free); ++ g_clear_object (&intermediate); + } + } + ++ return g_steal_pointer (&index); ++} ++ ++ ++gboolean ++modulemd_module_index_update_from_defaults_directory ( ++ ModulemdModuleIndex *self, ++ const gchar *path, ++ gboolean strict, ++ const gchar *overrides_path, ++ GError **error) ++{ ++ g_autoptr (ModulemdModuleIndex) defaults_idx = NULL; ++ g_autoptr (ModulemdModuleIndex) override_idx = NULL; ++ g_autoptr (GError) nested_error = NULL; ++ ++ /* Read the regular path first */ ++ defaults_idx = modules_from_directory ( ++ path, MMD_YAML_SUFFIX, strict, strict, &nested_error); ++ if (!defaults_idx) ++ { ++ g_propagate_error (error, g_steal_pointer (&nested_error)); ++ return FALSE; ++ } ++ ++ /* If an override path was provided, use that too */ + if (overrides_path) + { +- overridedir = g_dir_open (overrides_path, 0, error); +- if (!overridedir) ++ override_idx = modules_from_directory ( ++ overrides_path, MMD_YAML_SUFFIX, strict, strict, &nested_error); ++ if (!override_idx) + { ++ g_propagate_error (error, g_steal_pointer (&nested_error)); + return FALSE; + } + +- override_idx = modulemd_module_index_new (); +- while ((filename = g_dir_read_name (overridedir)) != NULL) +- { +- if (g_str_has_suffix (filename, MMD_YAML_SUFFIX)) +- { +- filepath = g_build_path ("/", overrides_path, filename, NULL); +- g_debug ("Reading default overrides from %s", filepath); +- if (!modulemd_module_index_update_from_file ( +- override_idx, filepath, strict, &failures, error)) +- { +- return FALSE; +- } +- g_clear_pointer (&failures, g_ptr_array_unref); +- g_clear_pointer (&filepath, g_free); +- } +- } +- + if (!modulemd_module_index_merge ( +- override_idx, self, TRUE, FALSE, error)) ++ override_idx, defaults_idx, TRUE, strict, &nested_error)) + { ++ g_propagate_error (error, g_steal_pointer (&nested_error)); + return FALSE; + } + } + ++ /* Now that we've verified that the content in the two paths is compatible, ++ * attempt to merge it into the existing index. ++ */ ++ if (!modulemd_module_index_merge ( ++ defaults_idx, self, TRUE, strict, &nested_error)) ++ { ++ g_propagate_error (error, g_steal_pointer (&nested_error)); ++ return FALSE; ++ } ++ + + return TRUE; + } +diff --git a/modulemd/tests/test-modulemd-moduleindex.c b/modulemd/tests/test-modulemd-moduleindex.c +index 03db9de9c707d0c6c96d7f21ed5aabddf315fbba..2fc1237ff0006a329abd2f0f311b7743d8dbba31 100644 +--- a/modulemd/tests/test-modulemd-moduleindex.c ++++ b/modulemd/tests/test-modulemd-moduleindex.c +@@ -1193,6 +1193,8 @@ test_module_index_read_def_dir (void) + g_autoptr (GError) error = NULL; + g_autofree gchar *path = + g_build_path ("/", g_getenv ("TEST_DATA_PATH"), "defaults", NULL); ++ g_autofree gchar *bad_path = ++ g_build_path ("/", g_getenv ("TEST_DATA_PATH"), "bad_defaults", NULL); + g_autofree gchar *overrides_path = + g_build_path ("/", path, "overrides", NULL); + g_auto (GStrv) module_names = NULL; +@@ -1213,8 +1215,6 @@ test_module_index_read_def_dir (void) + module_names = modulemd_module_index_get_module_names_as_strv (idx); + g_assert_nonnull (module_names); + +- g_assert_cmpint (g_strv_length (module_names), ==, 3); +- + g_assert_true ( + g_strv_contains ((const gchar *const *)module_names, "meson")); + g_assert_true ( +@@ -1222,6 +1222,8 @@ test_module_index_read_def_dir (void) + g_assert_true ( + g_strv_contains ((const gchar *const *)module_names, "nodejs")); + ++ g_assert_cmpint (g_strv_length (module_names), ==, 3); ++ + defaultdict = + modulemd_module_index_get_default_streams_as_hash_table (idx, NULL); + g_assert_nonnull (defaultdict); +@@ -1248,8 +1250,6 @@ test_module_index_read_def_dir (void) + module_names = modulemd_module_index_get_module_names_as_strv (idx); + g_assert_nonnull (module_names); + +- g_assert_cmpint (g_strv_length (module_names), ==, 4); +- + g_assert_true ( + g_strv_contains ((const gchar *const *)module_names, "meson")); + g_assert_true ( +@@ -1259,6 +1259,8 @@ test_module_index_read_def_dir (void) + g_assert_true ( + g_strv_contains ((const gchar *const *)module_names, "testmodule")); + ++ g_assert_cmpint (g_strv_length (module_names), ==, 4); ++ + defaultdict = + modulemd_module_index_get_default_streams_as_hash_table (idx, NULL); + g_assert_nonnull (defaultdict); +@@ -1284,6 +1286,65 @@ test_module_index_read_def_dir (void) + idx, path, TRUE, "nonexistent", &error)); + g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_NOENT); + g_clear_error (&error); ++ ++ ++ g_clear_object (&idx); ++ idx = modulemd_module_index_new (); ++ g_assert_nonnull (idx); ++ ++ /* Base directory contains two defaults with conflicting streams for the ++ * same module in separate files. Strict mode. ++ */ ++ g_assert_false (modulemd_module_index_update_from_defaults_directory ( ++ idx, bad_path, TRUE, NULL, &error)); ++ g_assert_error (error, MODULEMD_ERROR, MODULEMD_ERROR_VALIDATE); ++ g_clear_error (&error); ++ ++ /* Verify that the index has not been modified as a side-effect */ ++ module_names = modulemd_module_index_get_module_names_as_strv (idx); ++ g_assert_cmpint (g_strv_length (module_names), ==, 0); ++ ++ g_clear_pointer (&module_names, g_strfreev); ++ g_clear_object (&idx); ++ idx = modulemd_module_index_new (); ++ g_assert_nonnull (idx); ++ ++ /* Base directory contains two defaults with conflicting streams for the ++ * same module in separate files. Non-strict mode. ++ */ ++ g_assert_true (modulemd_module_index_update_from_defaults_directory ( ++ idx, bad_path, FALSE, NULL, &error)); ++ g_assert_no_error (error); ++ ++ ++ /* There should be three modules in the index now: ++ * - meson ++ * - ninja ++ * - nodejs ++ */ ++ module_names = modulemd_module_index_get_module_names_as_strv (idx); ++ g_assert_nonnull (module_names); ++ ++ g_assert_true ( ++ g_strv_contains ((const gchar *const *)module_names, "meson")); ++ g_assert_true ( ++ g_strv_contains ((const gchar *const *)module_names, "ninja")); ++ g_assert_true ( ++ g_strv_contains ((const gchar *const *)module_names, "nodejs")); ++ ++ g_assert_cmpint (g_strv_length (module_names), ==, 3); ++ ++ defaultdict = ++ modulemd_module_index_get_default_streams_as_hash_table (idx, NULL); ++ g_assert_nonnull (defaultdict); ++ ++ /* Make sure that in non-strict mode, meson's default is reset to NULL */ ++ g_assert_cmpstr (g_hash_table_lookup (defaultdict, "meson"), ==, NULL); ++ g_assert_cmpstr (g_hash_table_lookup (defaultdict, "ninja"), ==, "latest"); ++ g_assert_cmpstr (g_hash_table_lookup (defaultdict, "nodejs"), ==, NULL); ++ ++ g_clear_pointer (&module_names, g_strfreev); ++ g_clear_pointer (&defaultdict, g_hash_table_unref); + } + + +diff --git a/modulemd/tests/test_data/bad_defaults/meson-2.yaml b/modulemd/tests/test_data/bad_defaults/meson-2.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..9ebec5ed3de80307bd19c46e1a776a9e782ce56d +--- /dev/null ++++ b/modulemd/tests/test_data/bad_defaults/meson-2.yaml +@@ -0,0 +1,7 @@ ++document: modulemd-defaults ++version: 1 ++data: ++ module: meson ++ stream: old ++ profiles: ++ latest: [default] +diff --git a/modulemd/tests/test_data/bad_defaults/meson.yaml b/modulemd/tests/test_data/bad_defaults/meson.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..65c892223f41faf103db47698136b44e6543631d +--- /dev/null ++++ b/modulemd/tests/test_data/bad_defaults/meson.yaml +@@ -0,0 +1,7 @@ ++document: modulemd-defaults ++version: 1 ++data: ++ module: meson ++ stream: latest ++ profiles: ++ latest: [default] +diff --git a/modulemd/tests/test_data/bad_defaults/ninja.yaml b/modulemd/tests/test_data/bad_defaults/ninja.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..a7ecb6e58c0125bcd957c4bb9c9fd4a375d80bb6 +--- /dev/null ++++ b/modulemd/tests/test_data/bad_defaults/ninja.yaml +@@ -0,0 +1,7 @@ ++document: modulemd-defaults ++version: 1 ++data: ++ module: ninja ++ stream: latest ++ profiles: ++ latest: [default] +diff --git a/modulemd/tests/test_data/bad_defaults/nodejs.yaml b/modulemd/tests/test_data/bad_defaults/nodejs.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..9d2b38d52ce0136a2df34f145a54f33b15015767 +--- /dev/null ++++ b/modulemd/tests/test_data/bad_defaults/nodejs.yaml +@@ -0,0 +1,9 @@ ++document: modulemd-defaults ++version: 1 ++data: ++ modified: 201906261200 ++ module: nodejs ++ profiles: ++ 8: [default] ++ 10: [default] ++ 12: [default] +-- +2.23.0 + diff --git a/libmodulemd.spec b/libmodulemd.spec index 5b5af88..c3f4c75 100644 --- a/libmodulemd.spec +++ b/libmodulemd.spec @@ -9,7 +9,7 @@ Name: libmodulemd Version: 2.8.0 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Module metadata manipulation library License: MIT @@ -39,6 +39,10 @@ BuildRequires: valgrind # Patches +# Upstream patch to improve ModuleIndex.update_from_defaults_dir() +# https://github.com/fedora-modularity/libmodulemd/issues/366 +Patch0001: 0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch + %description C Library for manipulating module metadata files. @@ -143,6 +147,13 @@ export MMD_SKIP_VALGRIND=1 %changelog +* Wed Sep 18 2019 Stephen Gallagher - 2.8.0-2 +- Improvements to ModuleIndex.update_from_defaults_directory() + * Import each file in the directory as a merge rather than an overwrite so + we can detect conflicts. + * Modify the meaning of the 'strict' argument to fail if the merge would + result in a conflict in the default stream setting of a module. + * Wed Sep 04 2019 Stephen Gallagher - 2.8.0-1 - Update to 2.8.0 - API Changes