diff --git a/module_build_service/utils/reuse.py b/module_build_service/utils/reuse.py index e5662ebc..675de2f4 100644 --- a/module_build_service/utils/reuse.py +++ b/module_build_service/utils/reuse.py @@ -403,8 +403,8 @@ def get_reusable_component( # Create separate lists for the new and previous module build. These lists # will have an entry for every build batch *before* the component's # batch except for 1, which is reserved for the module-build-macros RPM. - # Each batch entry will contain a set of "(name, ref)" with the name and - # ref (commit) of the component. + # Each batch entry will contain a set of "(name, ref, arches)" with the name, + # ref (commit), and arches of the component. for i in range(new_module_build_component.batch - 1): # This is the first batch which we want to skip since it will always # contain only the module-build-macros RPM and it gets built every time @@ -412,37 +412,34 @@ def get_reusable_component( continue new_module_build_components.append({ - (value.package, value.ref) + (value.package, value.ref, + tuple(sorted(mmd.get_rpm_component(value.package).get_arches()))) for value in new_component_builds if value.batch == i + 1 }) previous_module_build_components.append({ - (value.package, value.ref) + (value.package, value.ref, + tuple(sorted(old_mmd.get_rpm_component(value.package).get_arches()))) for value in prev_component_builds if value.batch == i + 1 }) - # If the previous batches don't have the same ordering and hashes, then the + # If the previous batches don't have the same ordering, hashes, and arches, then the # component can't be reused if previous_module_build_components != new_module_build_components: message = ("Cannot reuse the component because a component in a previous" - " batch has been rebuilt") + " batch has been added, removed, or rebuilt") new_module_build_component.log_message(db_session, message) return None - for pkg_name in mmd.get_rpm_component_names(): - pkg = mmd.get_rpm_component(pkg_name) - if pkg_name not in old_mmd.get_rpm_component_names(): - message = ("Cannot reuse the component because a component was added or " - "removed since the compatible module") - new_module_build_component.log_message(db_session, message) - return None - if set(pkg.get_arches()) != set(old_mmd.get_rpm_component(pkg_name).get_arches()): - message = ("Cannot reuse the component because the architectures for the package {}" - " have changed since the compatible module build").format(pkg_name) - new_module_build_component.log_message(db_session, message) - return None + # check that arches have not changed + pkg = mmd.get_rpm_component(component_name) + if set(pkg.get_arches()) != set(old_mmd.get_rpm_component(component_name).get_arches()): + message = ("Cannot reuse the component because its architectures" + " have changed since the compatible module build").format(component_name) + new_module_build_component.log_message(db_session, message) + return None reusable_component = db_session.query(models.ComponentBuild).filter_by( package=component_name, module_id=previous_module_build.id).one() diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 9abc6fa4..0c85e069 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -163,6 +163,86 @@ class TestUtilsComponentReuse: tangerine = get_reusable_component(second_module_build, "tangerine") assert bool(tangerine is None) != bool(set_current_arch == set_database_arch) + @pytest.mark.parametrize( + "reuse_component", + ["perl-Tangerine", "perl-List-Compare", "tangerine"]) + @pytest.mark.parametrize( + "changed_component", + ["perl-Tangerine", "perl-List-Compare", "tangerine"]) + def test_get_reusable_component_different_batch( + self, changed_component, reuse_component, db_session + ): + """ + Test that we get the correct reuse behavior for the changed-and-after strategy. Changes + to earlier batches should prevent reuse, but changes to later batches should not. + For context, see https://pagure.io/fm-orchestrator/issue/1298 + """ + + if changed_component == reuse_component: + # we're only testing the cases where these are different + # this case is already covered by test_get_reusable_component_different_component + return + + second_module_build = models.ModuleBuild.get_by_id(db_session, 3) + + # update batch for changed component + changed_component = models.ComponentBuild.from_component_name( + db_session, changed_component, second_module_build.id) + orig_batch = changed_component.batch + changed_component.batch = orig_batch + 1 + db_session.commit() + + reuse_component = models.ComponentBuild.from_component_name( + db_session, reuse_component, second_module_build.id) + + reuse_result = module_build_service.utils.get_reusable_component( + db_session, second_module_build, reuse_component.package) + # Component reuse should only be blocked when an earlier batch has been changed. + # In this case, orig_batch is the earliest batch that has been changed (the changed + # component has been removed from it and added to the following one). + assert bool(reuse_result is None) == bool(reuse_component.batch > orig_batch) + + @pytest.mark.parametrize( + "reuse_component", + ["perl-Tangerine", "perl-List-Compare", "tangerine"]) + @pytest.mark.parametrize( + "changed_component", + ["perl-Tangerine", "perl-List-Compare", "tangerine"]) + def test_get_reusable_component_different_arch_in_batch( + self, changed_component, reuse_component, db_session + ): + """ + Test that we get the correct reuse behavior for the changed-and-after strategy. Changes + to the architectures in earlier batches should prevent reuse, but such changes to later + batches should not. + For context, see https://pagure.io/fm-orchestrator/issue/1298 + """ + if changed_component == reuse_component: + # we're only testing the cases where these are different + # this case is already covered by test_get_reusable_component_different_arches + return + + second_module_build = models.ModuleBuild.get_by_id(db_session, 3) + + # update arch for changed component + mmd = second_module_build.mmd() + component = mmd.get_rpm_component(changed_component) + component.reset_arches() + component.add_restricted_arch("i686") + second_module_build.modulemd = mmd_to_str(mmd) + db_session.commit() + + changed_component = models.ComponentBuild.from_component_name( + db_session, changed_component, second_module_build.id) + reuse_component = models.ComponentBuild.from_component_name( + db_session, reuse_component, second_module_build.id) + + reuse_result = module_build_service.utils.get_reusable_component( + db_session, second_module_build, reuse_component.package) + # Changing the arch of a component should prevent reuse only when the changed component + # is in a batch earlier than the component being considered for reuse. + assert bool(reuse_result is None) == bool(reuse_component.batch > changed_component.batch) + @pytest.mark.parametrize("rebuild_strategy", models.ModuleBuild.rebuild_strategies.keys()) def test_get_reusable_component_different_buildrequires_stream(self, rebuild_strategy): first_module_build = models.ModuleBuild.get_by_id(db_session, 2)