From c828f604441bf0d4092e1c5cbec5702c28b0ca0b Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Apr 13 2017 06:36:04 +0000 Subject: Do not replace branch names in mmd.components.rpms.ref by the commit hash, use our private mmd.xmd.mbs namespace to store that info. --- diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 5bae815..cbb6134 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -488,11 +488,11 @@ def _scm_get_latest(pkg): # we want to pull from, we need to resolve that f25 branch # to the specific commit available at the time of # submission (now). - pkg.ref = module_build_service.scm.SCM( + pkgref = module_build_service.scm.SCM( pkg.repository).get_latest(branch=pkg.ref) except Exception as e: - return "Failed to get the latest commit for %s#%s" % (pkg.repository, pkg.ref) - return None + return True, "Failed to get the latest commit for %s#%s" % (pkg.repository, pkg.ref) + return False, (pkg.name, pkgref) def format_mmd(mmd, scmurl): """ @@ -552,6 +552,8 @@ def format_mmd(mmd, scmurl): mmd.xmd['mbs']['buildrequires'] = {} if mmd.components: + if 'rpms' not in mmd.xmd['mbs']: + mmd.xmd['mbs']['rpms'] = {} # Add missing data in RPM components for pkgname, pkg in mmd.components.rpms.items(): if pkg.repository and not conf.rpms_allow_repository: @@ -577,17 +579,32 @@ def format_mmd(mmd, scmurl): mod.ref = 'master' # Check that SCM URL is valid and replace potential branches in - # pkg.ref by real SCM hash. + # pkg.ref by real SCM hash and store the result to our private xmd + # place in modulemd. pool = ThreadPool(20) - err_msgs = pool.map(_scm_get_latest, mmd.components.rpms.values()) - # TODO: only the first error message is raised, perhaps concatenate - # the messages together? - for err_msg in err_msgs: - if err_msg: - raise UnprocessableEntity(err_msg) + pkgrefs = pool.map(_scm_get_latest, mmd.components.rpms.values()) + err_msg = "" + for is_error, pkgref in pkgrefs: + if is_error: + err_msg += pkgref + "\n" + else: + mmd.xmd['mbs']['rpms'][pkgref[0]] = {'ref': pkgref[1]} + if err_msg: + raise UnprocessableEntity(err_msg) + +def merge_included_mmd(main_mmd, mmd): + """ + Merges two modulemds. This merges only metadata which are needed in + the `main_mmd` when it includes another module defined by `mmd` + """ + if 'rpms' in mmd.xmd['mbs']: + if 'rpms' not in main_mmd.xmd['mbs']: + main_mmd.xmd['mbs']['rpms'] = mmd.xmd['mbs']['rpms'] + else: + main_mmd.xmd['mbs']['rpms'].update(mmd.xmd['mbs']['rpms']) def record_component_builds(mmd, module, initial_batch = 1, - previous_buildorder = None): + previous_buildorder = None, main_mmd = None): import koji # Placed here to avoid py2/py3 conflicts... # Format the modulemd by putting in defaults and replacing streams that @@ -600,16 +617,15 @@ def record_component_builds(mmd, module, initial_batch = 1, db.session.commit() raise - # List of (pkg_name, git_url) tuples to be used to check - # the availability of git URLs in parallel later. - full_urls = [] + # When main_mmd is set, merge the metadata from this mmd to main_mmd, + # otherwise our current mmd is main_mmd. + if main_mmd: + merge_included_mmd(main_mmd, mmd) + else: + main_mmd = mmd # If the modulemd yaml specifies components, then submit them for build if mmd.components: - for pkgname, pkg in mmd.components.rpms.items(): - full_url = "%s?#%s" % (pkg.repository, pkg.ref) - full_urls.append((pkgname, full_url)) - components = mmd.components.all components.sort(key=lambda x: x.buildorder) @@ -632,12 +648,13 @@ def record_component_builds(mmd, module, initial_batch = 1, full_url = pkg.repository + "?#" + pkg.ref # It is OK to whitelist all URLs here, because the validity # of every URL have been already checked in format_mmd(...). - mmd = _fetch_mmd(full_url, whitelist_url=True)[0] - batch = record_component_builds(mmd, module, batch, - previous_buildorder) + included_mmd = _fetch_mmd(full_url, whitelist_url=True)[0] + batch = record_component_builds(included_mmd, module, batch, + previous_buildorder, main_mmd) continue - full_url = pkg.repository + "?#" + pkg.ref + pkgref = mmd.xmd['mbs']['rpms'][pkg.name]['ref'] + full_url = pkg.repository + "?#" + pkgref existing_build = models.ComponentBuild.query.filter_by( module_id=module.id, package=pkg.name).first() @@ -654,7 +671,7 @@ def record_component_builds(mmd, module, initial_batch = 1, format="rpms", scmurl=full_url, batch=batch, - ref=pkg.ref + ref=pkgref ) db.session.add(build) diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 1b6ab00..7b354ce 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -85,6 +85,7 @@ class TestUtils(unittest.TestCase): '4ceea43add2366d8b8c5a622a2fb563b625b9abf', 'fbed359411a1baa08d4a88e0d12d426fbf8f602c', '76f9d8c8e87eed0aab91034b01d3d5ff6bd5b4cb'] + original_refs = ["f25"] mocked_scm.return_value.get_latest.side_effect = hashes_returned mmd = modulemd.ModuleMetadata() with open(path.join(BASE_DIR, '..', 'staged_data', 'testmodule.yaml')) \ @@ -95,9 +96,14 @@ class TestUtils(unittest.TestCase): '?#620ec77321b2ea7b0d67d82992dda3e1d67055b4') module_build_service.utils.format_mmd(mmd, scmurl) - # Make sure all the commit hashes were properly set on the RPMs + # Make sure all the commit hashes were properly set in xmd section + # of modulemd. + xmd_pkg_refs = [pkg['ref'] for pkg in mmd.xmd['mbs']['rpms'].values()] + self.assertEqual(set(xmd_pkg_refs), set(hashes_returned)) + + # Make sure that original refs are not changed. mmd_pkg_refs = [pkg.ref for pkg in mmd.components.rpms.values()] - self.assertEqual(set(mmd_pkg_refs), set(hashes_returned)) + self.assertEqual(set(mmd_pkg_refs), set(original_refs)) self.assertEqual(mmd.buildrequires, {'base-runtime': 'master'}) xmd = { @@ -108,6 +114,9 @@ class TestUtils(unittest.TestCase): 'ref': '464026abf9cbe10fac1d800972e3229ac4d01975', 'stream': 'master', 'version': '20170404161234'}}, + 'rpms': {'perl-List-Compare': {'ref': '76f9d8c8e87eed0aab91034b01d3d5ff6bd5b4cb'}, + 'perl-Tangerine': {'ref': '4ceea43add2366d8b8c5a622a2fb563b625b9abf'}, + 'tangerine': {'ref': 'fbed359411a1baa08d4a88e0d12d426fbf8f602c'}}, 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/testmodule' '.git?#620ec77321b2ea7b0d67d82992dda3e1d67055b4', } @@ -138,6 +147,9 @@ class TestUtils(unittest.TestCase): 'ref': '464026abf9cbe10fac1d800972e3229ac4d01975', 'stream': 'master', 'version': '20170404161234'}}, + 'rpms': {'perl-List-Compare': {'ref': '76f9d8c8e87eed0aab91034b01d3d5ff6bd5b4cb'}, + 'perl-Tangerine': {'ref': '4ceea43add2366d8b8c5a622a2fb563b625b9abf'}, + 'tangerine': {'ref': 'fbed359411a1baa08d4a88e0d12d426fbf8f602c'}}, 'scmurl': None, } } @@ -358,13 +370,15 @@ class TestUtils(unittest.TestCase): self.assertTrue(str(cm.exception).endswith(' No value provided.')) + @vcr.use_cassette( + path.join(CASSETTES_DIR, 'tests.test_utils.TestUtils.test_format_mmd')) @patch('module_build_service.scm.SCM') def test_resubmit(self, mocked_scm): """ Tests that the module resubmit reintializes the module state and component states properly. """ - MockedSCM(mocked_scm, 'testmodule', 'testmodule-bootstrap.yaml', + MockedSCM(mocked_scm, 'testmodule', 'testmodule.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') with app.app_context(): test_reuse_component_init_data() diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index dd0330b..73f0d62 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -32,6 +32,7 @@ from mock import patch, Mock, PropertyMock from shutil import copyfile from os import path, mkdir from os.path import dirname +import hashlib from tests import app, init_data from module_build_service.errors import UnprocessableEntity @@ -96,7 +97,7 @@ class MockedSCM(object): return scm_dir def get_latest(self, branch='master'): - return branch + return hashlib.sha1(branch).hexdigest()[:10] class TestViews(unittest.TestCase): @@ -529,6 +530,17 @@ class TestViews(unittest.TestCase): self.assertEquals(batches['tangerine'], 3) self.assertEquals(batches["file"], 4) + build = ModuleBuild.query.filter(ModuleBuild.id == data['id']).one() + mmd = build.mmd() + + # Test that RPMs are properly merged in case of included modules in mmd. + xmd_rpms = {'ed': {'ref': '40bd001563'}, + 'perl-List-Compare': {'ref': '2ee8474e44'}, + 'tangerine': {'ref': '2ee8474e44'}, + 'file': {'ref': 'a2740663f8'}, + 'perl-Tangerine': {'ref': '2ee8474e44'}} + self.assertEqual(mmd.xmd['mbs']['rpms'], xmd_rpms) + @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') def test_submit_build_includedmodule_custom_repo_not_allowed(self,