diff --git a/SOURCES/0001-Double-valgrind-timeout.patch b/SOURCES/0001-Double-valgrind-timeout.patch index e7f128a..e10cd7e 100644 --- a/SOURCES/0001-Double-valgrind-timeout.patch +++ b/SOURCES/0001-Double-valgrind-timeout.patch @@ -1,7 +1,7 @@ From 1a7bf143761ff8e3f4f6585b82c0be4dbd511fca Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Thu, 23 May 2019 14:00:36 -0400 -Subject: [PATCH 1/3] Double valgrind timeout +Subject: [PATCH 1/8] Double valgrind timeout On some architectures under heavy load, the valgrind check on v2 is taking a long time. @@ -15,9 +15,7 @@ diff --git a/modulemd/meson.build b/modulemd/meson.build index 47bd1f58e6401d2634d8ff737c2b347f5ebc6bf5..e49c7a9df76dcf37a18ddeba3150d6c914aa4e25 100644 --- a/modulemd/meson.build +++ b/modulemd/meson.build -@@ -313,7 +313,7 @@ endif - if valgrind.found() - modulemd_valgrind_scripts = files('common/tests/test-valgrind.py') +@@ -315,5 +315,5 @@ if valgrind.found() test ('valgrind', python3, env : test_env, args : modulemd_valgrind_scripts, @@ -25,5 +23,5 @@ index 47bd1f58e6401d2634d8ff737c2b347f5ebc6bf5..e49c7a9df76dcf37a18ddeba3150d6c9 + timeout : 1200) endif -- -2.21.0 +2.23.0 diff --git a/SOURCES/0002-Parallelize-the-valgrind-tests.patch b/SOURCES/0002-Parallelize-the-valgrind-tests.patch index ba160a5..4d7a175 100644 --- a/SOURCES/0002-Parallelize-the-valgrind-tests.patch +++ b/SOURCES/0002-Parallelize-the-valgrind-tests.patch @@ -1,7 +1,7 @@ From d9b41f72d4b2d545b2600aff7bd8a27ed0093750 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Wed, 29 May 2019 11:33:57 -0400 -Subject: [PATCH 2/3] Parallelize the valgrind tests +Subject: [PATCH 2/8] Parallelize the valgrind tests This considerably reduces the time needed to perform the valgrind memory tests on systems with multiple available processors. In the @@ -16,9 +16,7 @@ diff --git a/modulemd/common/tests/test-valgrind.py b/modulemd/common/tests/test index 9be72c705fde79aa305d831eaa4f31f7e2cc663f..9349749658ccca0529776f3d534664d614c1cb4d 100644 --- a/modulemd/common/tests/test-valgrind.py +++ b/modulemd/common/tests/test-valgrind.py -@@ -16,13 +16,16 @@ import os - import sys - import subprocess +@@ -18,9 +18,12 @@ import subprocess import tempfile import xml.etree.ElementTree as ET @@ -31,11 +29,7 @@ index 9be72c705fde79aa305d831eaa4f31f7e2cc663f..9349749658ccca0529776f3d534664d6 failed = False # Get the list of tests to run - proc_result = subprocess.run(['meson', 'test', '--list'], - stdout=subprocess.PIPE, -@@ -47,13 +50,13 @@ for test in unfiltered_tests: - test == 'test_dirty_repo' or - test == 'valgrind'): +@@ -49,9 +52,9 @@ for test in unfiltered_tests: continue tests.append(test) @@ -47,11 +41,7 @@ index 9be72c705fde79aa305d831eaa4f31f7e2cc663f..9349749658ccca0529776f3d534664d6 valgrind_command = "/usr/bin/valgrind " \ "--leak-check=full " \ "--suppressions=/usr/share/glib-2.0/valgrind/glib.supp " \ - "--xml=yes " \ - "--xml-file=%s/%s.xml " % (tmpdirname, test) -@@ -64,45 +67,50 @@ with tempfile.TemporaryDirectory(prefix="libmodulemd_valgrind_") as tmpdirname: - '-t', '10', - '--logbase=%s' % test, +@@ -66,42 +69,47 @@ with tempfile.TemporaryDirectory(prefix="libmodulemd_valgrind_") as tmpdirname: '--wrap=%s' % valgrind_command, test]) @@ -130,7 +120,6 @@ index 9be72c705fde79aa305d831eaa4f31f7e2cc663f..9349749658ccca0529776f3d534664d6 if failed: - sys.exit(1) -- -2.21.0 +2.23.0 diff --git a/SOURCES/0003-Fix-transfer-type-for-Module.search_streams.patch b/SOURCES/0003-Fix-transfer-type-for-Module.search_streams.patch index 40ba286..b9aaefa 100644 --- a/SOURCES/0003-Fix-transfer-type-for-Module.search_streams.patch +++ b/SOURCES/0003-Fix-transfer-type-for-Module.search_streams.patch @@ -1,7 +1,7 @@ From 340e316bd6384562086b4e381c8cd42b1ccd0781 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Tue, 28 May 2019 14:28:30 -0400 -Subject: [PATCH 3/3] Fix transfer type for Module.search_streams() +Subject: [PATCH 3/8] Fix transfer type for Module.search_streams() Technically this is an API-breaking change, but no one is using it yet and it was always expected to be managed this way. @@ -21,9 +21,7 @@ diff --git a/modulemd/meson.build b/modulemd/meson.build index e49c7a9df76dcf37a18ddeba3150d6c914aa4e25..e5912d4041ba3e427d13b98c0eeca5217d48244b 100644 --- a/modulemd/meson.build +++ b/modulemd/meson.build -@@ -229,10 +229,11 @@ test_v2_python_scripts = files( - 'v2/tests/ModulemdTests/componentrpm.py', - 'v2/tests/ModulemdTests/defaults.py', +@@ -231,6 +231,7 @@ test_v2_python_scripts = files( 'v2/tests/ModulemdTests/defaultsv1.py', 'v2/tests/ModulemdTests/dependencies.py', 'v2/tests/ModulemdTests/merger.py', @@ -31,15 +29,11 @@ index e49c7a9df76dcf37a18ddeba3150d6c914aa4e25..e5912d4041ba3e427d13b98c0eeca521 'v2/tests/ModulemdTests/moduleindex.py', 'v2/tests/ModulemdTests/modulestream.py', 'v2/tests/ModulemdTests/profile.py', - 'v2/tests/ModulemdTests/rpmmap.py', - 'v2/tests/ModulemdTests/servicelevel.py', diff --git a/modulemd/v2/include/modulemd-2.0/modulemd-module.h b/modulemd/v2/include/modulemd-2.0/modulemd-module.h index 45be9c0ae96e203707da61d84f18c71c4a826035..0219cfe227652813d20bdf736f9033782084c5ad 100644 --- a/modulemd/v2/include/modulemd-2.0/modulemd-module.h +++ b/modulemd/v2/include/modulemd-2.0/modulemd-module.h -@@ -129,12 +129,12 @@ modulemd_module_get_stream_by_NSVC (ModulemdModule *self, - * @context: (nullable): The context of the stream to retrieve. If NULL, the - * context is not included in the search. +@@ -131,8 +131,8 @@ modulemd_module_get_stream_by_NSVC (ModulemdModule *self, * @arch: (nullable): The processor architecture of the stream to retrieve. If * NULL, the architecture is not included in the search. * @@ -50,15 +44,11 @@ index 45be9c0ae96e203707da61d84f18c71c4a826035..0219cfe227652813d20bdf736f903378 * fail, but it may return a zero-length list if no matches were found. The * returned streams will be in a predictable order, sorted first by stream * name, then by version (highest to lowest), then by context and finally by - * architecture. - * diff --git a/modulemd/v2/meson.build b/modulemd/v2/meson.build index 3f45db2c4e0e9a8996c74dffd949d5276082dc6f..e8a5a38f0528c4f860f0b84ef63609ff5fd89caa 100644 --- a/modulemd/v2/meson.build +++ b/modulemd/v2/meson.build -@@ -390,20 +390,35 @@ test ('dependencies_python2_debug', python2, - args : dependencies_python_script) - test ('dependencies_python2_release', python2, +@@ -392,6 +392,22 @@ test ('dependencies_python2_release', python2, env : py_test_release_env, args : dependencies_python_script) @@ -81,9 +71,7 @@ index 3f45db2c4e0e9a8996c74dffd949d5276082dc6f..e8a5a38f0528c4f860f0b84ef63609ff # -- Test Modulemd.ModuleIndex (Python) -- # moduleindex_python_script = files('tests/ModulemdTests/moduleindex.py') test ('moduleindex_python3_debug', python3, - env : py_test_env, - args : moduleindex_python_script) - test ('moduleindex_python3_release', python3, +@@ -401,7 +417,6 @@ test ('moduleindex_python3_release', python3, env : py_test_release_env, args : moduleindex_python_script) @@ -91,8 +79,6 @@ index 3f45db2c4e0e9a8996c74dffd949d5276082dc6f..e8a5a38f0528c4f860f0b84ef63609ff test ('moduleindex_python2_debug', python2, env : py_test_env, args : moduleindex_python_script) - test ('moduleindex_python2_release', python2, - env : py_test_release_env, diff --git a/modulemd/v2/tests/ModulemdTests/module.py b/modulemd/v2/tests/ModulemdTests/module.py new file mode 100644 index 0000000000000000000000000000000000000000..b604d9c9b357c4a5211d3ba5b4d0aba08c3a42bd @@ -146,5 +132,5 @@ index 0000000000000000000000000000000000000000..b604d9c9b357c4a5211d3ba5b4d0aba0 +if __name__ == '__main__': + unittest.main() -- -2.21.0 +2.23.0 diff --git a/SOURCES/0004-Extend-timeout-for-header-test.patch b/SOURCES/0004-Extend-timeout-for-header-test.patch new file mode 100644 index 0000000..fbda719 --- /dev/null +++ b/SOURCES/0004-Extend-timeout-for-header-test.patch @@ -0,0 +1,27 @@ +From 82e56d78e46504aab5917b606eae67e1a3b54603 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Wed, 29 May 2019 16:18:30 -0400 +Subject: [PATCH 4/8] Extend timeout for header test + +Signed-off-by: Stephen Gallagher +--- + modulemd/meson.build | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/modulemd/meson.build b/modulemd/meson.build +index e5912d4041ba3e427d13b98c0eeca5217d48244b..0741736a12b4457559ade0a5372ff4b25743fce3 100644 +--- a/modulemd/meson.build ++++ b/modulemd/meson.build +@@ -308,7 +308,8 @@ if build_api_v2 + import_header_script = find_program('common/tests/test-import-headers.sh') + test ('test_v2_import_headers', import_header_script, + env : test_env, +- args : modulemd_v2_hdrs) ++ args : modulemd_v2_hdrs, ++ timeout : 300) + endif + + if valgrind.found() +-- +2.23.0 + diff --git a/SOURCES/0005-Add-test_data_path-env-var-for-Python-tests.patch b/SOURCES/0005-Add-test_data_path-env-var-for-Python-tests.patch new file mode 100644 index 0000000..013dd78 --- /dev/null +++ b/SOURCES/0005-Add-test_data_path-env-var-for-Python-tests.patch @@ -0,0 +1,41 @@ +From b338dee563932c44bed50b54a1ce00c8e83c0465 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Wed, 23 Oct 2019 11:13:55 -0400 +Subject: [PATCH 5/8] Add test_data_path env var for Python tests + +Signed-off-by: Stephen Gallagher +--- + modulemd/v2/meson.build | 1 + + modulemd/v2/tests/ModulemdTests/base.py | 4 ++++ + 2 files changed, 5 insertions(+) + +diff --git a/modulemd/v2/meson.build b/modulemd/v2/meson.build +index e8a5a38f0528c4f860f0b84ef63609ff5fd89caa..5137b7d23ec66e39a56cf022974da6570c8e4822 100644 +--- a/modulemd/v2/meson.build ++++ b/modulemd/v2/meson.build +@@ -93,6 +93,7 @@ install_headers( + test_release_env = environment() + test_release_env.set('LC_ALL', 'C') + test_release_env.set ('MESON_SOURCE_ROOT', meson.source_root()) ++test_release_env.set ('TEST_DATA_PATH', meson.source_root() + '/modulemd/tests/test_data') + + # Test env with fatal warnings and criticals + test_env = test_release_env +diff --git a/modulemd/v2/tests/ModulemdTests/base.py b/modulemd/v2/tests/ModulemdTests/base.py +index 831309634100f25533c9a7dafe7f96bf7e100cd7..5f396958fd7a20e6567cfe69f99eaeb95825663a 100644 +--- a/modulemd/v2/tests/ModulemdTests/base.py ++++ b/modulemd/v2/tests/ModulemdTests/base.py +@@ -28,6 +28,10 @@ class TestBase(unittest.TestCase): + def source_root(self): + return os.getenv("MESON_SOURCE_ROOT") + ++ @property ++ def test_data_path(self): ++ return os.getenv("TEST_DATA_PATH") ++ + def _catch_signal(self, *sigargs): + if self._caught_signal: + raise AssertionError("Multiple signals were caught") +-- +2.23.0 + diff --git a/SOURCES/0006-Add-ModuleIndexMerger.resolve_ext.patch b/SOURCES/0006-Add-ModuleIndexMerger.resolve_ext.patch new file mode 100644 index 0000000..cf3ca31 --- /dev/null +++ b/SOURCES/0006-Add-ModuleIndexMerger.resolve_ext.patch @@ -0,0 +1,272 @@ +From 200838c96b48cbe849af652cbb7ece98205cbf9d Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Wed, 3 Jul 2019 08:05:25 -0400 +Subject: [PATCH 6/8] Add ModuleIndexMerger.resolve_ext() + +This will allow us to handle default streams strictly when merging. In most end-user +cases, we want default stream conflicts to simply drop the default stream (this is +fail-safe). But when we are creating a repo with defaults, we want to be stricter and +ensure that we aren't producing a repo with inconsistent data. + +Signed-off-by: Stephen Gallagher +--- + .../modulemd-module-index-merger.h | 33 +++++++++++++++++++ + .../private/modulemd-defaults-private.h | 3 ++ + .../private/modulemd-defaults-v1-private.h | 1 + + .../private/modulemd-module-index-private.h | 4 +++ + modulemd/v2/modulemd-defaults-v1.c | 12 +++++++ + modulemd/v2/modulemd-defaults.c | 2 ++ + modulemd/v2/modulemd-module-index-merger.c | 12 ++++++- + modulemd/v2/modulemd-module-index.c | 5 +-- + modulemd/v2/tests/ModulemdTests/merger.py | 21 ++++++++++++ + 9 files changed, 90 insertions(+), 3 deletions(-) + +diff --git a/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h b/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h +index b019f0ed856684a003e9c2f6abefc70e1448246a..97ca7ebb3824ef62aabbafee391b25d56bac43c3 100644 +--- a/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h ++++ b/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h +@@ -161,6 +161,10 @@ modulemd_module_index_merger_associate_index (ModulemdModuleIndexMerger *self, + * #ModulemdModuleIndexMerger is undefined. The only valid action on it after + * that point is g_object_unref(). + * ++ * This function is equivalent to calling ++ * modulemd_module_index_merger_resolve_ext() with ++ * `strict_default_streams=FALSE`. ++ * + * Returns: (transfer full): A newly-allocated #ModulemdModuleIndex containing + * the merged results. If this function encounters an unresolvable merge + * conflict, it will return NULL and set @error appropriately. +@@ -171,4 +175,33 @@ ModulemdModuleIndex * + modulemd_module_index_merger_resolve (ModulemdModuleIndexMerger *self, + GError **error); + ++ ++/** ++ * modulemd_module_index_merger_resolve_ext: ++ * @self: (in): This #ModulemdModuleIndexMerger object. ++ * @strict_default_streams: (in): If TRUE, merging two #ModulemdDefaults with ++ * conflicting default streams will raise an error. If FALSE, the module will ++ * have its default stream blocked. ++ * @error: (out): A #GError containing the reason for a failure to resolve the ++ * merges. ++ * ++ * Merges all added #ModulemdModuleIndex objects according to their priority. ++ * The logic of this merge is described in the Description of ++ * #ModulemdModuleIndexMerger. ++ * ++ * Once this function has been called, the internal state of the ++ * #ModulemdModuleIndexMerger is undefined. The only valid action on it after ++ * that point is g_object_unref(). ++ * ++ * Returns: (transfer full): A newly-allocated #ModulemdModuleIndex containing ++ * the merged results. If this function encounters an unresolvable merge ++ * conflict, it will return NULL and set @error appropriately. ++ * ++ * Since: 2.6 ++ */ ++ModulemdModuleIndex * ++modulemd_module_index_merger_resolve_ext (ModulemdModuleIndexMerger *self, ++ gboolean strict_default_streams, ++ GError **error); ++ + G_END_DECLS +diff --git a/modulemd/v2/include/private/modulemd-defaults-private.h b/modulemd/v2/include/private/modulemd-defaults-private.h +index 1863cf6365e769b184478ba5435c664ac05a0b2f..6ea3a54ff992890e88badfdaefa1483b9ba52429 100644 +--- a/modulemd/v2/include/private/modulemd-defaults-private.h ++++ b/modulemd/v2/include/private/modulemd-defaults-private.h +@@ -38,6 +38,8 @@ modulemd_defaults_set_module_name (ModulemdDefaults *self, + * modulemd_defaults_merge: + * @from: (in): A #ModulemdDefaults object to merge from + * @into: (in): A #ModulemdDefaults object being merged into ++ * @strict_default_streams: (in): Whether a stream conflict should throw an ++ * error or just unset the default stream. + * @error: (out): A #GError containing the reason for an unresolvable merge + * conflict + * +@@ -52,6 +54,7 @@ modulemd_defaults_set_module_name (ModulemdDefaults *self, + ModulemdDefaults * + modulemd_defaults_merge (ModulemdDefaults *from, + ModulemdDefaults *into, ++ gboolean strict_default_streams, + GError **error); + + G_END_DECLS +diff --git a/modulemd/v2/include/private/modulemd-defaults-v1-private.h b/modulemd/v2/include/private/modulemd-defaults-v1-private.h +index de2ede98e6fb49bfd792b5f2913868fad6ff27db..9144a7ef90853a28eee8345a3b814fdc6478be7b 100644 +--- a/modulemd/v2/include/private/modulemd-defaults-v1-private.h ++++ b/modulemd/v2/include/private/modulemd-defaults-v1-private.h +@@ -74,6 +74,7 @@ ModulemdDefaults * + modulemd_defaults_v1_merge (const gchar *module_name, + ModulemdDefaultsV1 *from, + ModulemdDefaultsV1 *into, ++ gboolean strict_default_streams, + GError **error); + + G_END_DECLS +diff --git a/modulemd/v2/include/private/modulemd-module-index-private.h b/modulemd/v2/include/private/modulemd-module-index-private.h +index fd37ab30dffa99f81b2762cb4e23546fe990b2f6..62fc71844f3a58030f79e7da628656b3ffcba5ae 100644 +--- a/modulemd/v2/include/private/modulemd-module-index-private.h ++++ b/modulemd/v2/include/private/modulemd-module-index-private.h +@@ -72,6 +72,9 @@ modulemd_module_index_update_from_parser (ModulemdModuleIndex *self, + * argument specifies whether the contents of @from will supersede those from + * @into. For specifics of how this works, see the Description section for + * #ModulemdIndexMerger. ++ * @strict_default_streams: (in): When merging #ModulemdDefaults, treat ++ * conflicting stream defaults as an error if this is True. Otherwise, on a ++ * conflict, the default stream will be unset. + * @error: (out): If the merge fails, this will return a #GError explaining the + * reason for it. + * +@@ -84,6 +87,7 @@ gboolean + modulemd_module_index_merge (ModulemdModuleIndex *from, + ModulemdModuleIndex *into, + gboolean override, ++ gboolean strict_default_streams, + GError **error); + + G_END_DECLS +diff --git a/modulemd/v2/modulemd-defaults-v1.c b/modulemd/v2/modulemd-defaults-v1.c +index 55f899b530ef59e0d60de982db76440c7a493325..f51e79d9b0a9fb0395077cd1c3be16529dd53f92 100644 +--- a/modulemd/v2/modulemd-defaults-v1.c ++++ b/modulemd/v2/modulemd-defaults-v1.c +@@ -1204,6 +1204,7 @@ ModulemdDefaults * + modulemd_defaults_v1_merge (const gchar *module_name, + ModulemdDefaultsV1 *from, + ModulemdDefaultsV1 *into, ++ gboolean strict_default_streams, + GError **error) + { + g_autoptr (ModulemdDefaultsV1) merged = NULL; +@@ -1246,6 +1247,17 @@ modulemd_defaults_v1_merge (const gchar *module_name, + g_info ("Module stream mismatch in merge: %s != %s", + into->default_stream, + from->default_stream); ++ if (strict_default_streams) ++ { ++ g_set_error (error, ++ MODULEMD_ERROR, ++ MODULEMD_ERROR_VALIDATE, ++ "Default stream mismatch in module %s: %s != %s", ++ module_name, ++ into->default_stream, ++ from->default_stream); ++ return NULL; ++ } + modulemd_defaults_v1_set_default_stream ( + merged, DEFAULT_MERGE_CONFLICT, NULL); + } +diff --git a/modulemd/v2/modulemd-defaults.c b/modulemd/v2/modulemd-defaults.c +index 9711fbdbd267570d01d8df4962a2c4e2a7d25c6e..2157f21fd4079ee096baee2a3a3552df5c273d06 100644 +--- a/modulemd/v2/modulemd-defaults.c ++++ b/modulemd/v2/modulemd-defaults.c +@@ -391,6 +391,7 @@ modulemd_defaults_init (ModulemdDefaults *self) + ModulemdDefaults * + modulemd_defaults_merge (ModulemdDefaults *from, + ModulemdDefaults *into, ++ gboolean strict_default_streams, + GError **error) + { + g_autoptr (ModulemdDefaults) merged_defaults = NULL; +@@ -446,6 +447,7 @@ modulemd_defaults_merge (ModulemdDefaults *from, + merged_defaults = modulemd_defaults_v1_merge (module_name, + MODULEMD_DEFAULTS_V1 (from), + MODULEMD_DEFAULTS_V1 (into), ++ strict_default_streams, + &nested_error); + if (!merged_defaults) + { +diff --git a/modulemd/v2/modulemd-module-index-merger.c b/modulemd/v2/modulemd-module-index-merger.c +index b8abda8eef91d88538d617b2e15fc77970110ba6..c3502f5c95f7b3227ed772db302d1cc67ca16c7c 100644 +--- a/modulemd/v2/modulemd-module-index-merger.c ++++ b/modulemd/v2/modulemd-module-index-merger.c +@@ -170,6 +170,14 @@ modulemd_module_index_merger_associate_index (ModulemdModuleIndexMerger *self, + ModulemdModuleIndex * + modulemd_module_index_merger_resolve (ModulemdModuleIndexMerger *self, + GError **error) ++{ ++ return modulemd_module_index_merger_resolve_ext (self, FALSE, error); ++} ++ ++ModulemdModuleIndex * ++modulemd_module_index_merger_resolve_ext (ModulemdModuleIndexMerger *self, ++ gboolean strict_default_streams, ++ GError **error) + { + MODULEMD_INIT_TRACE (); + g_autoptr (ModulemdModuleIndex) thislevel = NULL; +@@ -200,6 +208,7 @@ modulemd_module_index_merger_resolve (ModulemdModuleIndexMerger *self, + if (!modulemd_module_index_merge (g_ptr_array_index (indexes, j), + thislevel, + FALSE, ++ strict_default_streams, + &nested_error)) + { + g_propagate_error (error, g_steal_pointer (&nested_error)); +@@ -209,7 +218,8 @@ modulemd_module_index_merger_resolve (ModulemdModuleIndexMerger *self, + + + /* Merge 'thislevel' into 'final' with override=True */ +- if (!modulemd_module_index_merge (thislevel, final, TRUE, &nested_error)) ++ if (!modulemd_module_index_merge ( ++ thislevel, final, TRUE, strict_default_streams, &nested_error)) + { + g_propagate_error (error, g_steal_pointer (&nested_error)); + return NULL; +diff --git a/modulemd/v2/modulemd-module-index.c b/modulemd/v2/modulemd-module-index.c +index e67f91ec9ba7e0b629f871c85024fd76b6ef3c4d..61878d5b1cbf060b11729328c38027a7ba526b42 100644 +--- a/modulemd/v2/modulemd-module-index.c ++++ b/modulemd/v2/modulemd-module-index.c +@@ -908,6 +908,7 @@ gboolean + modulemd_module_index_merge (ModulemdModuleIndex *from, + ModulemdModuleIndex *into, + gboolean override, ++ gboolean strict_default_streams, + GError **error) + { + MODULEMD_INIT_TRACE (); +@@ -996,8 +997,8 @@ modulemd_module_index_merge (ModulemdModuleIndex *from, + } + else + { +- merged_defaults = +- modulemd_defaults_merge (defaults, into_defaults, &nested_error); ++ merged_defaults = modulemd_defaults_merge ( ++ defaults, into_defaults, strict_default_streams, &nested_error); + if (!merged_defaults) + { + g_propagate_error (error, g_steal_pointer (&nested_error)); +diff --git a/modulemd/v2/tests/ModulemdTests/merger.py b/modulemd/v2/tests/ModulemdTests/merger.py +index b87c05798af98d93dfcf67c8b0be629dd94efe4f..92c90d185f55b8a7d5f856aa16cdce5ff32f30d8 100644 +--- a/modulemd/v2/tests/ModulemdTests/merger.py ++++ b/modulemd/v2/tests/ModulemdTests/merger.py +@@ -210,6 +210,27 @@ class TestModuleIndexMerger(TestBase): + def test_merger_with_modified(self): + pass + ++ def test_strict_default_streams(self): ++ merger = Modulemd.ModuleIndexMerger.new() ++ ++ for stream in ("27", "38"): ++ default = """ ++--- ++document: modulemd-defaults ++version: 1 ++data: ++ module: python ++ stream: %s ++... ++""" % (stream) ++ ++ index = Modulemd.ModuleIndex() ++ index.update_from_string(default, strict=True) ++ merger.associate_index(index, 0) ++ ++ with self.assertRaisesRegexp(gi.repository.GLib.GError, "Default stream mismatch in module python"): ++ merger.resolve_ext(True) ++ + + if __name__ == '__main__': + unittest.main() +-- +2.23.0 + diff --git a/SOURCES/0007-Rework-defaults-merging-logic-2x.patch b/SOURCES/0007-Rework-defaults-merging-logic-2x.patch new file mode 100644 index 0000000..4292cd2 --- /dev/null +++ b/SOURCES/0007-Rework-defaults-merging-logic-2x.patch @@ -0,0 +1,949 @@ +From de49d4386cf3c842ce63d2005a624bdfc369c247 Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Fri, 27 Sep 2019 15:21:27 -0400 +Subject: [PATCH 7/8] Rework defaults merging logic + +Updates documentation of ModuleIndexMerger with the new algorithm. + +Also fixes a memory leak in DefaultsV1.set_default_stream() + +Fixes: https://github.com/fedora-modularity/libmodulemd/issues/368 + +Signed-off-by: Stephen Gallagher +--- + .../merger/add_conflicting_profile.yaml | 7 + + .../merger/add_conflicting_stream.yaml | 9 + + ...nflicting_stream_and_profile_modified.yaml | 11 + + modulemd/tests/test_data/merger/add_only.yaml | 8 + + modulemd/tests/test_data/merger/base.yaml | 34 +++ + .../modulemd-module-index-merger.h | 67 +++--- + .../private/modulemd-defaults-v1-private.h | 23 +- + modulemd/v2/modulemd-defaults-v1.c | 207 ++++++++++-------- + modulemd/v2/modulemd-defaults.c | 28 +-- + modulemd/v2/tests/ModulemdTests/merger.py | 119 ++++++++++ + modulemd/v2/tests/test-modulemd-merger.c | 147 +++++++++++++ + 11 files changed, 510 insertions(+), 150 deletions(-) + create mode 100644 modulemd/tests/test_data/merger/add_conflicting_profile.yaml + create mode 100644 modulemd/tests/test_data/merger/add_conflicting_stream.yaml + create mode 100644 modulemd/tests/test_data/merger/add_conflicting_stream_and_profile_modified.yaml + create mode 100644 modulemd/tests/test_data/merger/add_only.yaml + create mode 100644 modulemd/tests/test_data/merger/base.yaml + +diff --git a/modulemd/tests/test_data/merger/add_conflicting_profile.yaml b/modulemd/tests/test_data/merger/add_conflicting_profile.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..29b0dd0d41f61b8e83f6adde3334ee1a1f65893a +--- /dev/null ++++ b/modulemd/tests/test_data/merger/add_conflicting_profile.yaml +@@ -0,0 +1,7 @@ ++document: modulemd-defaults ++version: 1 ++data: ++ module: postgresql ++ stream: '8.1' ++ profiles: ++ '8.1': [client, server] +diff --git a/modulemd/tests/test_data/merger/add_conflicting_stream.yaml b/modulemd/tests/test_data/merger/add_conflicting_stream.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..615ca8746f8c880f220bdb01275c1ef37104185d +--- /dev/null ++++ b/modulemd/tests/test_data/merger/add_conflicting_stream.yaml +@@ -0,0 +1,9 @@ ++--- ++document: modulemd-defaults ++version: 1 ++data: ++ module: postgresql ++ stream: '8.2' ++ profiles: ++ '8.2': [client, server, foo] ++ +diff --git a/modulemd/tests/test_data/merger/add_conflicting_stream_and_profile_modified.yaml b/modulemd/tests/test_data/merger/add_conflicting_stream_and_profile_modified.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..db4bf38515502316d667a48d41a64a9f4a2937c3 +--- /dev/null ++++ b/modulemd/tests/test_data/merger/add_conflicting_stream_and_profile_modified.yaml +@@ -0,0 +1,11 @@ ++--- ++document: modulemd-defaults ++version: 1 ++data: ++ module: postgresql ++ stream: '8.2' ++ modified: 201909270000 ++ profiles: ++ '8.1': [client, server] ++ '8.2': [client, server, foo] ++ +diff --git a/modulemd/tests/test_data/merger/add_only.yaml b/modulemd/tests/test_data/merger/add_only.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..4a717807f928435ada8f3cf6fcdbd4c86927062e +--- /dev/null ++++ b/modulemd/tests/test_data/merger/add_only.yaml +@@ -0,0 +1,8 @@ ++--- ++document: modulemd-defaults ++version: 1 ++data: ++ module: httpd ++ stream: 2.8 ++ profiles: ++ '2.10': [notreal] +diff --git a/modulemd/tests/test_data/merger/base.yaml b/modulemd/tests/test_data/merger/base.yaml +new file mode 100644 +index 0000000000000000000000000000000000000000..12fbc7142040e4edbcba34b17a1dff341beaf1a4 +--- /dev/null ++++ b/modulemd/tests/test_data/merger/base.yaml +@@ -0,0 +1,34 @@ ++--- ++# Intents optional: ++document: modulemd-defaults ++version: 1 ++data: ++ module: httpd ++ profiles: ++ '2.2': [client, server] ++ '2.8': [notreal] ++ intents: ++ workstation: ++ stream: '2.4' ++ profiles: ++ '2.4': [client] ++ '2.6': [client, server, bindings] ++--- ++document: modulemd-defaults ++version: 1 ++data: ++ module: postgresql ++ stream: '8.1' ++ profiles: ++ '8.1': [client, server, foo] ++ '8.3': [client, server] ++--- ++document: modulemd-defaults ++version: 1 ++data: ++ module: nodejs ++ stream: '8.0' ++ modified: 201909270000 ++ profiles: ++ '6.0': [default] ++ '8.0': [super] +diff --git a/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h b/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h +index 97ca7ebb3824ef62aabbafee391b25d56bac43c3..ad7c02ca5ec236d75e9d3ad51af8e2f6afab54c4 100644 +--- a/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h ++++ b/modulemd/v2/include/modulemd-2.0/modulemd-module-index-merger.h +@@ -51,35 +51,48 @@ G_BEGIN_DECLS + * there exists two #ModulemdModuleStream entries that have different content + * for the same NSVC, the behavior is undefined. + * +- * Merging #ModulemdDefaults entries behaves as follows: ++ * Merging #ModulemdDefaults entries behaves as follows (note that this ++ * behavior has changed slightly as of 2.8.1): + * +- * - Within a ModuleIndex, if two or more default entries reference the same +- * module, the one with the highest modified field will be used and the +- * others discarded. +- * - When merging ModuleIndexes, if two or more indexes contain Defaults for +- * the same module, but different modified values, the one with the highest +- * modified value will be used and the others discarded. +- * - Any module default that is provided by a single repository is +- * authoritative. +- * - If the repos have different priorities (not common), then the default for +- * this module and stream name coming from the repo of higher priority will +- * be used and the default from the lower-priority repo will not be included +- * even if it has a higher modified value. ++ * - Any module defaults object that is provided by a single ++ * #ModulemdModuleIndex will be the defaults object in the resulting merged ++ * #ModulemdModuleIndex. ++ * - If the #ModulemdModuleIndex inputs have different priorities (not common), ++ * then the defaults from the highest priority #ModulemdModuleIndex will be ++ * used and the others entirely discarded. The `modified` value will not be ++ * considered at all. (Priority is intended for providing a total override, ++ * including an on-disk configuration). + * - If the repos have the same priority (such as "fedora" and "updates" in the +- * Fedora Project) and modified value, the entries will be merged as follows: +- * - If both repositories specify the same default stream for the module, use +- * it. +- * - If either repository specifies a default stream for the module and the +- * other does not, use the one specified. +- * - If both repositories specify different default streams, the merge will +- * unset the default stream and proceed with the merge. +- * - If both repositories specify a set of default profiles for a stream and +- * the sets are equivalent, use that set. +- * - If one repository specifies a set of default profiles for a stream and +- * the other does not, use the one specified. +- * - If both repositories specify a set of default profiles for a stream and +- * each are providing a different set, this is an unresolvable merge +- * conflict and the merge resolution will fail and report an error. ++ * Fedora Project) and `modified` value, the entries will be merged as ++ * follows for default streams: ++ * - If both #ModulemdModuleIndex objects specify the same default stream for ++ * the module, that one will be used. ++ * - If either #ModulemdModuleIndex specifies a default stream for the module ++ * and the other does not, the provided one will be used. ++ * - If both #ModulemdModuleIndex objects specify different default streams ++ * and have different `modified` values, the default stream from the ++ * #ModulemdDefaults object with the higher `modified` value will be used. ++ * - If both #ModulemdModuleIndex objects specify different default streams ++ * and have the same `modified` value, the merge will unset the default ++ * stream and leave no default stream in the resulting merged ++ * #ModulemdModuleIndex. This behavior can be controlled by using ++ * modulemd_module_index_merger_resolve_ext() and setting ++ * `strict_default_streams` to #TRUE. In that case, an error will be ++ * returned if conflicting default streams have been provided. ++ * - and for profile defaults: ++ * - If both #ModulemdModuleIndex objects specify a set of default profiles ++ * for a particular module and stream and the sets are equivalent, use that ++ * set. ++ * - If one #ModulemdModuleIndex object specifies a set of default profiles ++ * for a module and stream and the other does not, use the provided set. ++ * - If both #ModulemdModuleIndex objects specify a set of default profiles ++ * for a stream, each are providing a different set and the `modified` ++ * value differs, the set from the object with the higher `modified` value ++ * will be used. ++ * - If both #ModulemdModuleIndex objects specify a set of default profiles ++ * for a stream, each are providing a different set and the `modified` ++ * value is the same, this is an unresolvable merge conflict and the merge ++ * resolution will fail and return an error. + * - Intents behave in exactly the same manner as described for the top-level + * defaults, except that they merge beneath each intent name. + * +diff --git a/modulemd/v2/include/private/modulemd-defaults-v1-private.h b/modulemd/v2/include/private/modulemd-defaults-v1-private.h +index 9144a7ef90853a28eee8345a3b814fdc6478be7b..437bab1afa8c50c4a13924c8bc825ddb0bb211d4 100644 +--- a/modulemd/v2/include/private/modulemd-defaults-v1-private.h ++++ b/modulemd/v2/include/private/modulemd-defaults-v1-private.h +@@ -70,9 +70,28 @@ modulemd_defaults_v1_emit_yaml (ModulemdDefaultsV1 *self, + GError **error); + + ++/** ++ * modulemd_defaults_v1_merge: ++ * @from: (in): A #ModulemdDefaultsV1 object to merge from. ++ * @into: (in): A #ModulemdDefaultsV1 object being merged into. ++ * @strict_default_streams: (in): Whether a stream conflict should throw an ++ * error or just unset the default stream. ++ * @error: (out): A #GError containing the reason for an unresolvable merge ++ * conflict. ++ * ++ * Performs a merge of two #ModulemdDefaultsV1 objects representing the ++ * defaults for a single module name. See the documentation for ++ * #ModulemdModuleIndexMerger for details on the merge algorithm used. ++ * ++ * Returns: (transfer full): A newly-allocated #ModulemdDefaultsV1 object ++ * containing the merged values of @from and @into. If this function encounters ++ * an unresolvable merge conflict, it will return NULL and set @error ++ * appropriately. ++ * ++ * Since: 2.0 ++ */ + ModulemdDefaults * +-modulemd_defaults_v1_merge (const gchar *module_name, +- ModulemdDefaultsV1 *from, ++modulemd_defaults_v1_merge (ModulemdDefaultsV1 *from, + ModulemdDefaultsV1 *into, + gboolean strict_default_streams, + GError **error); +diff --git a/modulemd/v2/modulemd-defaults-v1.c b/modulemd/v2/modulemd-defaults-v1.c +index f51e79d9b0a9fb0395077cd1c3be16529dd53f92..8a18d645de5d95a96e3cc862f78eeedc74595a9e 100644 +--- a/modulemd/v2/modulemd-defaults-v1.c ++++ b/modulemd/v2/modulemd-defaults-v1.c +@@ -204,6 +204,7 @@ modulemd_defaults_v1_set_default_stream (ModulemdDefaultsV1 *self, + else + { + /* This is the fallback default for non-specific intents */ ++ g_clear_pointer (&self->default_stream, g_free); + self->default_stream = g_strdup (default_stream); + } + } +@@ -1192,17 +1193,18 @@ modulemd_defaults_v1_emit_intents (ModulemdDefaultsV1 *self, + } + + static gboolean +-modulemd_defaults_v1_merge_intent_profiles ( ++modulemd_defaults_v1_merge_default_profiles ( + GHashTable *from_profile_defaults, + GHashTable *merged_profile_defaults, ++ guint64 from_modified, ++ guint64 into_modified, + GError **error); + static GHashTable * + modulemd_defaults_v1_copy_intent_profiles (GHashTable *intent_profiles); + + + ModulemdDefaults * +-modulemd_defaults_v1_merge (const gchar *module_name, +- ModulemdDefaultsV1 *from, ++modulemd_defaults_v1_merge (ModulemdDefaultsV1 *from, + ModulemdDefaultsV1 *into, + gboolean strict_default_streams, + GError **error) +@@ -1212,83 +1214,85 @@ modulemd_defaults_v1_merge (const gchar *module_name, + gpointer key, value; + GHashTable *intent_profiles = NULL; + GHashTable *merged_intent_profiles = NULL; ++ guint64 from_modified; ++ guint64 into_modified; + g_autoptr (GHashTable) intent_profile_defaults = NULL; + gchar *intent_name = NULL; + gchar *intent_default_stream = NULL; + gchar *merged_default_stream = NULL; + g_autoptr (GError) nested_error = NULL; ++ const gchar *module_name = ++ modulemd_defaults_get_module_name (MODULEMD_DEFAULTS (into)); + +- merged = modulemd_defaults_v1_new (module_name); ++ from_modified = modulemd_defaults_get_modified (MODULEMD_DEFAULTS (from)); ++ into_modified = modulemd_defaults_get_modified (MODULEMD_DEFAULTS (into)); ++ ++ /* Start from a copy of "into" */ ++ merged = ++ MODULEMD_DEFAULTS_V1 (modulemd_defaults_copy (MODULEMD_DEFAULTS (into))); + + /* Merge the default streams */ +- if (into->default_stream && !from->default_stream) +- { +- modulemd_defaults_v1_set_default_stream ( +- merged, into->default_stream, NULL); +- } +- else if (from->default_stream && !into->default_stream) ++ if (from->default_stream && !merged->default_stream) + { + modulemd_defaults_v1_set_default_stream ( + merged, from->default_stream, NULL); + } +- else if (into->default_stream && from->default_stream) ++ else if (merged->default_stream && from->default_stream) + { +- if (g_str_equal (into->default_stream, DEFAULT_MERGE_CONFLICT)) ++ if (g_str_equal (merged->default_stream, DEFAULT_MERGE_CONFLICT)) + { + /* A previous pass over this same module encountered a merge +- * conflict, so we need to propagate that. ++ * conflict, so keep it. + */ +- modulemd_defaults_v1_set_default_stream ( +- merged, DEFAULT_MERGE_CONFLICT, NULL); + } +- else if (!g_str_equal (into->default_stream, from->default_stream)) ++ else if (!g_str_equal (merged->default_stream, from->default_stream)) + { +- /* They have conflicting default streams */ +- g_info ("Module stream mismatch in merge: %s != %s", +- into->default_stream, +- from->default_stream); +- if (strict_default_streams) ++ if (from_modified > into_modified) + { +- g_set_error (error, +- MODULEMD_ERROR, +- MODULEMD_ERROR_VALIDATE, +- "Default stream mismatch in module %s: %s != %s", +- module_name, +- into->default_stream, +- from->default_stream); +- return NULL; ++ modulemd_defaults_v1_set_default_stream ( ++ merged, from->default_stream, NULL); ++ } ++ else if (from_modified == into_modified) ++ { ++ /* They have conflicting default streams */ ++ g_info ("Module stream mismatch in merge: %s != %s", ++ into->default_stream, ++ from->default_stream); ++ if (strict_default_streams) ++ { ++ g_set_error ( ++ error, ++ MODULEMD_ERROR, ++ MODULEMD_ERROR_VALIDATE, ++ "Default stream mismatch in module %s: %s != %s", ++ module_name, ++ into->default_stream, ++ from->default_stream); ++ return NULL; ++ } ++ modulemd_defaults_v1_set_default_stream ( ++ merged, DEFAULT_MERGE_CONFLICT, NULL); + } +- modulemd_defaults_v1_set_default_stream ( +- merged, DEFAULT_MERGE_CONFLICT, NULL); + } + else + { +- /* They're the same, so store that */ +- modulemd_defaults_v1_set_default_stream ( +- merged, into->default_stream, NULL); ++ /* They're the same, so change nothing */ + } + } + else + { +- /* Both values were NULL. +- * Nothing to do, leave it blank. +- */ ++ /* The 'from' default stream was NULL. Make no changes. */ + } + + + /* == Merge profile defaults == */ + +- /* Iterate through 'into' and add them to merged_defaults */ +- if (!modulemd_defaults_v1_merge_intent_profiles ( +- into->profile_defaults, merged->profile_defaults, &nested_error)) +- { +- g_propagate_error (error, g_steal_pointer (&nested_error)); +- return NULL; +- } +- + /* Iterate through 'from' and see if there are additions or conflicts */ +- if (!modulemd_defaults_v1_merge_intent_profiles ( +- from->profile_defaults, merged->profile_defaults, &nested_error)) ++ if (!modulemd_defaults_v1_merge_default_profiles (from->profile_defaults, ++ merged->profile_defaults, ++ from_modified, ++ into_modified, ++ &nested_error)) + { + g_propagate_error (error, g_steal_pointer (&nested_error)); + return NULL; +@@ -1298,18 +1302,6 @@ modulemd_defaults_v1_merge (const gchar *module_name, + + /* Merge intent default stream values */ + +- /* Iterate through 'into' and add them to merged_defaults */ +- g_hash_table_iter_init (&iter, into->intent_default_streams); +- while (g_hash_table_iter_next (&iter, &key, &value)) +- { +- intent_name = (gchar *)key; +- intent_default_stream = (gchar *)value; +- +- g_hash_table_insert (merged->intent_default_streams, +- g_strdup (intent_name), +- g_strdup (intent_default_stream)); +- } +- + /* Iterate through 'from', adding any new values and checking the existing + * ones for equivalence. + */ +@@ -1321,46 +1313,44 @@ modulemd_defaults_v1_merge (const gchar *module_name, + merged_default_stream = + g_hash_table_lookup (merged->intent_default_streams, intent_name); + ++ /* If there is no new default stream, just jump to the next item */ ++ if (!intent_default_stream) ++ continue; ++ + if (!merged_default_stream) + { + /* New entry, just add it */ + g_hash_table_insert (merged->intent_default_streams, + g_strdup (intent_name), + g_strdup (intent_default_stream)); +- continue; + } + +- if (!g_str_equal (intent_default_stream, merged_default_stream)) ++ else if (!g_str_equal (intent_default_stream, merged_default_stream)) + { +- g_set_error (error, +- MODULEMD_ERROR, +- MODULEMD_ERROR_VALIDATE, +- "Profile default stream mismatch in intents: %s != %s", +- intent_default_stream, +- merged_default_stream); +- return NULL; ++ if (from_modified > into_modified) ++ { ++ g_hash_table_replace (merged->intent_default_streams, ++ g_strdup (intent_name), ++ g_strdup (intent_default_stream)); ++ } ++ else if (into_modified == from_modified) ++ { ++ g_set_error ( ++ error, ++ MODULEMD_ERROR, ++ MODULEMD_ERROR_VALIDATE, ++ "Profile default stream mismatch in intents: %s != %s", ++ intent_default_stream, ++ merged_default_stream); ++ return NULL; ++ } + } + } + + + /* Merge intent default profile values */ + +- /* First copy 'into' to 'merged' */ +- g_hash_table_iter_init (&iter, into->intent_default_profiles); +- while (g_hash_table_iter_next (&iter, &key, &value)) +- { +- intent_name = (gchar *)key; +- intent_profiles = (GHashTable *)value; +- +- intent_profile_defaults = +- modulemd_defaults_v1_copy_intent_profiles (intent_profiles); +- +- g_hash_table_insert (merged->intent_default_profiles, +- g_strdup (intent_name), +- g_steal_pointer (&intent_profile_defaults)); +- } +- +- /* Now copy 'from" into merged, checking for conflicts */ ++ /* Now copy 'from' into merged, checking for conflicts */ + g_hash_table_iter_init (&iter, from->intent_default_profiles); + while (g_hash_table_iter_next (&iter, &key, &value)) + { +@@ -1374,7 +1364,7 @@ modulemd_defaults_v1_merge (const gchar *module_name, + intent_profile_defaults = + modulemd_defaults_v1_copy_intent_profiles (intent_profiles); + +- /* This wasn't in 'into', so just add it */ ++ /* This wasn't in 'merged', so just add it */ + g_hash_table_insert (merged->intent_default_profiles, + g_strdup (intent_name), + g_steal_pointer (&intent_profile_defaults)); +@@ -1384,14 +1374,20 @@ modulemd_defaults_v1_merge (const gchar *module_name, + /* Go through each of the profile defaults and see if they're additive or + * conflicting + */ +- if (!modulemd_defaults_v1_merge_intent_profiles ( +- intent_profiles, merged_intent_profiles, &nested_error)) ++ if (!modulemd_defaults_v1_merge_default_profiles (intent_profiles, ++ merged_intent_profiles, ++ from_modified, ++ into_modified, ++ &nested_error)) + { + g_propagate_error (error, g_steal_pointer (&nested_error)); + return NULL; + } + } + ++ /* Set the modified value to the higher of the two provided */ ++ if (from_modified > into_modified) ++ modulemd_defaults_set_modified (MODULEMD_DEFAULTS (merged), from_modified); + + return MODULEMD_DEFAULTS (g_steal_pointer (&merged)); + } +@@ -1427,9 +1423,11 @@ modulemd_defaults_v1_copy_intent_profiles (GHashTable *intent_profiles) + } + + static gboolean +-modulemd_defaults_v1_merge_intent_profiles ( ++modulemd_defaults_v1_merge_default_profiles ( + GHashTable *from_profile_defaults, + GHashTable *merged_profile_defaults, ++ guint64 from_modified, ++ guint64 into_modified, + GError **error) + { + GHashTableIter iter; +@@ -1459,15 +1457,30 @@ modulemd_defaults_v1_merge_intent_profiles ( + /* Check to see if they match */ + if (!modulemd_hash_table_sets_are_equal (from_profiles, merged_profiles)) + { +- /* The profile sets differed. This is an unresolvable merge +- * conflict +- */ +- g_set_error (error, +- MODULEMD_ERROR, +- MODULEMD_ERROR_VALIDATE, +- "Profile default mismatch in stream: %s", +- stream_name); +- return FALSE; ++ if (from_modified > into_modified) ++ { ++ g_hash_table_replace ( ++ merged_profile_defaults, ++ g_strdup (stream_name), ++ modulemd_hash_table_deep_set_copy (from_profiles)); ++ } ++ else if (into_modified > from_modified) ++ { ++ /* Already there, so just continue */ ++ continue; ++ } ++ else ++ { ++ /* The profile sets differed. This is an unresolvable merge ++ * conflict ++ */ ++ g_set_error (error, ++ MODULEMD_ERROR, ++ MODULEMD_ERROR_VALIDATE, ++ "Profile default mismatch in stream: %s", ++ stream_name); ++ return FALSE; ++ } + } + + /* They were a complete match, so no need to add it a second time */ +diff --git a/modulemd/v2/modulemd-defaults.c b/modulemd/v2/modulemd-defaults.c +index 2157f21fd4079ee096baee2a3a3552df5c273d06..b551c578fda68a8a1ae7532696548e8c18501e68 100644 +--- a/modulemd/v2/modulemd-defaults.c ++++ b/modulemd/v2/modulemd-defaults.c +@@ -396,9 +396,6 @@ modulemd_defaults_merge (ModulemdDefaults *from, + { + g_autoptr (ModulemdDefaults) merged_defaults = NULL; + guint64 mdversion; +- const gchar *module_name = NULL; +- guint64 from_modified; +- guint64 into_modified; + g_autoptr (GError) nested_error = NULL; + + g_return_val_if_fail (MODULEMD_IS_DEFAULTS (from), NULL); +@@ -416,36 +413,19 @@ modulemd_defaults_merge (ModulemdDefaults *from, + NULL); + g_return_val_if_fail (mdversion == MD_DEFAULTS_VERSION_ONE, NULL); + +- from_modified = modulemd_defaults_get_modified (from); +- into_modified = modulemd_defaults_get_modified (into); +- +- if (from_modified > into_modified) +- { +- /* Just return 'from' if it has a higher modified value */ +- return modulemd_defaults_copy (from); +- } +- else if (into_modified > from_modified) +- { +- /* Just return 'into' if it has a higher modified value */ +- return modulemd_defaults_copy (into); +- } +- +- /* Modified value is the same, so we need to merge */ +- +- module_name = modulemd_defaults_get_module_name (into); +- if (!g_str_equal (module_name, modulemd_defaults_get_module_name (from))) ++ if (!g_str_equal (modulemd_defaults_get_module_name (into), ++ modulemd_defaults_get_module_name (from))) + { + g_set_error (error, + MODULEMD_ERROR, + MODULEMD_ERROR_VALIDATE, + "Module name mismatch in merge: %s != %s", +- module_name, ++ modulemd_defaults_get_module_name (into), + modulemd_defaults_get_module_name (from)); + return NULL; + } + +- merged_defaults = modulemd_defaults_v1_merge (module_name, +- MODULEMD_DEFAULTS_V1 (from), ++ merged_defaults = modulemd_defaults_v1_merge (MODULEMD_DEFAULTS_V1 (from), + MODULEMD_DEFAULTS_V1 (into), + strict_default_streams, + &nested_error); +diff --git a/modulemd/v2/tests/ModulemdTests/merger.py b/modulemd/v2/tests/ModulemdTests/merger.py +index 92c90d185f55b8a7d5f856aa16cdce5ff32f30d8..72bd43f9d775923abf6a456c70b01bf3d0a912fc 100644 +--- a/modulemd/v2/tests/ModulemdTests/merger.py ++++ b/modulemd/v2/tests/ModulemdTests/merger.py +@@ -14,6 +14,7 @@ + + from os import path + import sys ++import logging + try: + import unittest + import gi +@@ -231,6 +232,124 @@ data: + with self.assertRaisesRegexp(gi.repository.GLib.GError, "Default stream mismatch in module python"): + merger.resolve_ext(True) + ++ def test_merge_add_only(self): ++ base_idx = Modulemd.ModuleIndex() ++ self.assertTrue(base_idx.update_from_file( ++ path.join( ++ self.test_data_path, ++ "merger", ++ "base.yaml"), True)) ++ ++ add_only_idx = Modulemd.ModuleIndex() ++ self.assertTrue(add_only_idx.update_from_file( ++ path.join( ++ self.test_data_path, ++ "merger", ++ "add_only.yaml"), True)) ++ ++ merger = Modulemd.ModuleIndexMerger() ++ merger.associate_index(base_idx, 0) ++ merger.associate_index(add_only_idx, 0) ++ ++ merged_idx = merger.resolve() ++ self.assertIsNotNone(merged_idx) ++ ++ httpd = merged_idx.get_module('httpd') ++ self.assertIsNotNone(httpd) ++ httpd_defs = httpd.get_defaults() ++ ++ self.assertEqual(httpd_defs.get_default_stream(), "2.8") ++ expected_profile_defs = { ++ '2.2': set(['client', 'server']), ++ '2.8': set(['notreal', ]), ++ '2.10': set(['notreal', ]) ++ } ++ ++ for stream in expected_profile_defs.keys(): ++ self.assertEqual(set(httpd_defs.get_default_profiles_for_stream( ++ stream)), expected_profile_defs[stream]) ++ ++ self.assertEqual(httpd_defs.get_default_stream("workstation"), "2.4") ++ ++ def test_merge_add_conflicting_stream(self): ++ base_idx = Modulemd.ModuleIndex() ++ self.assertTrue(base_idx.update_from_file( ++ path.join( ++ self.test_data_path, ++ "merger", ++ "base.yaml"), True)) ++ ++ add_only_idx = Modulemd.ModuleIndex() ++ self.assertTrue(add_only_idx.update_from_file( ++ path.join( ++ self.test_data_path, ++ "merger", ++ "add_conflicting_stream.yaml"), True)) ++ ++ merger = Modulemd.ModuleIndexMerger() ++ merger.associate_index(base_idx, 0) ++ merger.associate_index(add_only_idx, 0) ++ ++ merged_idx = merger.resolve() ++ self.assertIsNotNone(merged_idx) ++ ++ psql = merged_idx.get_module('postgresql') ++ self.assertIsNotNone(psql) ++ ++ psql_defs = psql.get_defaults() ++ self.assertIsNotNone(psql_defs) ++ ++ self.assertIsNone(psql_defs.get_default_stream()) ++ ++ expected_profile_defs = { ++ '8.1': set(['client', 'server', 'foo']), ++ '8.2': set(['client', 'server', 'foo']), ++ } ++ ++ for stream in expected_profile_defs.keys(): ++ self.assertEqual(set(psql_defs.get_default_profiles_for_stream( ++ stream)), expected_profile_defs[stream]) ++ ++ def test_merge_add_conflicting_stream_and_profile_modified(self): ++ base_idx = Modulemd.ModuleIndex() ++ self.assertTrue(base_idx.update_from_file( ++ path.join( ++ self.test_data_path, ++ "merger", ++ "base.yaml"), True)) ++ ++ add_conflicting_idx = Modulemd.ModuleIndex() ++ self.assertTrue(add_conflicting_idx.update_from_file( ++ path.join( ++ self.test_data_path, ++ "merger", ++ "add_conflicting_stream_and_profile_modified.yaml"), True)) ++ ++ merger = Modulemd.ModuleIndexMerger() ++ merger.associate_index(base_idx, 0) ++ merger.associate_index(add_conflicting_idx, 0) ++ ++ merged_idx = merger.resolve() ++ self.assertIsNotNone(merged_idx) ++ ++ psql = merged_idx.get_module('postgresql') ++ self.assertIsNotNone(psql) ++ ++ psql_defs = psql.get_defaults() ++ self.assertIsNotNone(psql_defs) ++ ++ self.assertEqual(psql_defs.get_default_stream(), '8.2') ++ ++ expected_profile_defs = { ++ '8.1': set(['client', 'server']), ++ '8.2': set(['client', 'server', 'foo']), ++ '8.3': set(['client', 'server']), ++ } ++ ++ for stream in expected_profile_defs: ++ self.assertEqual(set(psql_defs.get_default_profiles_for_stream( ++ stream)), expected_profile_defs[stream]) ++ + + if __name__ == '__main__': + unittest.main() +diff --git a/modulemd/v2/tests/test-modulemd-merger.c b/modulemd/v2/tests/test-modulemd-merger.c +index e46b4d84441528310c2d4e5b5990f23e3719090e..967e2cc902a5708905e9762e4d4631ab2eb4e4eb 100644 +--- a/modulemd/v2/tests/test-modulemd-merger.c ++++ b/modulemd/v2/tests/test-modulemd-merger.c +@@ -14,6 +14,8 @@ + #include + #include + ++#include "modulemd-defaults.h" ++#include "modulemd-defaults-v1.h" + #include "modulemd-module-index.h" + #include "modulemd-module-index-merger.h" + #include "private/test-utils.h" +@@ -101,6 +103,142 @@ merger_test_deduplicate (CommonMmdTestFixture *fixture, + } + + ++static void ++merger_test_add_only (void) ++{ ++ g_autoptr (GPtrArray) failures = NULL; ++ g_autoptr (GError) error = NULL; ++ g_autoptr (ModulemdModuleIndex) base_idx = modulemd_module_index_new (); ++ g_autoptr (ModulemdModuleIndex) add_only_idx = modulemd_module_index_new (); ++ g_autoptr (ModulemdModuleIndex) merged_idx = NULL; ++ g_autoptr (ModulemdModuleIndexMerger) merger = ++ modulemd_module_index_merger_new (); ++ ModulemdModule *httpd = NULL; ++ ModulemdDefaults *httpd_defs = NULL; ++ g_autofree gchar *base_yaml = ++ g_strdup_printf ("%s/merger/base.yaml", g_getenv ("TEST_DATA_PATH")); ++ g_autofree gchar *add_only_yaml = ++ g_strdup_printf ("%s/merger/add_only.yaml", g_getenv ("TEST_DATA_PATH")); ++ ++ g_assert_true (modulemd_module_index_update_from_file ( ++ base_idx, base_yaml, TRUE, &failures, &error)); ++ g_assert_true (modulemd_module_index_update_from_file ( ++ add_only_idx, add_only_yaml, TRUE, &failures, &error)); ++ ++ modulemd_module_index_merger_associate_index (merger, base_idx, 0); ++ modulemd_module_index_merger_associate_index (merger, add_only_idx, 0); ++ ++ merged_idx = modulemd_module_index_merger_resolve_ext (merger, TRUE, &error); ++ g_assert_no_error (error); ++ g_assert_nonnull (merged_idx); ++ ++ httpd = modulemd_module_index_get_module (merged_idx, "httpd"); ++ g_assert_nonnull (httpd); ++ ++ httpd_defs = modulemd_module_get_defaults (httpd); ++ g_assert_nonnull (httpd_defs); ++ ++ g_assert_cmpstr (modulemd_defaults_v1_get_default_stream ( ++ MODULEMD_DEFAULTS_V1 (httpd_defs), NULL), ++ ==, ++ "2.8"); ++ ++ g_assert_cmpstr (modulemd_defaults_v1_get_default_stream ( ++ MODULEMD_DEFAULTS_V1 (httpd_defs), "workstation"), ++ ==, ++ "2.4"); ++} ++ ++ ++static void ++merger_test_add_conflicting_stream (void) ++{ ++ g_autoptr (GPtrArray) failures = NULL; ++ g_autoptr (GError) error = NULL; ++ g_autoptr (ModulemdModuleIndex) base_idx = modulemd_module_index_new (); ++ g_autoptr (ModulemdModuleIndex) add_conflicting_idx = ++ modulemd_module_index_new (); ++ g_autoptr (ModulemdModuleIndex) merged_idx = NULL; ++ g_autoptr (ModulemdModuleIndexMerger) merger = ++ modulemd_module_index_merger_new (); ++ ModulemdModule *psql = NULL; ++ ModulemdDefaults *psql_defs = NULL; ++ g_autofree gchar *base_yaml = ++ g_strdup_printf ("%s/merger/base.yaml", g_getenv ("TEST_DATA_PATH")); ++ g_autofree gchar *add_conflicting_yaml = g_strdup_printf ( ++ "%s/merger/add_conflicting_stream.yaml", g_getenv ("TEST_DATA_PATH")); ++ ++ g_assert_true (modulemd_module_index_update_from_file ( ++ base_idx, base_yaml, TRUE, &failures, &error)); ++ g_assert_true (modulemd_module_index_update_from_file ( ++ add_conflicting_idx, add_conflicting_yaml, TRUE, &failures, &error)); ++ ++ modulemd_module_index_merger_associate_index (merger, base_idx, 0); ++ modulemd_module_index_merger_associate_index ( ++ merger, add_conflicting_idx, 0); ++ ++ merged_idx = ++ modulemd_module_index_merger_resolve_ext (merger, FALSE, &error); ++ g_assert_no_error (error); ++ g_assert_nonnull (merged_idx); ++ ++ psql = modulemd_module_index_get_module (merged_idx, "postgresql"); ++ g_assert_nonnull (psql); ++ ++ psql_defs = modulemd_module_get_defaults (psql); ++ g_assert_nonnull (psql_defs); ++ ++ g_assert_null (modulemd_defaults_v1_get_default_stream ( ++ MODULEMD_DEFAULTS_V1 (psql_defs), NULL)); ++} ++ ++ ++static void ++merger_test_add_conflicting_stream_and_profile_modified (void) ++{ ++ g_autoptr (GPtrArray) failures = NULL; ++ g_autoptr (GError) error = NULL; ++ g_autoptr (ModulemdModuleIndex) base_idx = modulemd_module_index_new (); ++ g_autoptr (ModulemdModuleIndex) add_conflicting_idx = ++ modulemd_module_index_new (); ++ g_autoptr (ModulemdModuleIndex) merged_idx = NULL; ++ g_autoptr (ModulemdModuleIndexMerger) merger = ++ modulemd_module_index_merger_new (); ++ ModulemdModule *psql = NULL; ++ ModulemdDefaults *psql_defs = NULL; ++ g_autofree gchar *base_yaml = ++ g_strdup_printf ("%s/merger/base.yaml", g_getenv ("TEST_DATA_PATH")); ++ g_autofree gchar *add_conflicting_yaml = g_strdup_printf ( ++ "%s/merger/add_conflicting_stream_and_profile_modified.yaml", ++ g_getenv ("TEST_DATA_PATH")); ++ ++ g_assert_true (modulemd_module_index_update_from_file ( ++ base_idx, base_yaml, TRUE, &failures, &error)); ++ g_assert_true (modulemd_module_index_update_from_file ( ++ add_conflicting_idx, add_conflicting_yaml, TRUE, &failures, &error)); ++ ++ modulemd_module_index_merger_associate_index (merger, base_idx, 0); ++ modulemd_module_index_merger_associate_index ( ++ merger, add_conflicting_idx, 0); ++ ++ merged_idx = ++ modulemd_module_index_merger_resolve_ext (merger, FALSE, &error); ++ g_assert_no_error (error); ++ g_assert_nonnull (merged_idx); ++ ++ psql = modulemd_module_index_get_module (merged_idx, "postgresql"); ++ g_assert_nonnull (psql); ++ ++ psql_defs = modulemd_module_get_defaults (psql); ++ g_assert_nonnull (psql_defs); ++ ++ g_assert_cmpstr (modulemd_defaults_v1_get_default_stream ( ++ MODULEMD_DEFAULTS_V1 (psql_defs), NULL), ++ ==, ++ "8.2"); ++} ++ ++ + int + main (int argc, char *argv[]) + { +@@ -125,5 +263,14 @@ main (int argc, char *argv[]) + merger_test_deduplicate, + NULL); + ++ g_test_add_func ("/modulemd/module/index/merger/add_only", ++ merger_test_add_only); ++ ++ g_test_add_func ("/modulemd/module/index/merger/add_conflicting_stream", ++ merger_test_add_conflicting_stream); ++ ++ g_test_add_func ("/modulemd/module/index/merger/add_conflicting_both", ++ merger_test_add_conflicting_stream_and_profile_modified); ++ + return g_test_run (); + } +-- +2.23.0 + diff --git a/SOURCES/0008-Rework-defaults-merging-logic-1x.patch b/SOURCES/0008-Rework-defaults-merging-logic-1x.patch new file mode 100644 index 0000000..93bb0f2 --- /dev/null +++ b/SOURCES/0008-Rework-defaults-merging-logic-1x.patch @@ -0,0 +1,503 @@ +From a16e69a71778bc81e5bb35ab98048a650cd4edcc Mon Sep 17 00:00:00 2001 +From: Stephen Gallagher +Date: Tue, 8 Oct 2019 16:05:13 -0400 +Subject: [PATCH 8/8] Rework defaults merging logic + +Fixes: https://github.com/fedora-modularity/libmodulemd/issues/378 + +Signed-off-by: Stephen Gallagher +--- + modulemd/v1/modulemd-defaults.c | 347 +++++++++++++------- + modulemd/v1/tests/test-modulemd-defaults.c | 18 +- + test_data/defaults/overriding-modified.yaml | 5 +- + 3 files changed, 242 insertions(+), 128 deletions(-) + +diff --git a/modulemd/v1/modulemd-defaults.c b/modulemd/v1/modulemd-defaults.c +index d44076eef0ac3bffae05f74b852fb6b51c2aee64..9fa00f4104938f83e023070e6971608b0d27ddfa 100644 +--- a/modulemd/v1/modulemd-defaults.c ++++ b/modulemd/v1/modulemd-defaults.c +@@ -725,27 +725,36 @@ modulemd_defaults_copy (ModulemdDefaults *self) + } + + ++static gboolean ++modulemd_defaults_merge_default_profiles (GHashTable *from_profile_defaults, ++ GHashTable *merged_profile_defaults, ++ guint64 from_modified, ++ guint64 into_modified, ++ GError **error); ++ ++static void ++modulemd_defaults_merge_intent_default_streams (ModulemdIntent *from_intent, ++ ModulemdIntent *into_intent, ++ const gchar *intent_name, ++ guint64 from_modified, ++ guint64 into_modified); ++ + ModulemdDefaults * + modulemd_defaults_merge (ModulemdDefaults *first, + ModulemdDefaults *second, + gboolean override, + GError **error) + { +- g_autoptr (ModulemdDefaults) defaults = NULL; +- GHashTable *profile_defaults = NULL; +- g_autoptr (GHashTable) intents = NULL; +- ModulemdIntent *base_intent = NULL; +- ModulemdIntent *merge_intent = NULL; +- g_autoptr (ModulemdIntent) new_intent = NULL; +- g_autoptr (GHashTable) base_profiles = NULL; +- GHashTable *merge_profiles = NULL; ++ g_autoptr (ModulemdDefaults) merged = NULL; ++ ModulemdIntent *from_intent = NULL; ++ ModulemdIntent *merged_intent = NULL; + const gchar *intent_name = NULL; +- ModulemdSimpleSet *profile = NULL; +- GHashTableIter iter, profile_iter; +- gpointer key, value, orig_value, prof_key, prof_value; ++ GHashTableIter iter; ++ gpointer key, value; + +- g_return_val_if_fail (MODULEMD_IS_DEFAULTS (first), NULL); +- g_return_val_if_fail (MODULEMD_IS_DEFAULTS (second), NULL); ++ ++ g_return_val_if_fail (first && MODULEMD_IS_DEFAULTS (first), NULL); ++ g_return_val_if_fail (second && MODULEMD_IS_DEFAULTS (second), NULL); + + if (override) + { +@@ -755,156 +764,250 @@ modulemd_defaults_merge (ModulemdDefaults *first, + return modulemd_defaults_copy (second); + } + +- /* Compare modified values */ +- if (modulemd_defaults_get_modified (first) > +- modulemd_defaults_get_modified (second)) ++ /* Start from a copy of the base */ ++ merged = modulemd_defaults_copy (first); ++ ++ /* == Merge default streams == */ ++ if (second->default_stream && !merged->default_stream) + { +- /* If first has a higher modified value, return a copy of it */ +- return modulemd_defaults_copy (first); ++ /* Only the second Defaults had a default stream, so set that */ ++ modulemd_defaults_set_default_stream (merged, second->default_stream); + } +- else if (modulemd_defaults_get_modified (second) > +- modulemd_defaults_get_modified (first)) ++ else if (merged->default_stream && second->default_stream) + { +- /* If second has a higher modified value, return a copy of it */ +- return modulemd_defaults_copy (second); +- } +- ++ /* Both of them had a defaults set */ + +- /* They had the same 'modified' value (such as both zero, for +- * backwards-compatibility with 1.7.x and older. +- * Merge them as best we can. +- */ +- +- defaults = modulemd_defaults_copy (first); +- +- /* First check for incompatibilities with the streams */ +- if (g_strcmp0 (first->default_stream, second->default_stream)) +- { +- /* Default streams don't match and override is not set. +- * Return an error ++ /* Shortcut past if we already know there are conflicts in this ++ * default stream. + */ +- /* 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); +- } +- +- /* Merge the profile defaults */ +- profile_defaults = modulemd_defaults_peek_profile_defaults (defaults); +- +- g_hash_table_iter_init (&iter, +- modulemd_defaults_peek_profile_defaults (second)); +- while (g_hash_table_iter_next (&iter, &key, &value)) +- { +- orig_value = g_hash_table_lookup (profile_defaults, key); +- if (orig_value) ++ if (!g_str_equal (merged->default_stream, DEFAULT_MERGE_CONFLICT)) + { +- /* This key already exists in the first defaults object. +- * Check whether they have identical values ++ /* If second has a higher modified value, use its value. ++ * If first has a higher modified value, it's already saved in ++ * merged from the copy() + */ +- if (!modulemd_simpleset_is_equal (orig_value, value)) ++ if (second->modified > first->modified) + { +- g_set_error (error, +- MODULEMD_DEFAULTS_ERROR, +- MODULEMD_DEFAULTS_ERROR_CONFLICTING_PROFILES, +- "Conflicting profile defaults when merging " +- "defaults for module %s", +- modulemd_defaults_peek_module_name (first)); +- return NULL; ++ modulemd_defaults_set_default_stream (merged, ++ second->default_stream); ++ } ++ else if (first->modified == second->modified) ++ { ++ if (!g_str_equal (first->default_stream, second->default_stream)) ++ { ++ /* They have conflicting default streams */ ++ g_info ("Module stream mismatch in merge: %s != %s", ++ first->default_stream, ++ second->default_stream); ++ ++ /* Set the special conflicting value */ ++ modulemd_defaults_set_default_stream ( ++ merged, DEFAULT_MERGE_CONFLICT); ++ } ++ /* Otherwise, they are the same and merged will have the correct ++ * value from the copy() ++ */ + } +- } +- else +- { +- /* This key is new. Add it */ +- g_hash_table_replace (profile_defaults, +- g_strdup (key), +- g_object_ref (MODULEMD_SIMPLESET (value))); + } + } ++ /* If neither of the above matched, both first and second had NULL for the ++ * default stream, so nothing to do ++ */ ++ ++ ++ /* == Merge profile defaults == */ ++ if (!modulemd_defaults_merge_default_profiles (second->profile_defaults, ++ merged->profile_defaults, ++ second->modified, ++ first->modified, ++ error)) ++ { ++ return NULL; ++ } ++ ++ ++ /* == Merge intent defaults == */ + ++ /* --- Merge intent default stream values --- */ + +- /* Merge intents */ +- intents = modulemd_defaults_dup_intents (defaults); ++ /* Iterate through 'second', adding any new values and checking the existing ++ * ones for equivalence. ++ */ + g_hash_table_iter_init (&iter, modulemd_defaults_peek_intents (second)); + while (g_hash_table_iter_next (&iter, &key, &value)) + { +- merge_intent = MODULEMD_INTENT (value); +- /* Check if this module name exists in the current table */ +- intent_name = modulemd_intent_peek_intent_name (merge_intent); +- base_intent = g_hash_table_lookup ( +- intents, modulemd_intent_peek_intent_name (merge_intent)); ++ g_return_val_if_fail (value && MODULEMD_IS_INTENT (value), FALSE); + +- if (!base_intent) ++ intent_name = (gchar *)key; ++ from_intent = MODULEMD_INTENT (value); ++ ++ merged_intent = g_hash_table_lookup (merged->intents, intent_name); ++ if (!merged_intent) + { + /* This intent doesn't exist yet, so just add it completely. */ +- g_hash_table_insert (intents, ++ g_hash_table_insert (merged->intents, + g_strdup (intent_name), +- modulemd_intent_copy (merge_intent)); ++ modulemd_intent_copy (from_intent)); + continue; + } + +- /* Compare the default stream for this intent */ +- if (g_strcmp0 (modulemd_intent_peek_default_stream (base_intent), +- modulemd_intent_peek_default_stream (merge_intent))) ++ /* Merge the intent default streams */ ++ modulemd_defaults_merge_intent_default_streams (from_intent, ++ merged_intent, ++ intent_name, ++ second->modified, ++ first->modified); ++ ++ /* Merge the intent default profiles */ ++ if (!modulemd_defaults_merge_default_profiles ( ++ modulemd_intent_peek_profile_defaults (from_intent), ++ modulemd_intent_peek_profile_defaults (merged_intent), ++ second->modified, ++ first->modified, ++ error)) + { +- /* The streams didn't match, so bail out */ +- g_set_error (error, +- MODULEMD_DEFAULTS_ERROR, +- MODULEMD_DEFAULTS_ERROR_CONFLICTING_INTENT_STREAM, +- "Conflicting default stream for intent profile [%s]" +- "when merging defaults for module %s", +- (const gchar *)intent_name, +- modulemd_defaults_peek_module_name (first)); + return NULL; + } ++ } + +- /* Construct a new Intent with the merged values which will replace +- * the existing one at the end */ +- new_intent = modulemd_intent_copy (base_intent); ++ /* Set the modified value to the higher of the two provided */ ++ if (second->modified > first->modified) ++ modulemd_defaults_set_modified (merged, second->modified); + +- /* Merge the profile definitions for this intent */ +- base_profiles = modulemd_intent_dup_profile_defaults (new_intent); ++ return g_steal_pointer (&merged); ++} + +- merge_profiles = modulemd_intent_peek_profile_defaults (merge_intent); +- g_hash_table_iter_init (&profile_iter, merge_profiles); +- while (g_hash_table_iter_next (&profile_iter, &prof_key, &prof_value)) ++ ++static gboolean ++modulemd_defaults_merge_default_profiles (GHashTable *from_profile_defaults, ++ GHashTable *merged_profile_defaults, ++ guint64 from_modified, ++ guint64 into_modified, ++ GError **error) ++{ ++ GHashTableIter iter; ++ gpointer key, value; ++ gchar *stream_name = NULL; ++ ModulemdSimpleSet *from_profiles = NULL; ++ ModulemdSimpleSet *merged_profiles = NULL; ++ ModulemdSimpleSet *copied_profiles = NULL; ++ ++ g_hash_table_iter_init (&iter, from_profile_defaults); ++ while (g_hash_table_iter_next (&iter, &key, &value)) ++ { ++ stream_name = (gchar *)key; ++ from_profiles = (ModulemdSimpleSet *)value; ++ merged_profiles = ++ g_hash_table_lookup (merged_profile_defaults, stream_name); ++ ++ if (!merged_profiles) + { +- /* Check if this profile exists in this intent */ +- profile = g_hash_table_lookup (base_profiles, prof_key); ++ /* Didn't appear in the profiles list, so just add it to merged */ ++ modulemd_simpleset_copy (from_profiles, &copied_profiles); ++ g_hash_table_insert ( ++ merged_profile_defaults, g_strdup (stream_name), copied_profiles); ++ copied_profiles = NULL; ++ continue; ++ } + +- if (!profile) ++ /* Check to see if they match */ ++ if (!modulemd_simpleset_is_equal (from_profiles, merged_profiles)) ++ { ++ if (from_modified > into_modified) ++ { ++ modulemd_simpleset_copy (from_profiles, &copied_profiles); ++ g_hash_table_insert (merged_profile_defaults, ++ g_strdup (stream_name), ++ copied_profiles); ++ copied_profiles = NULL; ++ } ++ else if (into_modified > from_modified) + { +- /* Add this profile to the intent */ +- modulemd_simpleset_copy (prof_value, &profile); +- g_hash_table_insert ( +- base_profiles, g_strdup ((const gchar *)prof_key), profile); ++ /* Already there, so just continue */ + continue; + } +- +- if (!modulemd_simpleset_is_equal (profile, prof_value)) ++ else + { +- /* If we get here, the sets were unequal, so we need to fail */ ++ /* The profile sets differed. This is an unresolvable merge ++ * conflict ++ */ + g_set_error (error, + MODULEMD_DEFAULTS_ERROR, +- MODULEMD_DEFAULTS_ERROR_CONFLICTING_INTENT_PROFILE, +- "Conflicting intent profile [%s:%s] when merging " +- "defaults for module %s", +- (const gchar *)intent_name, +- (const gchar *)prof_key, +- modulemd_defaults_peek_module_name (first)); +- return NULL; ++ MODULEMD_DEFAULTS_ERROR_CONFLICTING_PROFILES, ++ "Profile default mismatch in stream: %s", ++ stream_name); ++ return FALSE; + } + } + +- modulemd_intent_set_profile_defaults (new_intent, base_profiles); +- g_clear_pointer (&base_profiles, g_hash_table_unref); +- g_hash_table_replace ( +- intents, g_strdup (intent_name), g_object_ref (new_intent)); +- g_clear_pointer (&new_intent, g_object_unref); ++ /* They were a complete match, so no need to add it a second time */ + } + +- modulemd_defaults_set_intents (defaults, intents); ++ return TRUE; ++} ++ ++static void ++modulemd_defaults_merge_intent_default_streams (ModulemdIntent *from_intent, ++ ModulemdIntent *into_intent, ++ const gchar *intent_name, ++ guint64 from_modified, ++ guint64 into_modified) ++{ ++ const gchar *from_default_stream = NULL; ++ const gchar *into_default_stream = NULL; ++ ++ g_return_if_fail (from_intent && MODULEMD_IS_INTENT (from_intent)); ++ g_return_if_fail (into_intent && MODULEMD_IS_INTENT (into_intent)); ++ g_return_if_fail (intent_name); ++ ++ from_default_stream = modulemd_intent_peek_default_stream (from_intent); ++ ++ /* If there is no new default stream, just jump to the next item */ ++ if (!from_default_stream) ++ return; ++ ++ into_default_stream = modulemd_intent_peek_default_stream (into_intent); ++ ++ ++ /* If a previous merge has already marked this as conflicting, just bail ++ * out here and move on to the next intent ++ */ ++ if (g_str_equal (into_default_stream, DEFAULT_MERGE_CONFLICT)) ++ return; + +- return g_object_ref (defaults); ++ ++ if (into_default_stream) ++ { ++ /* Both default stream names are present. ++ * If they are equal, there's nothing to do. ++ */ ++ ++ if (!g_str_equal (into_default_stream, from_default_stream)) ++ { ++ if (from_modified > into_modified) ++ { ++ /* Set the default stream of from as the merged value */ ++ modulemd_intent_set_default_stream (into_intent, ++ from_default_stream); ++ return; ++ } ++ else if (into_modified == from_modified) ++ { ++ g_info ( ++ "Module stream mismatch in merge: %s != %s for intent %s", ++ into_default_stream, ++ from_default_stream, ++ intent_name); ++ modulemd_intent_set_default_stream (into_intent, ++ DEFAULT_MERGE_CONFLICT); ++ return; ++ } ++ /* Otherwise into is already set, so do nothing */ ++ } ++ } ++ else /* !into_default_stream */ ++ { ++ /* There was no default stream set yet, so just add the new one */ ++ modulemd_intent_set_default_stream (into_intent, from_default_stream); ++ } + } +diff --git a/modulemd/v1/tests/test-modulemd-defaults.c b/modulemd/v1/tests/test-modulemd-defaults.c +index 35142b885b3ec30f5109d53befe17be3e08d7b5a..c088e87b442af37e69845216db33afa8361785cc 100644 +--- a/modulemd/v1/tests/test-modulemd-defaults.c ++++ b/modulemd/v1/tests/test-modulemd-defaults.c +@@ -913,7 +913,7 @@ modulemd_defaults_test_prioritizer_modified (DefaultsFixture *fixture, + g_assert_cmpstr ( + modulemd_defaults_peek_default_stream (defaults), ==, "2.4"); + htable = modulemd_defaults_peek_profile_defaults (defaults); +- g_assert_cmpint (g_hash_table_size (htable), ==, 2); ++ g_assert_cmpint (g_hash_table_size (htable), ==, 3); + g_assert_true (g_hash_table_contains (htable, "2.2")); + g_assert_true (modulemd_simpleset_contains ( + g_hash_table_lookup (htable, "2.2"), "client")); +@@ -924,7 +924,10 @@ modulemd_defaults_test_prioritizer_modified (DefaultsFixture *fixture, + g_hash_table_lookup (htable, "2.2"), "client")); + g_assert_true (modulemd_simpleset_contains ( + g_hash_table_lookup (htable, "2.4"), "server")); +- g_assert_false (g_hash_table_contains (htable, "2.8")); ++ g_assert_true (g_hash_table_contains (htable, "2.8")); ++ g_assert_true (modulemd_simpleset_contains ( ++ g_hash_table_lookup (htable, "2.8"), "notreal")); ++ + + /* NODEJS */ + defaults = MODULEMD_DEFAULTS (g_ptr_array_index (merged_objects, 1)); +@@ -956,14 +959,21 @@ modulemd_defaults_test_prioritizer_modified (DefaultsFixture *fixture, + g_assert_cmpstr ( + modulemd_defaults_peek_default_stream (defaults), ==, "8.1"); + htable = modulemd_defaults_peek_profile_defaults (defaults); +- g_assert_cmpint (g_hash_table_size (htable), ==, 1); ++ g_assert_cmpint (g_hash_table_size (htable), ==, 2); + g_assert_true (g_hash_table_contains (htable, "8.1")); + g_assert_true (modulemd_simpleset_contains ( + g_hash_table_lookup (htable, "8.1"), "client")); + g_assert_true (modulemd_simpleset_contains ( + g_hash_table_lookup (htable, "8.1"), "server")); +- g_assert_true ( ++ g_assert_false ( + modulemd_simpleset_contains (g_hash_table_lookup (htable, "8.1"), "foo")); ++ g_assert_true (g_hash_table_contains (htable, "8.2")); ++ g_assert_true (modulemd_simpleset_contains ( ++ g_hash_table_lookup (htable, "8.2"), "client")); ++ g_assert_true (modulemd_simpleset_contains ( ++ g_hash_table_lookup (htable, "8.2"), "server")); ++ g_assert_true ( ++ modulemd_simpleset_contains (g_hash_table_lookup (htable, "8.2"), "foo")); + } + + +diff --git a/test_data/defaults/overriding-modified.yaml b/test_data/defaults/overriding-modified.yaml +index 719892ee0e999e4c48a9c69f6d4fdf84b0c5d89d..b87ba98ca99da128f78f8c560af3a7d928e8fde9 100644 +--- a/test_data/defaults/overriding-modified.yaml ++++ b/test_data/defaults/overriding-modified.yaml +@@ -17,7 +17,7 @@ data: + '2.6': [client, server, bindings] + '2.8': [client, server, bindings, new] + --- +-# Reduce the number of profile defaults ++# Drop one profile from a stream and add it to another + document: modulemd-defaults + version: 1 + data: +@@ -25,7 +25,8 @@ data: + modified: 201812061200 + stream: '8.1' + profiles: +- '8.1': [client, server, foo] ++ '8.1': [client, server] ++ '8.2': [client, server, foo] + --- + # Override the default stream + document: modulemd-defaults +-- +2.23.0 + diff --git a/SPECS/libmodulemd.spec b/SPECS/libmodulemd.spec index a8a288c..39b7b33 100644 --- a/SPECS/libmodulemd.spec +++ b/SPECS/libmodulemd.spec @@ -3,7 +3,7 @@ Name: libmodulemd Version: %{libmodulemd_version} -Release: 2%{?dist} +Release: 4%{?dist} Summary: Module metadata manipulation library License: MIT @@ -32,6 +32,13 @@ Conflicts: libmodulemd1 < %{libmodulemd_v1_version}-%{release} Patch0001: 0001-Double-valgrind-timeout.patch Patch0002: 0002-Parallelize-the-valgrind-tests.patch Patch0003: 0003-Fix-transfer-type-for-Module.search_streams.patch +Patch0004: 0004-Extend-timeout-for-header-test.patch + +#RHBZ #1763779- Merging defaults from third-party repositories +Patch0005: 0005-Add-test_data_path-env-var-for-Python-tests.patch +Patch0006: 0006-Add-ModuleIndexMerger.resolve_ext.patch +Patch0007: 0007-Rework-defaults-merging-logic-2x.patch +Patch0008: 0008-Rework-defaults-merging-logic-1x.patch %description @@ -108,7 +115,15 @@ Python 3 bindings for libmodulemd1 %meson_build %define _vpath_builddir api2 + +%ifarch aarch64 +# aarch64 builders have I/O issues that often causes the valgrind tests to +# time out. Skip them from the RPM build +export MMD_SKIP_VALGRIND=True +%endif %meson -Ddeveloper_build=false -Dbuild_api_v1=false -Dbuild_api_v2=true -Dwith_py3_overrides=true -Dwith_py2_overrides=false + + %meson_build @@ -126,10 +141,10 @@ export MMD_SKIP_VALGRIND=1 %endif %define _vpath_builddir api1 -%meson_test +%{__meson} test -C %{_vpath_builddir} %{?_smp_mesonflags} --print-errorlogs -t 10 %define _vpath_builddir api2 -%meson_test +%{__meson} test -C %{_vpath_builddir} %{?_smp_mesonflags} --print-errorlogs -t 10 %install @@ -191,6 +206,10 @@ ln -s libmodulemd.so.%{libmodulemd_v1_version} \ %changelog +* Wed Oct 23 2019 Stephen Gallagher - 2.5.0-4 +- Improve default merging logic when dealing with third-party repos +- Resolves: rhbz#1763779 + * Wed May 29 2019 Stephen Gallagher - 2.5.0-2 - Fix memory corruption error using Module.search_rpms() from python - Speed up valgrind tests