From 2ae8710934b72f16076f5c74af865e5b4c674715 Mon Sep 17 00:00:00 2001
From: Qixiang Wan <qwan@redhat.com>
Date: Wed, 22 Feb 2017 10:48:05 +0800
Subject: [PATCH] consolidate repo option names

live_images:            additional_repos        -> repo
ostree:                 source_repo_from        -> repo_from
                        extra_source_repos      -> repo
ostree_installer:       source_repo_from        -> repo_from

With the change, the phases have consolidate option names for variant
repos and external repos.

Old option names will continue to work, old names will be converted
to new names after validation automatically if new options are not
specified in config.

Signed-off-by: Qixiang Wan <qwan@redhat.com>
---
 doc/configuration.rst                | 26 +++++++++++++++++++-------
 pungi/checks.py                      | 24 ++++++++++++++++++------
 pungi/ostree/tree.py                 |  4 ++--
 pungi/phases/live_images.py          |  2 +-
 pungi/phases/ostree.py               | 10 +++++-----
 pungi/phases/ostree_installer.py     |  2 +-
 tests/helpers.py                     |  5 +++++
 tests/test_liveimagesphase.py        | 18 +++++++++---------
 tests/test_ostree_installer_phase.py | 20 ++++++++++----------
 tests/test_ostree_phase.py           | 14 +++++++-------
 tests/test_ostree_script.py          |  8 ++++----
 11 files changed, 81 insertions(+), 52 deletions(-)

diff --git a/doc/configuration.rst b/doc/configuration.rst
index 5c51e863..2760e550 100644
--- a/doc/configuration.rst
+++ b/doc/configuration.rst
@@ -925,7 +925,7 @@ Live Images Settings
       * ``ksurl`` (*str*) [optional] -- where to get the kickstart from
       * ``name`` (*str*)
       * ``version`` (*str*)
-      * ``additional_repos`` (*list*) -- external repos specified by URL
+      * ``repo`` (*list*) -- external repos specified by URL
       * ``repo_from`` (*list*) -- repos from other variants
       * ``specfile`` (*str*) -- for images wrapped in RPM
       * ``scratch`` (*bool*) -- only RPM-wrapped images can use scratch builds,
@@ -935,6 +935,10 @@ Live Images Settings
         is ``appliance`` corresponding to ``koji spin-appliance``.
       * ``sign`` (*bool*) -- only RPM-wrapped images can be signed
 
+      Deprecated options:
+
+      * ``additional_repos`` (*list*) -- deprecated, use ``repo`` instead
+
 **live_images_no_rename**
     (*bool*) -- When set to ``True``, filenames generated by Koji will be used.
     When ``False``, filenames will be generated based on ``image_name_format``
@@ -1121,12 +1125,12 @@ a new commit.
 
     * ``treefile`` -- (*str*) Filename of configuration for ``rpm-ostree``.
     * ``config_url`` -- (*str*) URL for Git repository with the ``treefile``.
-    * ``source_repo_from`` -- (*str*) Name of variant serving as source repository.
+    * ``repo_from`` -- (*str*) Name of variant serving as source repository.
     * ``ostree_repo`` -- (*str*) Where to put the ostree repository
 
     These keys are optional:
 
-    * ``extra_source_repos`` -- (*[dict]*) Extra source repos to get packages
+    * ``repo`` -- (*[dict]*) Extra source repos to get packages
       while composing the OSTree repository. Each dict represents a yum repo.
       The allowed keys are:
 
@@ -1149,6 +1153,11 @@ a new commit.
     * ``tag_ref`` -- (*bool*, default ``True``) If set to ``False``, a git
       reference will not be created.
 
+    Deprecated options:
+
+    * ``source_repo_from`` -- (*str*) Deprecated, use ``repo_from`` instead.
+    * ``extra_source_repos`` -- (*[dict]*) Deprecated, use ``repo`` instead.
+
 
 Example config
 --------------
@@ -1159,8 +1168,8 @@ Example config
             "x86_64": {
                 "treefile": "fedora-atomic-docker-host.json",
                 "config_url": "https://git.fedorahosted.org/git/fedora-atomic.git",
-                "source_repo_from": "Server",
-                "extra_source_repos": [
+                "repo_from": "Server",
+                "repo": [
                     {
                         "name": "repo_a",
                         "baseurl": "http://example.com/repo/x86_64/os",
@@ -1193,7 +1202,7 @@ an OSTree repository. This always runs in Koji as a ``runroot`` task.
 
     The configuration dict for each variant arch pair must have this key:
 
-    * ``source_repo_from`` -- (*str|[str]*) Name of variant or a name list of
+    * ``repo_from`` -- (*str|[str]*) Name of variant or a name list of
       variants serving as source repositories.
 
     These keys are optional:
@@ -1220,6 +1229,9 @@ an OSTree repository. This always runs in Koji as a ``runroot`` task.
     ``template_repo`` needs to point to a Git repository from which to take the
     templates.
 
+    Deprecated options:
+
+    * ``source_repo_from`` -- (*str|[str]*) Deprecated, use ``repo_from`` instead.
 
 Example config
 --------------
@@ -1228,7 +1240,7 @@ Example config
     ostree_installer = [
         ("^Atomic$", {
             "x86_64": {
-                "source_repo_from": "Everything",
+                "repo_from": "Everything",
                 "release": None,
                 "installpkgs": ["fedora-productimg-atomic"],
                 "add_template": ["atomic-installer/lorax-configure-repo.tmpl"],
diff --git a/pungi/checks.py b/pungi/checks.py
index a2e410eb..4932b25a 100644
--- a/pungi/checks.py
+++ b/pungi/checks.py
@@ -418,7 +418,10 @@ def _make_schema():
                     "name": {"type": "string"},
                     "subvariant": {"type": "string"},
                     "version": {"type": "string"},
-                    "additional_repos": {"$ref": "#/definitions/strings"},
+                    "repo": {
+                        "$ref": "#/definitions/strings",
+                        "alias": "additional_repos",
+                    },
                     "repo_from": {"$ref": "#/definitions/strings"},
                     "specfile": {"type": "string"},
                     "scratch": {"type": "boolean"},
@@ -764,8 +767,14 @@ def _make_schema():
                 "properties": {
                     "treefile": {"type": "string"},
                     "config_url": {"type": "string"},
-                    "source_repo_from": {"type": "string"},
-                    "extra_source_repos": {"$ref": "#/definitions/list_of_source_repo_dicts"},
+                    "repo_from": {
+                        "type": "string",
+                        "alias": "source_repo_from",
+                    },
+                    "repo": {
+                        "$ref": "#/definitions/list_of_source_repo_dicts",
+                        "alias": "extra_source_repos",
+                    },
                     "keep_original_sources": {"type": "boolean"},
                     "ostree_repo": {"type": "string"},
                     "failable": {"$ref": "#/definitions/list_of_strings"},
@@ -774,7 +783,7 @@ def _make_schema():
                     "config_branch": {"type": "string"},
                     "tag_ref": {"type": "boolean"},
                 },
-                "required": ["treefile", "config_url", "source_repo_from", "ostree_repo"],
+                "required": ["treefile", "config_url", "repo_from", "ostree_repo"],
                 "additionalProperties": False,
             }),
 
@@ -782,7 +791,10 @@ def _make_schema():
                 "type": "object",
                 "properties": {
                     "repo": {"$ref": "#/definitions/strings"},
-                    "source_repo_from": {"$ref": "#/definitions/strings"},
+                    "repo_from": {
+                        "$ref": "#/definitions/strings",
+                        "alias": "source_repo_from",
+                    },
                     "release": {"$ref": "#/definitions/optional_string"},
                     "failable": {"$ref": "#/definitions/list_of_strings"},
                     "installpkgs": {"$ref": "#/definitions/list_of_strings"},
@@ -794,7 +806,7 @@ def _make_schema():
                     "template_repo": {"type": "string"},
                     "template_branch": {"type": "string"},
                 },
-                "required": ["source_repo_from"],
+                "required": ["repo_from"],
                 "additionalProperties": False,
             }),
 
diff --git a/pungi/ostree/tree.py b/pungi/ostree/tree.py
index 49a8b66d..6fb1abd9 100644
--- a/pungi/ostree/tree.py
+++ b/pungi/ostree/tree.py
@@ -89,8 +89,8 @@ class Tree(OSTree):
         self.extra_config = self.args.extra_config
         if self.extra_config:
             self.extra_config = json.load(open(self.extra_config, 'r'))
-            source_repo_from = self.extra_config.get('source_repo_from', None)
-            extra_source_repos = self.extra_config.get('extra_source_repos', [])
+            source_repo_from = self.extra_config.get('repo_from', None)
+            extra_source_repos = self.extra_config.get('repo', [])
             keep_original_sources = self.extra_config.get('keep_original_sources', False)
             repos = extra_source_repos + [{'name': 'source_repo_from', 'baseurl': source_repo_from}]
             tweak_treeconf(self.treefile, source_repos=repos, keep_original_sources=keep_original_sources)
diff --git a/pungi/phases/live_images.py b/pungi/phases/live_images.py
index 2905dbca..20cbbfc0 100644
--- a/pungi/phases/live_images.py
+++ b/pungi/phases/live_images.py
@@ -64,7 +64,7 @@ class LiveImagesPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG
                 self.compose, self.compose.paths.compose.repository(arch, variant, create_dir=False)))
 
         # additional repos
-        repos.extend(data.get("additional_repos", []))
+        repos.extend(data.get("repo", []))
         repos.extend(self._get_extra_repos(arch, variant, force_list(data.get('repo_from', []))))
         return repos
 
diff --git a/pungi/phases/ostree.py b/pungi/phases/ostree.py
index b33c004c..b7ecf0f7 100644
--- a/pungi/phases/ostree.py
+++ b/pungi/phases/ostree.py
@@ -46,7 +46,7 @@ class OSTreeThread(WorkerThread):
                                                (arch, variant.uid, self.num))
         repodir = os.path.join(workdir, 'config_repo')
 
-        source_variant = compose.all_variants[config['source_repo_from']]
+        source_variant = compose.all_variants[config['repo_from']]
         source_repo = translate_path(compose,
                                      compose.paths.compose.repository('$basearch',
                                                                       source_variant,
@@ -54,10 +54,10 @@ class OSTreeThread(WorkerThread):
 
         self._clone_repo(repodir, config['config_url'], config.get('config_branch', 'master'))
 
-        source_repos = [{'name': '%s-%s' % (compose.compose_id, config['source_repo_from']),
+        source_repos = [{'name': '%s-%s' % (compose.compose_id, config['repo_from']),
                          'baseurl': source_repo}]
 
-        extra_source_repos = config.get('extra_source_repos', None)
+        extra_source_repos = config.get('repo', None)
         if extra_source_repos:
             for extra in extra_source_repos:
                 baseurl = extra['baseurl']
@@ -77,9 +77,9 @@ class OSTreeThread(WorkerThread):
 
         # repos in configuration can have repo url set to variant UID,
         # update it to have the actual url that we just translated.
-        new_config.update({'source_repo_from': source_repo})
+        new_config.update({'repo_from': source_repo})
         if extra_source_repos:
-            new_config.update({'extra_source_repos': extra_source_repos})
+            new_config.update({'repo': extra_source_repos})
 
         # remove unnecessary (for 'pungi-make-ostree tree' script ) elements
         # from config, it doesn't hurt to have them, however remove them can
diff --git a/pungi/phases/ostree_installer.py b/pungi/phases/ostree_installer.py
index 77eddcd4..3bfdce19 100644
--- a/pungi/phases/ostree_installer.py
+++ b/pungi/phases/ostree_installer.py
@@ -47,7 +47,7 @@ class OstreeInstallerThread(WorkerThread):
         self.logdir = compose.paths.log.topdir('%s/ostree_installer' % arch)
 
         source_from_repos = [self._get_source_repo(compose, arch, v)
-                             for v in shortcuts.force_list(config['source_repo_from'])]
+                             for v in shortcuts.force_list(config['repo_from'])]
         repos = shortcuts.force_list(config.pop('repo', []))
         source_repos = source_from_repos + repos
         output_dir = os.path.join(compose.paths.work.topdir(arch), variant.uid, 'ostree_installer')
diff --git a/tests/helpers.py b/tests/helpers.py
index 5bc69168..b459ac58 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -38,6 +38,11 @@ class MockVariant(mock.Mock):
         return self.uid
 
 
+class IterableMock(mock.Mock):
+    def __iter__(self):
+        return iter([])
+
+
 class DummyCompose(object):
     def __init__(self, topdir, config):
         self.supported = True
diff --git a/tests/test_liveimagesphase.py b/tests/test_liveimagesphase.py
index 6840d857..e9ff5677 100644
--- a/tests/test_liveimagesphase.py
+++ b/tests/test_liveimagesphase.py
@@ -23,7 +23,7 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': {
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything', 'Server-optional'],
                         'release': None,
                     }
@@ -76,7 +76,7 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': {
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': 'Everything',
                         'release': None,
                     }
@@ -124,7 +124,7 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': {
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                         'release': None,
                     }
@@ -171,11 +171,11 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': [{
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                     }, {
                         'kickstart': 'another.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                     }]
                 })
@@ -244,7 +244,7 @@ class TestLiveImagesPhase(PungiTestCase):
                     'amd64': {
                         'kickstart': 'test.ks',
                         'ksurl': 'https://git.example.com/kickstarts.git?#HEAD',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                         'type': 'appliance',
                     }
@@ -299,7 +299,7 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': {
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                         'type': 'appliance',
                     }
@@ -354,7 +354,7 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': {
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                         'type': 'appliance',
                     }
@@ -406,7 +406,7 @@ class TestLiveImagesPhase(PungiTestCase):
                 ('^Client$', {
                     'amd64': {
                         'kickstart': 'test.ks',
-                        'additional_repos': ['http://example.com/repo/'],
+                        'repo': ['http://example.com/repo/'],
                         'repo_from': ['Everything'],
                         'release': None,
                     }
diff --git a/tests/test_ostree_installer_phase.py b/tests/test_ostree_installer_phase.py
index 0db6af80..8037eb6b 100644
--- a/tests/test_ostree_installer_phase.py
+++ b/tests/test_ostree_installer_phase.py
@@ -20,7 +20,7 @@ class OstreeInstallerPhaseTest(helpers.PungiTestCase):
 
     @mock.patch('pungi.phases.ostree_installer.ThreadPool')
     def test_run(self, ThreadPool):
-        cfg = mock.Mock()
+        cfg = helpers.IterableMock()
         compose = helpers.DummyCompose(self.topdir, {
             'ostree_installer': [
                 ('^Everything$', {'x86_64': cfg})
@@ -133,7 +133,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
         self.compose.supported = False
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': '20160321.n.0',
         }
         koji = KojiWrapper.return_value
@@ -169,7 +169,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                  get_file_size, get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'http://example.com/repo/$arch/',
+            'repo_from': 'http://example.com/repo/$arch/',
             'release': '20160321.n.0',
         }
         koji = KojiWrapper.return_value
@@ -203,7 +203,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                get_file_size, get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': '20160321.n.0',
             'repo': [
                 'https://example.com/extra-repo1.repo',
@@ -241,7 +241,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                              get_file_size, get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': ['Everything', 'Server'],
+            'repo_from': ['Everything', 'Server'],
             'release': '20160321.n.0',
             'repo': [
                 'https://example.com/extra-repo1.repo',
@@ -281,7 +281,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                                           get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': '20160321.n.0',
             'add_template': ['some-file.txt'],
         }
@@ -314,7 +314,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                  get_dir_from_scm):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': '20160321.n.0',
             'add_template': ['some_file.txt'],
             'add_arch_template': ['other_file.txt'],
@@ -362,7 +362,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                        get_file_size, get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': None,
             "installpkgs": ["fedora-productimg-atomic"],
             "add_template": ["/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl"],
@@ -423,7 +423,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                         get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': None,
             'failable': ['x86_64']
         }
@@ -449,7 +449,7 @@ class OstreeThreadTest(helpers.PungiTestCase):
                                get_file_size, get_mtime, ImageCls, run):
         pool = mock.Mock()
         cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'release': None,
             'failable': ['*'],
         }
diff --git a/tests/test_ostree_phase.py b/tests/test_ostree_phase.py
index ec458c76..405a7343 100644
--- a/tests/test_ostree_phase.py
+++ b/tests/test_ostree_phase.py
@@ -19,7 +19,7 @@ class OSTreePhaseTest(helpers.PungiTestCase):
 
     @mock.patch('pungi.phases.ostree.ThreadPool')
     def test_run(self, ThreadPool):
-        cfg = mock.Mock()
+        cfg = helpers.IterableMock()
         compose = helpers.DummyCompose(self.topdir, {
             'ostree': [
                 ('^Everything$', {'x86_64': cfg})
@@ -51,7 +51,7 @@ class OSTreeThreadTest(helpers.PungiTestCase):
         self.repo = os.path.join(self.topdir, 'place/for/atomic')
         os.makedirs(os.path.join(self.repo, 'refs', 'heads'))
         self.cfg = {
-            'source_repo_from': 'Everything',
+            'repo_from': 'Everything',
             'config_url': 'https://git.fedorahosted.org/git/fedora-atomic.git',
             'config_branch': 'f24',
             'treefile': 'fedora-atomic-docker-host.json',
@@ -305,8 +305,8 @@ class OSTreeThreadTest(helpers.PungiTestCase):
         koji.run_runroot_cmd.side_effect = self._mock_runroot(0)
 
         cfg = {
-            'source_repo_from': 'Everything',
-            'extra_source_repos': [
+            'repo_from': 'Everything',
+            'repo': [
                 {
                     'name': 'repo_a',
                     'baseurl': 'http://url/to/repo/a',
@@ -333,9 +333,9 @@ class OSTreeThreadTest(helpers.PungiTestCase):
         self.assertTrue(os.path.isfile(extra_config_file))
         extra_config = json.load(open(extra_config_file, 'r'))
         self.assertTrue(extra_config.get('keep_original_sources', False))
-        self.assertEqual(extra_config.get('source_repo_from', None), 'http://example.com/Everything/$basearch/os')
-        self.assertEqual(len(extra_config.get('extra_source_repos', [])), len(cfg['extra_source_repos']))
-        self.assertEqual(extra_config.get('extra_source_repos').pop()['baseurl'], 'http://example.com/Server/$basearch/os')
+        self.assertEqual(extra_config.get('repo_from', None), 'http://example.com/Everything/$basearch/os')
+        self.assertEqual(len(extra_config.get('repo', [])), len(cfg['repo']))
+        self.assertEqual(extra_config.get('repo').pop()['baseurl'], 'http://example.com/Server/$basearch/os')
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/test_ostree_script.py b/tests/test_ostree_script.py
index 94868c7e..68335552 100644
--- a/tests/test_ostree_script.py
+++ b/tests/test_ostree_script.py
@@ -156,8 +156,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
 
         extra_config_file = os.path.join(self.topdir, 'extra_config.json')
         extra_config = {
-            "source_repo_from": "http://www.example.com/Server.repo",
-            "extra_source_repos": [
+            "repo_from": "http://www.example.com/Server.repo",
+            "repo": [
                 {
                     "name": "optional",
                     "baseurl": "http://example.com/repo/x86_64/optional",
@@ -230,8 +230,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
 
         extra_config_file = os.path.join(self.topdir, 'extra_config.json')
         extra_config = {
-            "source_repo_from": "http://www.example.com/Server.repo",
-            "extra_source_repos": [
+            "repo_from": "http://www.example.com/Server.repo",
+            "repo": [
                 {
                     "name": "optional",
                     "baseurl": "http://example.com/repo/x86_64/optional",