#971 Use the libmodulemd API properly when setting values on existing objects
Merged a year ago by mprahl. Opened a year ago by mprahl.

@@ -100,11 +100,13 @@ 

          else:

              xmd['mbs']['commit'] = scm.get_latest()

  

-     if mmd.get_rpm_components() or mmd.get_module_components():

+     rpm_components = mmd.get_rpm_components()

+     module_components = mmd.get_module_components()

+     if rpm_components or module_components:

          if 'rpms' not in xmd['mbs']:

              xmd['mbs']['rpms'] = {}

          # Add missing data in RPM components

-         for pkgname, pkg in mmd.get_rpm_components().items():

+         for pkgname, pkg in rpm_components.items():

              if pkg.get_repository() and not conf.rpms_allow_repository:

                  raise Forbidden(

                      "Custom component repositories aren't allowed.  "

@@ -119,9 +121,10 @@ 

                  pkg.set_cache(conf.rpms_default_cache + pkgname)

              if not pkg.get_ref():

                  pkg.set_ref('master')

+         mmd.set_rpm_components(rpm_components)

  

          # Add missing data in included modules components

-         for modname, mod in mmd.get_module_components().items():

+         for modname, mod in module_components.items():

              if mod.get_repository() and not conf.modules_allow_repository:

                  raise Forbidden(

                      "Custom module repositories aren't allowed.  "

@@ -130,11 +133,12 @@ 

                  mod.set_repository(conf.modules_default_repository + modname)

              if not mod.get_ref():

                  mod.set_ref('master')

+         mmd.set_module_components(module_components)

  

          # Check that SCM URL is valid and replace potential branches in pkg refs

          # by real SCM hash and store the result to our private xmd place in modulemd.

          pool = ThreadPool(20)

-         pkg_dicts = pool.map(_scm_get_latest, mmd.get_rpm_components().values())

+         pkg_dicts = pool.map(_scm_get_latest, rpm_components.values())

          err_msg = ""

          for pkg_dict in pkg_dicts:

              if pkg_dict["error"]:

file modified
+3 -1

@@ -291,7 +291,9 @@ 

          current_dir, 'staged_data', 'formatted_testmodule.yaml')

      mmd = Modulemd.Module().new_from_file(formatted_testmodule_yml_path)

      mmd.upgrade()

-     mmd.get_rpm_components()['tangerine'].set_buildorder(0)

+     components = mmd.get_rpm_components()

+     components['tangerine'].set_buildorder(0)

+     mmd.set_rpm_components(components)

  

      build_one = module_build_service.models.ModuleBuild()

      build_one.name = 'testmodule'

@@ -90,8 +90,9 @@ 

          second_module_build = models.ModuleBuild.query.filter_by(id=3).one()

          if changed_component:

              mmd = second_module_build.mmd()

-             mmd.get_rpm_components()['tangerine'].set_ref(

-                 '00ea1da4192a2030f9ae023de3b3143ed647bbab')

+             components = mmd.get_rpm_components()

+             components['tangerine'].set_ref('00ea1da4192a2030f9ae023de3b3143ed647bbab')

+             mmd.set_rpm_components(components)

              second_module_build.modulemd = mmd.dumps()

              second_module_changed_component = models.ComponentBuild.query.filter_by(

                  package=changed_component, module_id=3).one()

@@ -204,7 +205,9 @@ 

          mmd = second_module_build.mmd()

          br_list = Modulemd.SimpleSet()

          br_list.add('master')

-         mmd.get_dependencies()[0].set_buildrequires({'some_module': br_list})

+         deps = mmd.get_dependencies()

+         deps[0].set_buildrequires({'some_module': br_list})

+         mmd.set_dependencies(deps)

          xmd = glib.from_variant_dict(mmd.get_xmd())

          xmd['mbs']['buildrequires'] = {

              'some_module': {

@@ -306,9 +309,11 @@ 

          mmd = Modulemd.Module().new_from_file(

              path.join(BASE_DIR, '..', 'staged_data', 'testmodule.yaml'))

          mmd.upgrade()

+         components = mmd.get_rpm_components()

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

-         mmd.get_rpm_components()['perl-Tangerine'].set_ref('f28')

-         mmd.get_rpm_components()['tangerine'].set_ref('f27')

+         components['perl-Tangerine'].set_ref('f28')

+         components['tangerine'].set_ref('f27')

+         mmd.set_rpm_components(components)

          module_build_service.utils.format_mmd(mmd, scmurl)

  

          # Make sure that original refs are not changed.

MBS has been using the libmodulemd API incorrectly by assuming that methods like get_rpm_components return the actual object in memory and not a copy. This has been true but wasn't something sgallagh intended. We found this out after he had me test his new version of libmodulemd. This PR removes those assumptions.

@sgallagh could you please review?

rebased onto 9b09f1fdf00a2265e00760150243909d2ab7277e

a year ago

rebased onto 4c59d8c083de91a78aaca28734af91083a0b15d7

a year ago

rebased onto 8173040

a year ago

Pull-Request has been merged by mprahl

a year ago

@sgallagh since you decided to leave libmodulemd exposing the actual value in memory and not a copy, and this PR didn't actually fix all those instances where we weren't assuming it was a copy, I decided to revert this. This will reduce unnecessary risk in the next release and deployment.

@mprahl Makes sense to me. Thanks for the update.