#1568 Skip git ref checks for rpm components with srpm overrides
Merged 4 years ago by mprahl. Opened 4 years ago by merlinm.

@@ -31,7 +31,7 @@

      add_default_modules, handle_collisions_with_base_module_rpms)

  from module_build_service.scheduler.greenwave import greenwave

  from module_build_service.scheduler.reuse import attempt_to_reuse_all_components

- from module_build_service.scheduler.submit import format_mmd

+ from module_build_service.scheduler.submit import format_mmd, get_module_srpm_overrides

  from module_build_service.scheduler.ursine import handle_stream_collision_modules


@@ -183,9 +183,11 @@

          arches = [arch.name for arch in build.arches]

          defaults_added = add_default_modules(mmd)


+         # Get map of packages that have SRPM overrides

+         srpm_overrides = get_module_srpm_overrides(build)

          # Format the modulemd by putting in defaults and replacing streams that

          # are branches with commit hashes

-         format_mmd(mmd, build.scmurl, build, db_session)

+         format_mmd(mmd, build.scmurl, build, db_session, srpm_overrides)

          record_component_builds(mmd, build)


          # The ursine.handle_stream_collision_modules is Koji specific.

@@ -182,7 +182,7 @@

      return {"pkg_name": pkg.get_name(), "pkg_ref": pkgref, "error": None}



- def format_mmd(mmd, scmurl, module=None, db_session=None):

+ def format_mmd(mmd, scmurl, module=None, db_session=None, srpm_overrides=None):


      Prepares the modulemd for the MBS. This does things such as replacing the

      branches of components with commit hashes and adding metadata in the xmd
@@ -192,7 +192,11 @@

      :param module: When specified together with `session`, the time_modified

          of a module is updated regularly in case this method takes lot of time.

      :param db_session: Database session to update the `module`.

+     :param dict srpm_overrides: Mapping of package names to SRPM links for all

+         component packages which have custom SRPM overrides specified.


+     srpm_overrides = srpm_overrides or {}


      xmd = mmd.get_xmd()

      if "mbs" not in xmd:

          xmd["mbs"] = {}
@@ -263,12 +267,18 @@

          pool = ThreadPool(20)


              # Filter out the packages which we have already resolved in possible

-             # previous runs of this method (can be caused by module build resubmition).

-             pkgs_to_resolve = [

-                 mmd.get_rpm_component(name)

-                 for name in mmd.get_rpm_component_names()

-                 if name not in xmd["mbs"]["rpms"]

-             ]

+             # previous runs of this method (can be caused by module build resubmition)

+             # or which have custom SRPMs and shouldn't be resolved.

+             pkgs_to_resolve = []

+             for name in mmd.get_rpm_component_names():

+                 if name not in xmd["mbs"]["rpms"]:

+                     if name in srpm_overrides:

+                         # If this package has a custom SRPM, store an empty

+                         # ref entry so no further verification takes place.

+                         xmd["mbs"]["rpms"][name] = {"ref": None}

+                     else:

+                         pkgs_to_resolve.append(mmd.get_rpm_component(name))


              async_result = pool.map_async(_scm_get_latest, pkgs_to_resolve)


              # For modules with lot of components, the _scm_get_latest can take a lot of time.
@@ -436,7 +446,7 @@

              # It is OK to whitelist all URLs here, because the validity

              # of every URL have been already checked in format_mmd(...).

              included_mmd = fetch_mmd(full_url, whitelist_url=True)[0]

-             format_mmd(included_mmd, module.scmurl, module, db_session)

+             format_mmd(included_mmd, module.scmurl, module, db_session, srpm_overrides)

              batch = record_component_builds(

                  included_mmd, module, batch, previous_buildorder, main_mmd)


@@ -97,14 +97,17 @@




+     @pytest.mark.parametrize(

+         "srpm_overrides",

+         [

+             {"perl-List-Compare": "/path/to/perl-List-Compare.src.rpm"},

+             None,

+         ],

+     )


-     def test_format_mmd(self, mocked_scm, scmurl):

+     def test_format_mmd(self, mocked_scm, srpm_overrides, scmurl):

          mocked_scm.return_value.commit = "620ec77321b2ea7b0d67d82992dda3e1d67055b4"

          # For all the RPMs in testmodule, get_latest is called

-         mocked_scm.return_value.get_latest.side_effect = [

-             "4ceea43add2366d8b8c5a622a2fb563b625b9abf",

-             "fbed359411a1baa08d4a88e0d12d426fbf8f602c",

-         ]

          hashes_returned = {

              "master": "fbed359411a1baa08d4a88e0d12d426fbf8f602c",

              "f28": "4ceea43add2366d8b8c5a622a2fb563b625b9abf",
@@ -112,24 +115,33 @@



          def mocked_get_latest(ref="master"):

-             return hashes_returned[ref]

+             if ref in hashes_returned:

+                 return hashes_returned[ref]

+             raise RuntimeError("ref %s not found." % ref)


          mocked_scm.return_value.get_latest = mocked_get_latest

          mmd = load_mmd(read_staged_data("testmodule"))

          # Modify the component branches so we can identify them later on



-         format_mmd(mmd, scmurl)

+         if srpm_overrides:

+             # Set a bogus ref that will raise an exception if not properly ignored.

+             mmd.get_rpm_component("perl-List-Compare").set_ref("bogus")

+         format_mmd(mmd, scmurl, srpm_overrides=srpm_overrides)


          # Make sure that original refs are not changed.

          mmd_pkg_refs = [


              for pkg_name in mmd.get_rpm_component_names()


-         assert set(mmd_pkg_refs) == set(hashes_returned.keys())

+         if srpm_overrides:

+             assert set(mmd_pkg_refs) == {'f27', 'f28', 'bogus'}

+         else:

+             assert set(mmd_pkg_refs) == {'f27', 'f28', 'master'}

          deps = mmd.get_dependencies()[0]

          assert deps.get_buildtime_modules() == ["platform"]

          assert deps.get_buildtime_streams("platform") == ["f28"]

+         match_anything = type('eq_any', (), {"__eq__": lambda left, right: True})()

          xmd = {

              "mbs": {

                  "commit": "",
@@ -144,6 +156,8 @@

          if scmurl:

              xmd["mbs"]["commit"] = "620ec77321b2ea7b0d67d82992dda3e1d67055b4"

              xmd["mbs"]["scmurl"] = scmurl

+         if srpm_overrides:

+             xmd["mbs"]["rpms"]["perl-List-Compare"]["ref"] = match_anything

          mmd_xmd = mmd.get_xmd()

          assert mmd_xmd == xmd


When attempting to submit scratch module builds that include custom SRPMs, I have occasionally run into the situation where my modulemd file includes a new package that is not yet published to a distgit repo, or it contains a git 'ref' to a commit in an existing repo that doesn't yet (or no longer) exist. In these cases, the scratch module build promptly fails with a error such as the following:

Reason: Failed to get the latest commit for git+https://src.fedoraproject.org/rpms/cool-new-package#master

For a non-scratch build, it is critical that MBS validates all of the git repos and refs. For a scratch build, however, as long as a corresponding SRPM has been provided, such checks should be skipped. This PR implements the skipping of such checks for rpm components with srpm overrides.

I also started out this PR with a commit that removes a bunch of unused code in tests/test_utils/test_utils.py that was causing me confusion while updating test_format_mmd() to include tests for the revised ref checking.

@mikem and @breilly could you please review this?

@breilly and @mikem, could you please review this?

@breilly and @mikem, could you please review this?

@merlinm could you please rebase this?

rebased onto df8e5879f810a5b2398cfc1ddbe9ea5b2813f2d7

4 years ago

@mprahl Rebased. (Things sure moved around quite a bit since I initially created this PR! )

I'm confused by the Jenkins failure only with Python 3 & SQLite. It doesn't seem to be related to my change...

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

pretty please pagure-ci rebuild

4 years ago

rebased onto ab90c7b

4 years ago

Pull-Request has been merged by mprahl

4 years ago