From f2978a99c7994566b1f5efedf1d77ae253ae1642 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Tue, 15 Oct 2019 11:08:27 -0400 Subject: [PATCH] Improve the merge logic to handle third-party repos more sanely Signed-off-by: Stephen Gallagher --- ...ModuleIndex.update_from_defaults_dir.patch | 395 ------------------ libmodulemd.spec | 11 +- 2 files changed, 5 insertions(+), 401 deletions(-) delete mode 100644 0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch diff --git a/0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch b/0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch deleted file mode 100644 index 61ce19f..0000000 --- a/0001-Improvements-to-ModuleIndex.update_from_defaults_dir.patch +++ /dev/null @@ -1,395 +0,0 @@ -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 c3f4c75..20f09a3 100644 --- a/libmodulemd.spec +++ b/libmodulemd.spec @@ -8,8 +8,8 @@ %endif Name: libmodulemd -Version: 2.8.0 -Release: 2%{?dist} +Version: 2.8.1 +Release: 1%{?dist} Summary: Module metadata manipulation library License: MIT @@ -39,10 +39,6 @@ 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. @@ -147,6 +143,9 @@ export MMD_SKIP_VALGRIND=1 %changelog +* Tue Oct 15 2019 Stephen Gallagher - 2.8.1-1 +- Improve the merge logic to handle third-party repos more sanely + * 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