pungi

Clone

#891 Include module defaults in the repodata
Merged 6 years ago by lsedlar. Opened 6 years ago by psabata.
psabata/pungi module-defaults  into  master

file modified
+5 
@@ -21,6 +21,7 @@ 
 

      # GENERAL SETTINGS
 

      comps_file = "comps-f23.xml"
 

      variants_file = "variants-f23.xml"
 

+     module_defaults_dir = "module_defaults"
 

  

      # KOJI
 

      koji_profile = "koji"
 
@@ -135,6 +136,10 @@ 
 

      (:ref:`scm_dict <scm_support>` or *str*) -- reference to variants XML file
 

      that defines release variants and architectures
 

  

+ **module_defaults_dir** [optional]

shouldn't this configuration option be defined in 'pungi/checks.py' as well?

@onosek, yes, that would make sense. Thanks for pointing it out.

 
+     (:ref:`scm_dict <scm_support>` or *str*) -- reference the module defaults
 

+     directory containing modulemd-defaults YAML documents
 

+ 

  **failable_deliverables** [optional]
 

      (*list*) -- list which deliverables on which variant and architecture can
 

      fail and not abort the whole compose. This only applies to ``buildinstall``

file modified
+1 
@@ -678,6 +678,7 @@ 
 

                  "type": "boolean",
 

                  "default": True
 

              },
 

+             "module_defaults_dir": {"$ref": "#/definitions/str_or_scm_dict"},
 

  

              "pkgset_repos": {
 

                  "type": "object",

file modified
+4 
@@ -194,6 +194,10 @@ 
 

          return bool(self.conf.get("comps_file", False))
 

  

      @property
 

+     def has_module_defaults(self):
 

+         return bool(self.conf.get("module_defaults_dir", False))
 

+ 

+     @property
 

      def config_dir(self):
 

          return os.path.dirname(self.conf._open_file or "")

file modified
+7 -1 
@@ -218,9 +218,15 @@ 
 

                      raise AttributeError("module_metadata parameter was not passed and it is needed for module processing")
 

              modules.append(repo_mmd)
 

  

+         module_names = set([ x.get_name() for x in modules ])

I understand 'x' it is used only here, but 'item' or another word would be more intelligible.

@onosek, I've always thought x was rather common in list comprehensions but I can replace it with item if you prefer.

 
+         for mmddeffile in glob.glob(os.path.join(compose.config_dir, "module_defaults", "*.yaml")):
 

+             for mmddef in Modulemd.objects_from_file(mmddeffile):
 

+                 if isinstance(mmddef, Modulemd.Defaults) and mmddef.peek_module_name() in module_names:

'gi.repository.Modulemd' object has no attribute 'Defaults'
do we use the same library?

@onosek, as the first commit states, you need libmodulemd-1.2.0 or later for this to work.

 
+                     modules.append(mmddef)
 

+ 

          with temp_dir() as tmp_dir:
 

              modules_path = os.path.join(tmp_dir, "modules.yaml")
 

-             Modulemd.Module.dump_all(modules, modules_path)
 

+             Modulemd.dump(modules, modules_path)

'gi.repository.Modulemd' object has no attribute 'dump'
Question: what is the purpose of this change?

@onosek, this is a new method in 1.2.0 which can dump multiple document types at once -- in this case a mixture of modulemd and modulemd-defaults documents.

 
  

              cmd = repo.get_modifyrepo_cmd(os.path.join(repo_dir, "repodata"),
 

                                            modules_path, mdtype="modules",

file modified
+20 -1 
@@ -23,7 +23,7 @@ 
 

  from pungi.phases.gather import write_prepopulate_file
 

  from pungi.wrappers.createrepo import CreaterepoWrapper
 

  from pungi.wrappers.comps import CompsWrapper
 

- from pungi.wrappers.scm import get_file_from_scm
 

+ from pungi.wrappers.scm import get_file_from_scm, get_dir_from_scm
 

  

  

  class InitPhase(PhaseBase):
 
@@ -50,6 +50,10 @@ 
 

  

          # download variants.xml / product.xml?
 

  

+         # download module defaults
 

+         if self.compose.has_module_defaults:
 

+             write_module_defaults(self.compose)
 

+ 

          # write prepopulate file
 

          write_prepopulate_file(self.compose)
 

  
@@ -142,3 +146,18 @@ 
 

                                        checksum=createrepo_checksum)
 

          run(cmd, logfile=compose.paths.log.log_file(arch, "comps_repo"), show_cmd=True)
 

          compose.log_info("[DONE ] %s" % msg)
 

+ 

+ def write_module_defaults(compose):
 

+     scm_dict = compose.conf["module_defaults_dir"]
 

+     if isinstance(scm_dict, dict):
 

+         if scm_dict["scm"] == "file":
 

+             scm_dict["dir"] = os.path.join(compose.config_dir, scm_dict["dir"])
 

+     else:
 

+         scm_dict = os.path.join(compose.config_dir, scm_dict)
 

+ 

+     tmp_dir = compose.mkdtemp(prefix="moduledefaults_")
 

+     get_dir_from_scm(scm_dict, tmp_dir, logger=compose._logger)
 

+     compose.log_debug("Writing module defaults")
 

+     shutil.rmtree(os.path.join(compose.config_dir, "module_defaults"), ignore_errors=True)

Why is this deleted? What creates this directory?

There could be some stale data that would clobber ours. The directory is created right on the next line.

I see. In that case it's the wrong location. config_dir is wherever pungi reads the configuration file from, generally outside of the compose. This could delete some unrelated data. A better place would be a new location in work/ subdir. I'll create a patch for that.

 
+     shutil.copytree(tmp_dir, os.path.join(compose.config_dir, "module_defaults"))
 

+     shutil.rmtree(tmp_dir)

file modified
+3 
@@ -24,6 +24,7 @@ 
 

      def test_run(self, write_prepopulate, write_variant, create_comps, write_arch, write_global):
 

          compose = DummyCompose(self.topdir, {})
 

          compose.has_comps = True
 

+         compose.has_module_defaults = False
 

          compose.setup_optional()
 

          phase = init.InitPhase(compose)
 

          phase.run()
 
@@ -51,6 +52,7 @@ 
 

                                 write_arch, write_global):
 

          compose = DummyCompose(self.topdir, {})
 

          compose.has_comps = True
 

+         compose.has_module_defaults = False
 

          compose.variants['Everything'].groups = []
 

          compose.variants['Everything'].modules = []
 

          phase = init.InitPhase(compose)
 
@@ -78,6 +80,7 @@ 
 

                                 write_arch, write_global):
 

          compose = DummyCompose(self.topdir, {})
 

          compose.has_comps = False
 

+         compose.has_module_defaults = False
 

          phase = init.InitPhase(compose)
 

          phase.run()

This changeset adds a new configuration option, module_defaults_dir, which points to a directory containing module defaults to be included in modular variants. Defaults for modules that are included in the variant are pulled in.

Tested on staging composer. There aren't any unit tests at this point since I haven't had a good idea how to test this on that level yet.

'gi.repository.Modulemd' object has no attribute 'Defaults'
do we use the same library?

I understand 'x' it is used only here, but 'item' or another word would be more intelligible.

'gi.repository.Modulemd' object has no attribute 'dump'
Question: what is the purpose of this change?

shouldn't this configuration option be defined in 'pungi/checks.py' as well?

May I suggest this construct?
from pungi.util import temp_dir
with temp_dir(prefix="moduledefaults_") as tmp_dir:
...
-- shutil.rmtree(tmp_dir)

@onosek, yes, that would make sense. Thanks for pointing it out.

@onosek, I've always thought x was rather common in list comprehensions but I can replace it with item if you prefer.

@onosek, as the first commit states, you need libmodulemd-1.2.0 or later for this to work.

@onosek, this is a new method in 1.2.0 which can dump multiple document types at once -- in this case a mixture of modulemd and modulemd-defaults documents.

@onosek, I tried to make it similar to the comps_file code just above for consistency.

Generally the code looks reasonable to me. The tests pass with libmodulemd 1.2.0, but since it's still in updates-testing repo I don't think we can get it to Jenkins just yet. It should go to stable in at most 2 days though.

@sgallagh, please, don't obsolete this update :)

@lsedlar, okay, thanks. I'll rebase the PR and, following @onosek's comments, add a check to pungi/checks.py before that happens.

rebased onto c25fb70
6 years ago

Rebased & updated the JSON schema.

@lsedlar, 1.2.0 is now stable on f27 and f28. f26 and epel7 are still in testing. Which release do you need?

Jenkins is running on F26. I opened a ticket fedora-infrastructure 6848 to request the upgrade.
Edited 6 years ago by lsedlar

Jenkins updated, the tests are working now: https://jenkins.fedorainfracloud.org/job/pungi/1473/

Pretty please pagure-ci rebuild

Why is this deleted? What creates this directory?

There could be some stale data that would clobber ours. The directory is created right on the next line.

I see. In that case it's the wrong location. config_dir is wherever pungi reads the configuration file from, generally outside of the compose. This could delete some unrelated data. A better place would be a new location in work/ subdir. I'll create a patch for that.

Commit 0e7f770 fixes this pull-request

Pull-Request has been merged by lsedlar
6 years ago
Changes Summary 6
+5 -0
file changed
doc/configuration.rst
+1 -0
file changed
pungi/checks.py
+4 -0
file changed
pungi/compose.py
+7 -1
file changed
pungi/phases/createrepo.py
+20 -1
file changed
pungi/phases/init.py
+3 -0
file changed
tests/test_initphase.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.