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.

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)
 

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()

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,