#1170 Allow overriding the base module marking used in RPM disttags and don't allow dashes in the marking
Merged 4 months ago by mprahl. Opened 4 months ago by mprahl.

file modified
+1 -1

@@ -15,7 +15,7 @@ 

  

  Then run the tests with::

  

-     $ sudo docker run -t -v $PWD:/src:Z mbs/test

+     $ sudo docker run -t --rm -v $PWD:/src:Z mbs/test

  

  

  Development

@@ -240,13 +240,36 @@ 

                      .format(module_build.id))

          buildrequires = None

  

-     base_module_stream = ''

+     # Determine which base module is buildrequired and its marking in the disttag

+     base_module_marking = ''

+     # If the buildrequires are recorded in the xmd then we can try to find the base module that

+     # is buildrequired

      if buildrequires:

+         # Looping through all the base modules in conf.base_module_names instead of looping through

+         # all the buildrequires guarantees the order in conf.base_module_names is preserved for

+         # which base module is used as the marking

          for base_module in conf.base_module_names:

-             base_module_stream = buildrequires.get(base_module, {}).get('stream', '')

-             if base_module_stream:

-                 base_module_stream += '+'

-                 break

+             bm_in_xmd = buildrequires.get(base_module)

+ 

+             if not bm_in_xmd:

+                 continue

+ 

+             with models.make_session(conf) as session:

+                 base_module_obj = models.ModuleBuild.get_build_from_nsvc(

+                     session, base_module, bm_in_xmd['stream'], bm_in_xmd['version'],

+                     bm_in_xmd['context'])

+             if not base_module_obj:

+                 continue

+ 

+             # Default to using the base module's stream, but if the base module has disttag_marking

+             # set in the xmd, use that instead

+             try:

+                 marking = base_module_obj.mmd().get_xmd()['mbs']['disttag_marking']

+             # We must check for a KeyError because a Variant object doesn't support the `get` method

+             except KeyError:

+                 marking = base_module_obj.stream

+             base_module_marking = marking + '+'

+             break

          else:

              log.warning('Module build {0} does not buildrequire a base module ({1})'

                          .format(module_build.id, ' or '.join(conf.base_module_names)))

@@ -254,9 +277,9 @@ 

      # use alternate prefix for scratch module build components so they can be identified

      prefix = ('scrmod+' if module_build.scratch else conf.default_dist_tag_prefix)

  

-     return '{prefix}{base_module_stream}{index}+{dist_hash}'.format(

+     return '{prefix}{base_module_marking}{index}+{dist_hash}'.format(

          prefix=prefix,

-         base_module_stream=base_module_stream,

+         base_module_marking=base_module_marking,

          index=index,

          dist_hash=dist_hash,

      )

@@ -312,6 +335,23 @@ 

      version = str(mmd.get_version())

      context = mmd.get_context()

  

+     try:

+         disttag_marking = mmd.get_xmd()["mbs"]["disttag_marking"]

+     except (ValueError, KeyError):

+         disttag_marking = None

+ 

+     # If it is a base module, then make sure the value that will be used in the RPM disttags

+     # doesn't contain a dash since a dash isn't allowed in the release field of the NVR

+     if name in conf.base_module_names:

+         if disttag_marking and "-" in disttag_marking:

+             msg = "The disttag_marking cannot contain a dash"

+             log.error(msg)

+             raise UnprocessableEntity(msg)

+         elif not disttag_marking and "-" in stream:

+             msg = "The stream cannot contain a dash unless disttag_marking is set"

+             log.error(msg)

+             raise UnprocessableEntity(msg)

+ 

      # Log messages collected during import

      msgs = []

  

@@ -321,6 +321,32 @@ 

              assert mmd_context == models.DEFAULT_MODULE_CONTEXT

              assert build.context == models.DEFAULT_MODULE_CONTEXT

  

+     @pytest.mark.parametrize('stream, disttag_marking, error_msg', (

+         ('f28', None, None),

+         ('f28', 'fedora28', None),

+         ('f-28', 'f28', None),

+         ('f-28', None, 'The stream cannot contain a dash unless disttag_marking is set'),

+         ('f28', 'f-28', 'The disttag_marking cannot contain a dash'),

+         ('f-28', 'fedora-28', 'The disttag_marking cannot contain a dash')

+     ))

+     def test_import_mmd_base_module(self, stream, disttag_marking, error_msg):

+         clean_database(add_platform_module=False)

+         mmd = Modulemd.Module().new_from_file(

+             path.join(BASE_DIR, '..', 'staged_data', 'platform.yaml'))

+         mmd.upgrade()

+         mmd.set_stream(stream)

+ 

+         if disttag_marking:

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

+             xmd['mbs']['disttag_marking'] = disttag_marking

+             mmd.set_xmd(glib.dict_values(xmd))

+ 

+         if error_msg:

+             with pytest.raises(UnprocessableEntity, match=error_msg):

+                 module_build_service.utils.import_mmd(db.session, mmd)

+         else:

+             module_build_service.utils.import_mmd(db.session, mmd)

+ 

      def test_get_rpm_release_mse(self):

          init_data(contexts=True)

          build_one = models.ModuleBuild.query.get(2)

@@ -336,6 +362,23 @@ 

          release = module_build_service.utils.get_rpm_release(build_one)

          assert release == 'module+f28+2+814cfa39'

  

+     def test_get_rpm_release_platform_stream_override(self):

+         scheduler_init_data(1)

+ 

+         # Set the disttag_marking override on the platform

+         platform = models.ModuleBuild.query.filter_by(name='platform', stream='f28').first()

+         platform_mmd = platform.mmd()

+         platform_xmd = glib.from_variant_dict(platform_mmd.get_xmd())

+         platform_xmd['mbs']['disttag_marking'] = 'fedora28'

+         platform_mmd.set_xmd(glib.dict_values(platform_xmd))

+         platform.modulemd = to_text_type(platform_mmd.dumps())

+         db.session.add(platform)

+         db.session.commit()

+ 

+         build_one = models.ModuleBuild.query.get(2)

+         release = module_build_service.utils.get_rpm_release(build_one)

+         assert release == 'module+fedora28+2+814cfa39'

+ 

      def test_get_rpm_release_mse_scratch(self):

          init_data(contexts=True, scratch=True)

          build_one = models.ModuleBuild.query.get(2)

MBS uses the base module's stream that was buildrequired by the module in the RPM disttags for that module build. The stream name may not be ideal for all situations, so now this is customizable by setting the xmd['mbs']['disttag_marking'] in the base module's modulemd.

Additionally, this code ensures that the marking that will be used does not contain a dash since it is not allowed in the release field of an NVR.

3 new commits added

  • Don't allow a dash in the value that will be used for an RPM disttag
  • Add the ability to override the base module marking used in the RPM disttags
  • Instruct the user to delete the container after the test is run
4 months ago

Looks good code-wise. I was only thinking about moving the documentation of those special XMD keys from https://docs.google.com/document/d/1FHj9Z1xTNKf0x3UtskS-ubatV4i4bFAs5XkPt7_ESYY/edit#heading=h.s87r5qagdp9r to some file in DOCS directory and also mentioning that disttag_marking there.

It can be done as separate PR, but maybe it would also make sense to just add that documentation in this PR if you don't mind :).

@jkaluza I'll merge this as is and submit a new PR.

Pull-Request has been merged by mprahl

4 months ago