Differential D260

Adding input rpm to solvpool repos to fix codependency errors
ClosedPublic
Actions

Authored by tflink on Oct 29 2014, 3:11 PM.

Details

Reviewers
jdulaney
jskladan
Maniphest Tasks
T366: depcheck is failing with strange "none of the providers can be installed" for a recent kernel update
Commits
rDEPCKdc4f42723ee9: Restructuring depcheck to add all incoming rpms to solvpool. Fixes T366
Summary

Restructuring depcheck to add all incoming rpms to solvpool. Fixes T366

Also added a '--workdir' to depcheck to facilitate local running and made some small changes to the functest code:

  • only pick up "*.yaml" or "*.yml" files (kept picking up .swp files from vim)
  • ignore a 'known_fail' directory in the scenarios
  • updated depcheck_scenarios to a new commit with new scenarios

A unit test was removed because it is no longer relevant - with this patch, it is not possible for an rpm to be run more than once and thus, the exception couldn't be raised

Test Plan

unit tests, functional tests and working with a re-creation of the repos that triggered the original ticket (now passes every time)

Diff Detail

Repository
rDEPCK task-depcheck
Lint
Lint Skipped
Unit
Unit Tests Skipped
tflink added a task: T366: depcheck is failing with strange "none of the providers can be installed" for a recent kernel update.Oct 29 2014, 3:11 PM
tflink retitled this revision from to Adding input rpm to solvpool repos to fix codependency errors.
tflink updated this object.
tflink edited the test plan for this revision. (Show Details)
tflink added reviewers: jskladan, jdulaney.
tflink added a subscriber: kparal.
tflink updated this revision to Diff 727.Oct 29 2014, 3:14 PM
  • removing a note in comments that shouldn't have been part of the review
tflink planned changes to this revision.Oct 29 2014, 3:31 PM

bah, made some sloppy mistakes that need small changes, another revision is incoming shortly

tflink updated this revision to Diff 728.Oct 29 2014, 3:33 PM
  • removing stray rpm reordering and fixed logging messages
tflink updated this revision to Diff 729.Oct 29 2014, 3:34 PM
  • removing commented out line
tflink added a comment.Oct 29 2014, 3:35 PM

sorry for all the changes - apparently it's time to get more coffee :-/

jskladan accepted this revision.Oct 31 2014, 8:18 AM

Overall, this looks good.

depcheck/depcheck.py
137–138

Maybe this could use some comment? I guess that it fills the solvrepo with rpms, and stores some kind of 'reference' to which rpm 'should be installed/checked' to the solvable, so in the end, the solvrepo contains all rpms, but you can still run 'try install xyz' per rpm?

This has probably a lot to do with /me not knowing how libsolv works, so if it is obvious to those who do, disregard this ;)

This revision is now accepted and ready to land.Oct 31 2014, 8:18 AM
tflink added inline comments.Oct 31 2014, 3:46 PM
depcheck/depcheck.py
137–138

The code should be understandable without an understanding of libsolv - I'll add some comments to better explain what's going on

jdulaney accepted this revision.Oct 31 2014, 4:36 PM

Looks good here; I tested against a known to fail rpm set and it passed.

Joseph, the problem is in the way it was populating the libsolv solve pool (essentially, the way the set of rpms to be tested is passed to libsolv).

tflink closed this revision.Nov 3 2014, 5:11 PM
tflink updated this revision to Diff 730.

Closed by commit rDEPCKdc4f42723ee9 (authored by @tflink).

Revision Contents

Diff 730

View Options

depcheck/__init__.py

Show First 20 Lines • Show All 85 Lines • ▼ Show 20 Line(s)65def run(rpms, arch, repos, report_format="updates", workdir=None):
86 if arch == 'armv7l': 86​ if arch == 'armv7l':
87 arch = 'armhfp' 87​ arch = 'armhfp'
88 88​
89 # fix arch for libsolv 89​ # fix arch for libsolv
90 if arch == 'i386': 90​ if arch == 'i386':
91 arch = 'i686' 91​ arch = 'i686'
92 92​
93 # handle data formatting for repo metadata (list of dicts) 93​ # handle data formatting for repo metadata (list of dicts)
94 log.logger.debug("Prepairing metadata download") 94​ log.logger.debug("Preparing metadata download")
95 handled_repos = {} 95​ handled_repos = {}
96 for repo, path in repos.items(): 96​ for repo, path in repos.items():
97 log.logger.debug(" - %r: %r, %r", repo, arch, path) 97​ log.logger.debug(" - %r: %r, %r", repo, arch, path)
98 handled_repos[repo] = {'arch': arch, 'path': path} 98​ handled_repos[repo] = {'arch': arch, 'path': path}
99 99​
100 if workdir is None: 100​ if workdir is None:
101 workdir = tempfile.mkdtemp('', 'depcheck-', '/var/tmp/') 101​ workdir = tempfile.mkdtemp('', 'depcheck-', '/var/tmp/')
102 else: 102​ else:
Show All 35 Lines131def depcheck_cli():
138 parser.add_argument("-r", "--repo", action="append", dest="repos", help=""" 138​ parser.add_argument("-r", "--repo", action="append", dest="repos", help="""
139 Repo to be tested against. Repo can be local (in the form /dir/of/repo) 139​ Repo to be tested against. Repo can be local (in the form /dir/of/repo)
140 or remote (direct link or standard metalink). 140​ or remote (direct link or standard metalink).
141 Can be used multiple times, to specify more repos. 141​ Can be used multiple times, to specify more repos.
142 """) 142​ """)
143 parser.add_argument("-f", "--format", choices=["rpms", "updates"], help=""" 143​ parser.add_argument("-f", "--format", choices=["rpms", "updates"], help="""
144 Specify output format to override configuration. 144​ Specify output format to override configuration.
145 """) 145​ """)
146 146​ parser.add_argument("-w", "--workdir", help="workdir to use for depcheck")
147 147​
148 args = parser.parse_args() 148​ args = parser.parse_args()
149 # autodetect arch if none is passed in 149​ # autodetect arch if none is passed in
150 arch = args.arch 150​ arch = args.arch
151 if arch is None: 151​ if arch is None:
152 arch = platform.machine() 152​ arch = platform.machine()
153 153​
154 154​
155 repos = dict([(repo, repo) for repo in args.repos]) 155​ repos = dict([(repo, repo) for repo in args.repos])
156 156​
157 try: 157​ try:
158 print run(args.rpm, arch, repos, args.format) 158​ print run(args.rpm, arch, repos, args.format, args.workdir)
159 except KeyboardInterrupt, e: 159​ except KeyboardInterrupt, e:
160 print >> sys.stderr, ("\n\nExiting on user cancel.") 160​ print >> sys.stderr, ("\n\nExiting on user cancel.")
161 sys.exit(1) 161​ sys.exit(1)
162 162​
163 163​
View Options

depcheck/depcheck.py

Show All 24 Lines
25# 25​#
26# EPEL dep: python-argparse 26​# EPEL dep: python-argparse
27 27​
28 28​
29import solv 29​import solv
30import tempfile 30​import tempfile
31 31​
32from . import Pass, Fail 32​from . import Pass, Fail
33from . import exc ​
34from . import log 33​from . import log
35 34​
36 35​
37class Depcheck(object): 36​class Depcheck(object):
38 def __init__(self, arch, repodata, workdir=None): 37​ def __init__(self, arch, repodata, workdir=None):
39 """Depcheck uses existing repodata to check a set of rpm files for 38​ """Depcheck uses existing repodata to check a set of rpm files for
40 depsolving issues. 39​ depsolving issues.
41 40​
Show All 30 Lines70 if self.solvrepo_object is None:
72 self.solvrepo_object = self.solvpool.add_repo(self.arch) 71​ self.solvrepo_object = self.solvpool.add_repo(self.arch)
73 return self.solvrepo_object 72​ return self.solvrepo_object
74 73​
75 def _prepare_libsolv(self): 74​ def _prepare_libsolv(self):
76 """Initialize libsolv and add the needed repository information for 75​ """Initialize libsolv and add the needed repository information for
77 checking dependencies for the supplied repositories 76​ checking dependencies for the supplied repositories
78 """ 77​ """
79 78​
79​ log.logger.debug('preparing libsolv repos for depcheck run')
80​
80 for repo in self.repos: 81​ for repo in self.repos:
81 primary = open(self.repos[repo]['primary'], 'rb') 82​ primary = open(self.repos[repo]['primary'], 'rb')
82 filelist = open(self.repos[repo]['filelists'], 'rb') 83​ filelist = open(self.repos[repo]['filelists'], 'rb')
83 84​
84 self.solvrepo.add_rpmmd(primary, None) 85​ self.solvrepo.add_rpmmd(primary, None)
85 self.solvrepo.add_rpmmd(filelist, None, 86​ self.solvrepo.add_rpmmd(filelist, None,
86 solv.Repo.REPO_EXTEND_SOLVABLES) 87​ solv.Repo.REPO_EXTEND_SOLVABLES)
87 88​
88 def _check_rpm(self, rpmfile): 89​ def _check_rpm(self, rpmfile, solvable):
89 """Check a single rpm file against prepared repodata 90​ """Check a single rpm file against prepared repodata
90 :param rpmfile: filename of rpm file to check 91​ :param rpmfile: filename of rpm file to check
91 :type rpmfile: str 92​ :type rpmfile: str
93​ :param solvable: libsolv solvable from solvpool for the rpm to check
94​ :type solvable: libsolv.Solver.Solvable
92 """ 95​ """
93 solvable = self.solvrepo.add_rpm(rpmfile) ​
94 self.solvpool.addfileprovides() ​
95 self.solvpool.createwhatprovides() ​
96 96​
97​ # the job setup here is mostly taken from libsolv's installcheck tool
98​ # create a job using the predetermined solvable in the repo
97 job = self.solvpool.Job(solv.Job.SOLVER_INSTALL | solv.Job.SOLVER_SOLVABLE, 99​ job = self.solvpool.Job(solv.Job.SOLVER_INSTALL | solv.Job.SOLVER_SOLVABLE,
98 solvable.id) 100​ solvable.id)
101​ self.solvpool.set_flag(solv.Solver.SOLVER_FLAG_IGNORE_RECOMMENDED, 1)
99 102​
100 # important note - make sure to keep the solver in the same scope as 103​ # solve the deps for the input solvable's rpm and look for problems
101 # the problem analysis code or else python-solv segfults will ensue 104​ solver = self.solvpool.Solver()
102 solvpool_solver = self.solvpool.Solver() 105​ solv_problems = solver.solve([job])
103 solv_problems = solvpool_solver.solve([job]) ​
104 106​
105 if len(solv_problems) == 0: 107​ if len(solv_problems) == 0:
106 return Pass, [] 108​ return {'result': Pass,'details':[]}
107 ​
108 else: 109​ else:
109 problem_strs = [] 110​ problem_strs = []
110 for problem in solv_problems: 111​ for problem in solv_problems:
111 probrules = problem.findallproblemrules() 112​ probrules = problem.findallproblemrules()
112 ​
113 for rule in probrules: 113​ for rule in probrules:
114 ruleinfos = rule.allinfos() 114​ ruleinfos = rule.allinfos()
115 for info in ruleinfos: 115​ for info in ruleinfos:
116 problem_strs.append(info.problemstr()) 116​ problem_strs.append(info.problemstr())
117​ return {'result':Fail, 'details':problem_strs}
117 118​
118 return Fail, problem_strs ​
119 119​
120 def check_rpm_deps(self, rpmfiles): 120​ def check_rpm_deps(self, rpmfiles):
121 """Check dependencies of the given rpms against supplied repodata 121​ """Check dependencies of the given rpms against supplied repodata
122 122​
123 :param rpmfiles: local absolute paths to rpms to check 123​ :param rpmfiles: local absolute paths to rpms to check
124 :type rpmfiles: list of str 124​ :type rpmfiles: list of str
125 :returns: dictionary of results {rpm: {'results': Pass/Fail, 'details': []}} 125​ :returns: dictionary of results {rpm: {'results': Pass/Fail, 'details': []}}
126 """ 126​ """
127 127​
128 self._prepare_libsolv() 128​ log.logger.debug('adding all rpms to libsolv repos and preparing solvpool')
129 129​
130 run_result = {} ​
131 130​
131​ # add all rpmfiles that we're checking to the libsolv repo and save the
132​ # solvable that is assigned to them.
133​ # this makes libsolv aware of all the incoming rpms and allows it to
134​ # check for any dep breakage between the rpms
135​ self._prepare_libsolv()
136​ solvables = {}
132 for rpmfile in rpmfiles: 137​ for rpmfile in rpmfiles:
133 log.logger.debug('running depcheck for %s' % rpmfile) 138​ rpmname = rpmfile.split('/')[-1]
jskladanUnsubmitted
Not Done

Maybe this could use some comment? I guess that it fills the solvrepo with rpms, and stores some kind of 'reference' to which rpm 'should be installed/checked' to the solvable, so in the end, the solvrepo contains all rpms, but you can still run 'try install xyz' per rpm?

This has probably a lot to do with /me not knowing how libsolv works, so if it is obvious to those who do, disregard this ;)

tflinkAuthorUnsubmitted
Not Done

The code should be understandable without an understanding of libsolv - I'll add some comments to better explain what's going on

134 result_state, result_details = self._check_rpm(rpmfile) 139​ solvable = self.solvrepo.add_rpm(rpmfile)
140​ solvables[rpmname] = solvable
135 141​
136 if rpmfile in run_result: 142​ # now that the rpms have been added, prepare the solvpool
137 if result_state != run_result[rpmfile]['result']: 143​ self.solvpool.addfileprovides()
138 #TODO maybe do something saner, ideas welcomed 144​ self.solvpool.createwhatprovides()
139 raise exc.DepcheckException("RPM %r has multiple conflicting results" % rpmfile) ​
140 else: ​
141 run_result[rpmfile]['details'].append(result_details) ​
142 145​
143 else: ​
144 run_result[rpmfile] = {'result': result_state, ​
145 'details': result_details} ​
146 146​
147 return run_result 147​ # check each rpm using its solvable that was assigned while adding to
148​ # the repo
149​ results = {}
150​ for rpmname in solvables:
151​ log.logger.debug('running depcheck for %s' % rpmfile)
152​ run_result = self._check_rpm(rpmname, solvables[rpmname])
153​ results[rpmname] = run_result
148 154​
155​ return results
149 156​
150 157​
View Options

depcheck/squash_results.py

1import os ​
2import json 1​import json
3 2​
4from . import Pass, Fail 3​from . import Pass, Fail
5from . import log 4​from . import log
6 5​
7from libtaskotron.koji_utils import KojiClient 6​from libtaskotron.koji_utils import KojiClient
8from libtaskotron.bodhi_utils import BodhiUtils 7​from libtaskotron.bodhi_utils import BodhiUtils
9from libtaskotron import check 8​from libtaskotron import check
10from libtaskotron.rpm_utils import rpmformat ​
11 9​
12def _squash_rpms_to_builds(run_result, koji_client = None): 10​def _squash_rpms_to_builds(run_result, koji_client = None):
13 """ 11​ """
14 Converts the per-rpm results created by check_rpm_deps into per-build. 12​ Converts the per-rpm results created by check_rpm_deps into per-build.
15 :param run_result: output of the check_rpm_deps method 13​ :param run_result: output of the check_rpm_deps method
16 :type run_result: dict 14​ :type run_result: dict
17 :returns: dict containing per-nvr results 15​ :returns: dict containing per-nvr results
18 """ 16​ """
▲ Show 20 Lines • Show All 120 Lines • Show Last 20 Lines
View Options

tests/depcheck_scenarios

1Subproject commit f1e5d57b0e68c53bd05c1b4d6eab3078706d269e 1​Subproject commit 3ad160ad3145dc4595286291e5e43e2d91737ea9
View Options

tests/functest_depcheck_scenarios.py

Show All 30 Lines24def get_depcheck_scenarios():
31 yamlfiles = [] 31​ yamlfiles = []
32 for scenario_type in ['pass', 'fail']: 32​ for scenario_type in ['pass', 'fail']:
33 scenario_path = os.path.join(os.path.dirname(__file__), 33​ scenario_path = os.path.join(os.path.dirname(__file__),
34 'depcheck_scenarios/scenarios/%s' % scenario_type) 34​ 'depcheck_scenarios/scenarios/%s' % scenario_type)
35 dircontents = os.walk(scenario_path) 35​ dircontents = os.walk(scenario_path)
36 for dircontent in dircontents: 36​ for dircontent in dircontents:
37 if len(dircontent[2]) == 0: 37​ if len(dircontent[2]) == 0:
38 continue 38​ continue
39​ if dircontent[1] == 'known_broken':
40​ continue
39 for infile in dircontent[2]: 41​ for infile in dircontent[2]:
40 filename = '%s/%s' % (dircontent[0], infile) 42​ filename = '%s/%s' % (dircontent[0], infile)
41 yamlfiles.append((scenario_type == 'pass', filename)) 43​ if infile.split('.')[-1] in ['yml', 'yaml']:
44​ yamlfiles.append((scenario_type == 'pass', filename))
42 return yamlfiles 45​ return yamlfiles
43 46​
44 47​
45@pytest.fixture(params=get_depcheck_scenarios()) 48​@pytest.fixture(params=get_depcheck_scenarios())
46def depcheck_scenario(request): 49​def depcheck_scenario(request):
47 return request.param 50​ return request.param
48 51​
49 52​
Show All 39 Lines
View Options

tests/test_depcheck.py

Show All 30 Lines30 def test_check_rpm_deps_output(self, monkeypatch):
31 """ 31​ """
32 Tests whether rpm_deps provides correct output for a given input. 32​ Tests whether rpm_deps provides correct output for a given input.
33 """ 33​ """
34 34​
35 test_depcheck = depcheck.Depcheck(None, None, "fakedir") 35​ test_depcheck = depcheck.Depcheck(None, None, "fakedir")
36 monkeypatch.setattr(test_depcheck, '_prepare_libsolv', Dingus()) 36​ monkeypatch.setattr(test_depcheck, '_prepare_libsolv', Dingus())
37 37​
38 rpmfiles = ['rpm_1', 'rpm_2'] 38​ rpmfiles = ['rpm_1', 'rpm_2']
39 def fake_check_rpm(rpmfile): 39​ def fake_check_rpm(rpmfile, solvable):
40 results = { 40​ results = {
41 rpmfiles[0]: (Pass, []), 41​ rpmfiles[0]: {'result':Pass, 'details':[]},
42 rpmfiles[1]: (Fail, ['message']), 42​ rpmfiles[1]: {'result':Fail, 'details':['message']},
43 } 43​ }
44 return results.get(rpmfile, (Pass, [])) 44​ return results.get(rpmfile, (Pass, []))
45 45​
46 monkeypatch.setattr(test_depcheck, '_check_rpm', fake_check_rpm) 46​ monkeypatch.setattr(test_depcheck, '_check_rpm', fake_check_rpm)
47 47​
48 outcome = test_depcheck.check_rpm_deps(rpmfiles) 48​ outcome = test_depcheck.check_rpm_deps(rpmfiles)
49 49​
50 assert len(outcome) == 2 50​ assert len(outcome) == 2
51 assert 'rpm_1' in outcome 51​ assert 'rpm_1' in outcome
52 assert 'rpm_2' in outcome 52​ assert 'rpm_2' in outcome
53 assert outcome['rpm_1']['result'] == Pass 53​ assert outcome['rpm_1']['result'] == Pass
54 assert outcome['rpm_2']['result'] == Fail 54​ assert outcome['rpm_2']['result'] == Fail
55 assert outcome['rpm_2']['details'] == ['message'] 55​ assert outcome['rpm_2']['details'] == ['message']
56 56​
57 def test_check_rpm_deps_conflicting_results(self, monkeypatch): ​
58 """ ​
59 Checks whether rpm_deps raises Exceptions when conflicting ​
60 results are provided for one RPM. ​
61 """ ​
62 ​
63 test_depcheck = depcheck.Depcheck(None, None, "fakedir") ​
64 monkeypatch.setattr(test_depcheck, '_prepare_libsolv', Dingus()) ​
65 ​
66 self.tmp_result = Pass ​
67 def fake_check_rpm(rpmfile): ​
68 if self.tmp_result == Pass: ​
69 self.tmp_result = Fail ​
70 else: ​
71 self.tmp_result = Pass ​
72 return (self.tmp_result, []) ​
73 ​
74 monkeypatch.setattr(test_depcheck, '_check_rpm', fake_check_rpm) ​
75 ​
76 with pytest.raises(exc.DepcheckException): ​
77 outcome = test_depcheck.check_rpm_deps(['rpm1', 'rpm1']) ​
78 ​