From 079177c5bf498fe21c1f0b03d127bfa25fa09fe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Wed, 11 Apr 2018 09:20:51 +0200 Subject: [PATCH 3/3] Revert "Ostree can use pkgset repos" This reverts commit c7cc200246300c6a3946b2e3a9f5f7693896a7d6. --- pungi/phases/ostree.py | 7 +---- pungi/util.py | 35 +++++++++++++----------- tests/test_config_validate_script.py | 4 ++- tests/test_ostree_phase.py | 52 +++++++----------------------------- tests/test_util.py | 26 +++++++++++------- 5 files changed, 50 insertions(+), 74 deletions(-) diff --git a/pungi/phases/ostree.py b/pungi/phases/ostree.py index 2918fff8..0550b18c 100644 --- a/pungi/phases/ostree.py +++ b/pungi/phases/ostree.py @@ -58,12 +58,7 @@ class OSTreeThread(WorkerThread): repodir = os.path.join(workdir, 'config_repo') self._clone_repo(repodir, config['config_url'], config.get('config_branch', 'master')) - repo_baseurl = compose.paths.work.arch_repo('$basearch', create_dir=False) - comps_repo = compose.paths.work.comps_repo('$basearch', variant=variant, create_dir=False) - repos = shortcuts.force_list(config['repo']) + [translate_path(compose, repo_baseurl)] - if compose.has_comps: - repos.append(translate_path(compose, comps_repo)) - repos = get_repo_dicts(repos, logger=self.pool) + repos = get_repo_dicts(compose, shortcuts.force_list(config['repo'])) # copy the original config and update before save to a json file new_config = copy.copy(config) diff --git a/pungi/util.py b/pungi/util.py index 31e763b5..9f0aab69 100644 --- a/pungi/util.py +++ b/pungi/util.py @@ -728,16 +728,19 @@ def _translate_url_to_repo_id(url): return ''.join([s if s in list(_REPOID_CHARS) else '_' for s in url]) -def get_repo_dict(repo): +def get_repo_dict(compose, repo, arch='$basearch'): """ Convert repo to a dict of repo options. - If repo is a string that represents url, set it as 'baseurl' in result dict, - also generate a repo id/name as 'name' key in result dict. - If repo is a dict, and if 'name' key is missing in the dict, generate one for it. - Repo (str or dict) that has not url format is no longer processed. + If repo is a string, translate it to repo url if necessary (when it's + not a url), and set it as 'baseurl' in result dict, also generate + a repo id/name as 'name' key in result dict. + If repo is a dict, translate value of 'baseurl' key to url if necessary, + if 'name' key is missing in the dict, generate one for it. + @param compose - required for access to variants @param repo - A string or dict, if it is a dict, key 'baseurl' is required + @param arch - string to be used as arch in repo url """ repo_dict = {} if isinstance(repo, dict): @@ -747,8 +750,10 @@ def get_repo_dict(repo): if name is None: name = _translate_url_to_repo_id(url) else: - # url is variant uid - this possibility is now discontinued - return {} + # url is variant uid + if name is None: + name = '%s-%s' % (compose.compose_id, url) + url = get_repo_url(compose, url, arch=arch) repo['name'] = name repo['baseurl'] = url return repo @@ -759,24 +764,24 @@ def get_repo_dict(repo): repo_dict['name'] = _translate_url_to_repo_id(repo) repo_dict['baseurl'] = repo else: - return {} + repo_dict['name'] = '%s-%s' % (compose.compose_id, repo) + repo_dict['baseurl'] = get_repo_url(compose, repo) + return repo_dict -def get_repo_dicts(repos, logger=None): +def get_repo_dicts(compose, repos, arch='$basearch'): """ Convert repos to a list of repo dicts. + @param compose - required for access to variants @param repo - A list of string or dict, if item is a dict, key 'baseurl' is required + @param arch - string to be used as arch in repo url """ repo_dicts = [] for repo in repos: - repo_dict = get_repo_dict(repo) - if repo_dict == {}: - if logger: - logger.log_warning("Variant-type source repository is deprecated and will be ignored during 'ostree' phase: %s" % (repo)) - else: - repo_dicts.append(repo_dict) + repo_dict = get_repo_dict(compose, repo, arch=arch) + repo_dicts.append(repo_dict) return repo_dicts diff --git a/tests/test_config_validate_script.py b/tests/test_config_validate_script.py index 031a15a4..c73fe126 100644 --- a/tests/test_config_validate_script.py +++ b/tests/test_config_validate_script.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- +import mock import os import subprocess import sys @@ -19,7 +20,8 @@ from tests import helpers class ConfigValidateScriptTest(helpers.PungiTestCase): - def test_validate_dummy_config(self): + @mock.patch('kobo.shortcuts.run') + def test_validate_dummy_config(self, run): DUMMY_CONFIG = os.path.join(HERE, 'data/dummy-pungi.conf') interp = 'python2' if six.PY2 else 'python3' p = subprocess.Popen([interp, PUNGI_CONFIG_VALIDATE, DUMMY_CONFIG], diff --git a/tests/test_ostree_phase.py b/tests/test_ostree_phase.py index ad14eca4..3a9ebf32 100644 --- a/tests/test_ostree_phase.py +++ b/tests/test_ostree_phase.py @@ -118,7 +118,7 @@ class OSTreeThreadTest(helpers.PungiTestCase): 'koji_profile': 'koji', 'runroot_tag': 'rrt', 'translate_paths': [ - (self.topdir, 'http://example.com') + (self.topdir + '/compose', 'http://example.com') ] }) self.pool = mock.Mock() @@ -148,36 +148,6 @@ class OSTreeThreadTest(helpers.PungiTestCase): return {'task_id': 1234, 'retcode': retcode, 'output': 'Foo bar\n'} return fake_runroot - @mock.patch('pungi.wrappers.scm.get_dir_from_scm') - @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper') - def test_extra_config_content(self, KojiWrapper, get_dir_from_scm): - get_dir_from_scm.side_effect = self._dummy_config_repo - self.compose.conf['runroot_weights'] = {'ostree': 123} - - koji = KojiWrapper.return_value - koji.run_runroot_cmd.side_effect = self._mock_runroot(0) - - t = ostree.OSTreeThread(self.pool) - - extra_config_file = os.path.join(self.topdir, 'work/ostree-1/extra_config.json') - self.assertFalse(os.path.isfile(extra_config_file)) - - t.process((self.compose, self.compose.variants['Everything'], 'x86_64', self.cfg), 1) - - self.assertTrue(os.path.isfile(extra_config_file)) - with open(extra_config_file, 'r') as f: - extraconf_content = json.load(f) - - proper_extraconf_content = { - "repo": [ - {"name": "http:__example.com_work__basearch_repo", - "baseurl": "http://example.com/work/$basearch/repo"}, - {"name": "http:__example.com_work__basearch_comps_repo_Everything", - "baseurl": "http://example.com/work/$basearch/comps_repo_Everything"} - ] - } - self.assertEqual(proper_extraconf_content, extraconf_content) - @mock.patch('pungi.wrappers.scm.get_dir_from_scm') @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper') def test_run(self, KojiWrapper, get_dir_from_scm): @@ -299,7 +269,7 @@ class OSTreeThreadTest(helpers.PungiTestCase): arch='x86_64', ref='fedora-atomic/25/x86_64', commitid=None, - repo_path='http://example.com/place/for/atomic', + repo_path=self.repo, local_repo_path=self.repo)]) @mock.patch('pungi.wrappers.scm.get_dir_from_scm') @@ -432,14 +402,14 @@ class OSTreeThreadTest(helpers.PungiTestCase): koji.run_runroot_cmd.side_effect = self._mock_runroot(0) cfg = { - 'repo': [ # Variant type repos will not be included into extra_config. This part of the config is deprecated - 'Everything', # do not include + 'repo': [ + 'Everything', { 'name': 'repo_a', 'baseurl': 'http://url/to/repo/a', 'exclude': 'systemd-container' }, - { # do not include + { 'name': 'Server', 'baseurl': 'Server', 'exclude': 'systemd-container' @@ -458,16 +428,12 @@ class OSTreeThreadTest(helpers.PungiTestCase): extra_config_file = os.path.join(self.topdir, 'work/ostree-1/extra_config.json') self.assertTrue(os.path.isfile(extra_config_file)) - with open(extra_config_file, 'r') as extra_config_fd: - extra_config = json.load(extra_config_fd) + extra_config = json.load(open(extra_config_file, 'r')) self.assertTrue(extra_config.get('keep_original_sources', False)) - # should equal to number of valid repositories in cfg['repo'] + default repository + comps repository - self.assertEqual(len(extra_config.get('repo', [])), 3) - self.assertEqual(extra_config.get('repo').pop()['baseurl'], - 'http://example.com/work/$basearch/comps_repo_Everything') - self.assertEqual(extra_config.get('repo').pop()['baseurl'], 'http://example.com/work/$basearch/repo') + self.assertEqual(len(extra_config.get('repo', [])), len(cfg['repo'])) + self.assertEqual(extra_config.get('repo').pop()['baseurl'], 'http://example.com/Server/$basearch/os') self.assertEqual(extra_config.get('repo').pop()['baseurl'], 'http://url/to/repo/a') - + self.assertEqual(extra_config.get('repo').pop()['baseurl'], 'http://example.com/Everything/$basearch/os') if __name__ == '__main__': unittest.main() diff --git a/tests/test_util.py b/tests/test_util.py index 739ea9e1..ab4f3459 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -587,35 +587,43 @@ class GetRepoFuncsTestCase(unittest.TestCase): self.assertEqual(util.get_repo_urls(self.compose, repos), expect) def test_get_repo_dict_from_normal_url(self): - repo_dict = util.get_repo_dict('http://example.com/repo') + repo_dict = util.get_repo_dict(self.compose, 'http://example.com/repo') expect = {'name': 'http:__example.com_repo', 'baseurl': 'http://example.com/repo'} self.assertEqual(repo_dict, expect) def test_get_repo_dict_from_variant_uid(self): - repo_dict = util.get_repo_dict('Server') # this repo format is deprecated - expect = {} + repo_dict = util.get_repo_dict(self.compose, 'Server') + expect = { + 'name': "%s-%s" % (self.compose.compose_id, 'Server'), + 'baseurl': 'http://example.com/RHEL-8.0-20180101.n.0/compose/Server/$basearch/os', + } self.assertEqual(repo_dict, expect) def test_get_repo_dict_from_repo_dict(self): - repo = {'baseurl': 'Server'} # this repo format is deprecated - expect = {} - repo_dict = util.get_repo_dict(repo) + repo = {'baseurl': 'Server'} + expect = { + 'name': '%s-%s' % (self.compose.compose_id, 'Server'), + 'baseurl': 'http://example.com/RHEL-8.0-20180101.n.0/compose/Server/$basearch/os' + } + repo_dict = util.get_repo_dict(self.compose, repo) self.assertEqual(repo_dict, expect) def test_get_repo_dicts(self): repos = [ 'http://example.com/repo', - 'Server', # this repo format is deprecated (and will not be included into final repo_dict) - {'baseurl': 'Client'}, # this repo format is deprecated + 'Server', + {'baseurl': 'Client'}, {'baseurl': 'ftp://example.com/linux/repo'}, {'name': 'testrepo', 'baseurl': 'ftp://example.com/linux/repo'}, ] expect = [ {'name': 'http:__example.com_repo', 'baseurl': 'http://example.com/repo'}, + {'name': '%s-%s' % (self.compose.compose_id, 'Server'), 'baseurl': 'http://example.com/RHEL-8.0-20180101.n.0/compose/Server/$basearch/os'}, + {'name': '%s-%s' % (self.compose.compose_id, 'Client'), 'baseurl': 'http://example.com/RHEL-8.0-20180101.n.0/compose/Client/$basearch/os'}, {'name': 'ftp:__example.com_linux_repo', 'baseurl': 'ftp://example.com/linux/repo'}, {'name': 'testrepo', 'baseurl': 'ftp://example.com/linux/repo'}, ] - repos = util.get_repo_dicts(repos) + repos = util.get_repo_dicts(self.compose, repos) self.assertEqual(repos, expect) -- 2.14.4