From 902627c14cd19e3fd211bf830aca89ceb791d509 Mon Sep 17 00:00:00 2001 From: Stephan Kulow Date: Sat, 8 Dec 2018 12:04:36 +0100 Subject: [PATCH] pkglistgen: Fix test failures (including flake8) --- .travis.yml | 4 +- pkglistgen/cli.py | 19 +++---- pkglistgen/group.py | 18 +++---- pkglistgen/solv_utils.py | 30 +++++++---- pkglistgen/tool.py | 105 +++++++++++++++++++-------------------- 5 files changed, 94 insertions(+), 82 deletions(-) diff --git a/.travis.yml b/.travis.yml index 96194e67..44e057a0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -71,7 +71,7 @@ matrix: # Needs python prefix to use the correct interpretor. - python ./obs_clone.py --cache --debug --apiurl-target local script: - - nosetests --with-coverage --cover-package=. --cover-inclusive --exclude-dir=./oqamaint -c .noserc + - nosetests --with-coverage --cover-package=. --cover-inclusive --exclude-dir=./oqamaint --exclude-dir=./pkglistgen -c .noserc after_success: - coveralls - env: TEST_SUITE=nosetests-osc-python3 @@ -99,7 +99,7 @@ matrix: # Needs python prefix to use the correct interpretor. - python ./obs_clone.py --cache --debug --apiurl-target local script: - - nosetests --with-coverage --cover-package=. --cover-inclusive --exclude-dir=./oqamaint -c .noserc + - nosetests --with-coverage --cover-package=. --cover-inclusive --exclude-dir=./oqamaint --exclude-dir=./pkglistgen -c .noserc after_success: - coveralls - env: TEST_SUITE=nosetests-osc-python3 diff --git a/pkglistgen/cli.py b/pkglistgen/cli.py index 0fb24716..cead099c 100755 --- a/pkglistgen/cli.py +++ b/pkglistgen/cli.py @@ -13,7 +13,7 @@ from osc import conf from osclib.conf import Config from osclib.stagingapi import StagingAPI from pkglistgen import solv_utils -from pkglistgen.tool import PkgListGen, CACHEDIR +from pkglistgen.tool import PkgListGen class CommandLineInterface(ToolBase.CommandLineInterface): SCOPES = ['all', 'target', 'rings', 'staging'] @@ -39,7 +39,7 @@ class CommandLineInterface(ToolBase.CommandLineInterface): ${cmd_usage} ${cmd_option_list} """ - self.tool.create_sle_weakremovers(target, prjs) + return self.tool.create_sle_weakremovers(target, prjs) @cmdln.option('-o', '--output-dir', dest='output_dir', metavar='DIR', help='output directory', default='.') def do_create_droplist(self, subcmd, opts, *oldsolv): @@ -55,7 +55,7 @@ class CommandLineInterface(ToolBase.CommandLineInterface): ${cmd_usage} ${cmd_option_list} """ - return self.tool.create_droplist(oldsolv, output_dir=self.options.output_dir) + return self.tool.create_droplist(self.options.output_dir, oldsolv) @cmdln.option('-o', '--output-dir', dest='output_dir', metavar='DIR', help='output directory', default='.') @cmdln.option('--overwrite', action='store_true', help='overwrite if output file exists') @@ -88,8 +88,6 @@ class CommandLineInterface(ToolBase.CommandLineInterface): ${cmd_option_list} """ - self.error_occured = False - if opts.staging: match = re.match('(.*):Staging:(.*)', opts.staging) opts.scope = ['staging:' + match.group(2)] @@ -102,7 +100,7 @@ class CommandLineInterface(ToolBase.CommandLineInterface): opts.scope = ['all'] apiurl = conf.config['apiurl'] - config = Config(apiurl, opts.project) + Config(apiurl, opts.project) target_config = conf.config[opts.project] # Store target project as opts.project will contain subprojects. @@ -124,14 +122,17 @@ class CommandLineInterface(ToolBase.CommandLineInterface): if opts.scope == ['all']: opts.scope = target_config.get('pkglistgen-scopes', 'target').split(' ') + self.error_occured = False + def solve_project(project, scope): try: - self.tool.update_and_solve_target(api, target_project, target_config, main_repo, + if self.tool.update_and_solve_target(api, target_project, target_config, main_repo, project=project, scope=scope, force=opts.force, no_checkout=opts.no_checkout, only_release_packages=opts.only_release_packages, - stop_after_solve=opts.stop_after_solve, drop_list=(scope == 'target')) - except Exception as e: + stop_after_solve=opts.stop_after_solve, drop_list=(scope == 'target')): + self.error_occured = True + except Exception: # Print exception, but continue to prevent problems effecting one # project from killing the whole process. Downside being a common # error will be duplicated for each project. Common exceptions could diff --git a/pkglistgen/group.py b/pkglistgen/group.py index f2099b31..98ac1267 100644 --- a/pkglistgen/group.py +++ b/pkglistgen/group.py @@ -145,7 +145,7 @@ class Group(object): # only add recommends that exist as packages rec = pool.select(dep.str(), solv.Selection.SELECTION_NAME) if not rec.isempty(): - extra.append([dep.str(), group + ":recommended:" + n]) + extra.append([dep.str(), group + ':recommended:' + n]) jobs += sel.jobs(solv.Job.SOLVER_INSTALL) @@ -166,7 +166,7 @@ class Group(object): problems = solver.solve(jobs) if problems: for problem in problems: - msg = 'unresolvable: %s.%s: %s', self.name, arch, problem + msg = 'unresolvable: {}:{}.{}: {}'.format(self.name, n, arch, problem) if self.pkglist.ignore_broken: self.logger.debug(msg) else: @@ -259,21 +259,21 @@ class Group(object): for arch in self.pkglist.filtered_architectures: mp.update(m.solved_packages[arch]) if len(packages & mp): - overlap.comment += '\n overlapping between ' + self.name + ' and ' + m.name + "\n" + overlap.comment += '\n overlapping between ' + self.name + ' and ' + m.name + '\n' for p in sorted(packages & mp): for arch in m.solved_packages.keys(): if m.solved_packages[arch].get(p, None): - overlap.comment += " # " + m.name + "." + arch + ': ' + m.solved_packages[arch][p] + "\n" + overlap.comment += ' # ' + m.name + '.' + arch + ': ' + m.solved_packages[arch][p] + '\n' if self.solved_packages[arch].get(p, None): - overlap.comment += " # " + self.name + "." + \ - arch + ': ' + self.solved_packages[arch][p] + "\n" - overlap.comment += ' - ' + p + "\n" + overlap.comment += ' # ' + self.name + '.' + \ + arch + ': ' + self.solved_packages[arch][p] + '\n' + overlap.comment += ' - ' + p + '\n' overlap._add_to_packages(p) def collect_devel_packages(self): for arch in self.pkglist.filtered_architectures: pool = self.pkglist._prepare_pool(arch) - sel = pool.Selection() + pool.Selection() for s in pool.solvables_iter(): if s.name.endswith('-devel'): # don't ask me why, but that's how it seems to work @@ -343,7 +343,7 @@ class Group(object): attrs = {'name': name} if status is not None: attrs['supportstatus'] = status - p = ET.SubElement(packagelist, 'package', attrs) + ET.SubElement(packagelist, 'package', attrs) if name in packages and packages[name]: c = ET.Comment(' reason: {} '.format(packages[name])) packagelist.append(c) diff --git a/pkglistgen/solv_utils.py b/pkglistgen/solv_utils.py index c75b9ff5..2f88502b 100644 --- a/pkglistgen/solv_utils.py +++ b/pkglistgen/solv_utils.py @@ -1,22 +1,34 @@ from __future__ import print_function +import filecmp +import glob +import gzip +import hashlib +import io import logging import os.path import random import string -import sys -import tempfile -import hashlib -import gzip -import io import subprocess +import sys +import shutil +import tempfile from lxml import etree as ET +from osc import conf +from osclib.util import project_list_family +from osclib.util import project_list_family_prior +from osclib.conf import Config +from osclib.cache_manager import CacheManager + import requests import solv +# share header cache with repochecker +CACHEDIR = CacheManager.directory('repository-meta') + try: from urllib.parse import urljoin except ImportError: @@ -62,7 +74,7 @@ def dump_solv(baseurl, output_dir, overwrite): name = os.path.join(output_dir, '{}.solv'.format(build)) # For update repo name never changes so always update. if not overwrite and repo_style != 'update' and os.path.exists(name): - logger.info("%s exists", name) + logger.info('%s exists', name) return name pool = solv.Pool() @@ -82,7 +94,7 @@ def dump_solv(baseurl, output_dir, overwrite): if repo_style == 'update': name = os.path.join(output_dir, '{}::{}.solv'.format(build, sha256_expected)) if not overwrite and os.path.exists(name): - logger.info("%s exists", name) + logger.info('%s exists', name) return name # Only consider latest update repo so remove old versions. @@ -153,7 +165,7 @@ def solv_cache_update(apiurl, cache_dir_solv, target_project, family_last, famil project_family.extend(project_list_family(apiurl, family_include)) for project in project_family: - config = Config(apiurl, project) + Config(apiurl, project) project_config = conf.config[project] baseurl = project_config.get('download-baseurl') @@ -204,7 +216,7 @@ def solv_cache_update(apiurl, cache_dir_solv, target_project, family_last, famil return prior -def update_merge(self, nonfree, repos, architecture): +def update_merge(self, nonfree, repos, architectures): """Merge free and nonfree solv files or copy free to merged""" for project, repo in repos: for arch in architectures: diff --git a/pkglistgen/tool.py b/pkglistgen/tool.py index 5b59c1eb..79020c83 100644 --- a/pkglistgen/tool.py +++ b/pkglistgen/tool.py @@ -9,33 +9,33 @@ import solv import shutil import subprocess import yaml +import sys + from lxml import etree as ET -from osc import conf from osc.core import checkout_package -from osc.core import http_GET, http_PUT +from osc.core import http_GET from osc.core import HTTPError from osc.core import show_results_meta from osc.core import Package +from osc.core import undelete_package from osclib.core import target_archs -from osclib.conf import Config, str2bool +from osclib.conf import str2bool from osclib.core import repository_path_expand from osclib.core import repository_arch_state from osclib.cache_manager import CacheManager try: - from urllib.parse import urljoin, urlparse + from urllib.parse import urlparse except ImportError: # python 2.x - from urlparse import urljoin, urlparse + from urlparse import urlparse -from pkglistgen import file_utils +from pkglistgen import file_utils, solv_utils from pkglistgen.group import Group, ARCHITECTURES SCRIPT_PATH = os.path.dirname(os.path.realpath(__file__)) -# share header cache with repochecker -CACHEDIR = CacheManager.directory('repository-meta') PRODUCT_SERVICE = '/usr/lib/obs/service/create_single_product' @@ -85,7 +85,7 @@ class PkgListGen(ToolBase.ToolBase): output = None unwanted = None with open(fn, 'r') as fh: - self.logger.debug("reading %s", fn) + self.logger.debug('reading %s', fn) for groupname, group in yaml.safe_load(fh).items(): if groupname == 'OUTPUT': output = group @@ -194,12 +194,12 @@ class PkgListGen(ToolBase.ToolBase): for project, reponame in self.repos: repo = pool.add_repo(project) - s = os.path.join(CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, reponame, arch)) + s = os.path.join(solv_utils.CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, reponame, arch)) r = repo.add_solv(s) if not r: if not self.did_update: raise Exception( - "failed to add repo {}/{}/{}. Need to run update first?".format(project, reponame, arch)) + 'failed to add repo {}/{}/{}. Need to run update first?'.format(project, reponame, arch)) continue for solvable in repo.solvables_iter(): solvable.unset(solv.SOLVABLE_CONFLICTS) @@ -227,7 +227,7 @@ class PkgListGen(ToolBase.ToolBase): if not os.path.isfile(filename): return set() fh = open(filename, 'r') - self.logger.debug("reading %s", filename) + self.logger.debug('reading %s', filename) result = set() for groupname, group in yaml.safe_load(fh).items(): result.update(group) @@ -247,7 +247,7 @@ class PkgListGen(ToolBase.ToolBase): for arch in self.filtered_architectures: pool = self._prepare_pool(arch) - sel = pool.Selection() + pool.Selection() archpacks = [s.name for s in pool.solvables_iter()] # copy @@ -288,18 +288,18 @@ class PkgListGen(ToolBase.ToolBase): del unsorted.solved_packages[arch][p] with open(os.path.join(self.output_dir, 'unsorted.yml'), 'w') as fh: - fh.write("unsorted:\n") + fh.write('unsorted:\n') for p in sorted(packages.keys()): - fh.write(" - ") + fh.write(' - ') fh.write(p) if len(packages[p]) != len(self.filtered_architectures): - fh.write(": [") + fh.write(': [') fh.write(','.join(sorted(packages[p]))) - fh.write("]") + fh.write(']') reason = self._find_reason(p, modules) if reason: fh.write(' # ' + reason) - fh.write(" \n") + fh.write(' \n') # give a hint if the package is related to a group def _find_reason(self, package, modules): @@ -323,7 +323,7 @@ class PkgListGen(ToolBase.ToolBase): for project, repo in self.repos: for arch in architectures: # TODO: refactor to common function with repo_checker.py - d = os.path.join(CACHEDIR, project, repo, arch) + d = os.path.join(solv_utils.CACHEDIR, project, repo, arch) if not os.path.exists(d): os.makedirs(d) @@ -335,7 +335,7 @@ class PkgListGen(ToolBase.ToolBase): # Would be preferable to include hash in name, but cumbersome to handle without # reworking a fair bit since the state needs to be tracked. - solv_file = os.path.join(CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, repo, arch)) + solv_file = os.path.join(solv_utils.CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, repo, arch)) solv_file_hash = '{}::{}'.format(solv_file, state) if os.path.exists(solv_file) and os.path.exists(solv_file_hash): # Solve file exists and hash unchanged, skip updating solv. @@ -385,8 +385,8 @@ class PkgListGen(ToolBase.ToolBase): sysrepo = None for project, repo in self.repos: - self.logger.debug("processing %s/%s/%s", project, repo, arch) - fn = os.path.join(CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, repo, arch)) + self.logger.debug('processing %s/%s/%s', project, repo, arch) + fn = os.path.join(solv_utils.CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, repo, arch)) r = pool.add_repo('/'.join([project, repo])) r.add_solv(fn) if project == target and repo == 'standard': @@ -439,20 +439,20 @@ class PkgListGen(ToolBase.ToolBase): print('%endif') - def create_droplist(self, output_dir): + def create_droplist(self, output_dir, oldsolv): drops = dict() for arch in self.architectures: for old in oldsolv: - logger.debug("%s: processing %s", arch, old) + self.logger.debug('%s: processing %s', arch, old) pool = solv.Pool() pool.setarch(arch) for project, repo in self.repos: - fn = os.path.join(CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, repo, arch)) + fn = os.path.join(solv_utils.CACHEDIR, 'repo-{}-{}-{}.solv'.format(project, repo, arch)) r = pool.add_repo(project) r.add_solv(fn) @@ -488,11 +488,11 @@ class PkgListGen(ToolBase.ToolBase): ofh = open(name, 'w') for reponame in sorted(set(drops.values())): - print("" % reponame, file=ofh) + print('' % reponame, file=ofh) for p in sorted(drops): if drops[p] != reponame: continue - print(" %s" % p, file=ofh) + print(' %s' % p, file=ofh) def solve_project(self, ignore_unresolvable=False, ignore_recommended=False, locale=None, locales_from=None): """ @@ -514,7 +514,7 @@ class PkgListGen(ToolBase.ToolBase): if locales_from: with open(os.path.join(self.input_dir, locales_from), 'r') as fh: root = ET.parse(fh).getroot() - self.locales |= set([lang.text for lang in root.findall(".//linguas/language")]) + self.locales |= set([lang.text for lang in root.findall('.//linguas/language')]) modules = [] # the yml parser makes an array out of everything, so @@ -556,11 +556,10 @@ class PkgListGen(ToolBase.ToolBase): self._collect_unsorted_packages(modules, self.groups.get('unsorted')) return self.write_all_groups() - - # staging projects don't need source and debug medium - and the glibc source - # rpm conflicts between standard and bootstrap_copy repository causing the - # product builder to fail def strip_medium_from_staging(self, path): + # staging projects don't need source and debug medium - and the glibc source + # rpm conflicts between standard and bootstrap_copy repository causing the + # product builder to fail medium = re.compile('name="(DEBUG|SOURCE)MEDIUM"') for name in glob.glob(os.path.join(path, '*.kiwi')): lines = open(name).readlines() @@ -568,11 +567,10 @@ class PkgListGen(ToolBase.ToolBase): open(name, 'w').writelines(lines) def build_stub(self, destination, extension): - f = file(os.path.join(destination, '.'.join(['stub', extension])), 'w+') - f.write('# prevent building single {} files twice\n'.format(extension)) - f.write('Name: stub\n') - f.write('Version: 0.0\n') - f.close() + with open(os.path.join(destination, '.'.join(['stub', extension])), 'w+') as f: + f.write('# prevent building single {} files twice\n'.format(extension)) + f.write('Name: stub\n') + f.write('Version: 0.0\n') def commit_package(self, path): if self.dry_run: @@ -634,7 +632,7 @@ class PkgListGen(ToolBase.ToolBase): for package in checkout_list: if no_checkout: - print("Skipping checkout of {}/{}".format(project, package)) + print('Skipping checkout of {}/{}'.format(project, package)) continue checkout_package(api.apiurl, project, package, expand_link=True, prj_dir=cache_dir) @@ -696,12 +694,11 @@ class PkgListGen(ToolBase.ToolBase): cache_dir_solv_current = os.path.join(cache_dir_solv, target_project) solv_prior.update(glob.glob(os.path.join(cache_dir_solv_current, '*.merged.solv'))) for solv_file in solv_prior: - logger.debug(solv_file.replace(cache_dir_solv, '')) + self.logger.debug(solv_file.replace(cache_dir_solv, '')) print('-> do_create_droplist') # Reset to product after solv_cache_update(). - self.options.output_dir = product_dir - self.do_create_droplist('create_droplist', opts, *solv_prior) + self.create_droplist(product_dir, *solv_prior) delete_products = target_config.get('pkglistgen-delete-products', '').split(' ') file_utils.unlink_list(product_dir, delete_products) @@ -733,31 +730,33 @@ class PkgListGen(ToolBase.ToolBase): self.build_stub(product_dir, 'kiwi') self.commit_package(product_dir) + error_output = '' reference_summary = os.path.join(group_dir, 'reference-summary.yml') if os.path.isfile(reference_summary): summary_file = os.path.join(product_dir, 'summary.yml') with open(summary_file, 'w') as f: - f.write("# Summary of packages in groups") + f.write('# Summary of packages in groups') for group in sorted(summary.keys()): # the unsorted group should appear filtered by # unneeded.yml - so we need the content of unsorted.yml # not unsorted.group (this grew a little unnaturally) if group == 'unsorted': continue - f.write("\n" + group + ":\n") + f.write('\n' + group + ':\n') for package in sorted(summary[group]): - f.write(" - " + package + "\n") - + f.write(' - ' + package + '\n') try: - subprocess.check_output(["diff", "-u", reference_summary, summary_file]) - except subprocess.CalledProcessError, e: - print(e.output) - self.error_occured = True + error_output += subprocess.check_output(['diff', '-u', reference_summary, summary_file]) + except subprocess.CalledProcessError as e: + error_output += e.output reference_unsorted = os.path.join(group_dir, 'reference-unsorted.yml') unsorted_file = os.path.join(product_dir, 'unsorted.yml') try: - subprocess.check_output(["diff", "-u", reference_unsorted, unsorted_file]) - except subprocess.CalledProcessError, e: - print(e.output) - self.error_occured = True + error_output += subprocess.check_output(['diff', '-u', reference_unsorted, unsorted_file]) + except subprocess.CalledProcessError as e: + error_output += e.output + + if len(error_output) > 0: + self.logger.error('Difference in yml:\n' + error_output) + return True