PR#871: [WIP] Add MMDResolver to resolve deps between modules using libsolv and libmodulemd. - fm-orchestrator

fm-orchestrator

#871 [WIP] Add MMDResolver to resolve deps between modules using libsolv and libmodulemd.

Closed 6 years ago by ignatenkobrain. Opened 6 years ago by jkaluza.

jkaluza/fm-orchestrator libsolv into master

Add MMDResolver to resolve deps between modules using libsolv and libmodulemd.

Jan Kaluza • 6 years ago

e522a0b

module_build_service/mmd_resolver.py

file added

+205

		`@@ -0,0 +1,205 @@`
		`+ # -- coding: utf-8 --`
		`+ #`
		`+ # Copyright (c) 2018 Red Hat, Inc.`
		`+ #`
		`+ # Permission is hereby granted, free of charge, to any person obtaining a copy`
		`+ # of this software and associated documentation files (the "Software"), to deal`
		`+ # in the Software without restriction, including without limitation the rights`
		`+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell`
		`+ # copies of the Software, and to permit persons to whom the Software is`
		`+ # furnished to do so, subject to the following conditions:`
		`+ #`
		`+ # The above copyright notice and this permission notice shall be included in all`
		`+ # copies or substantial portions of the Software.`
		`+ #`
		`+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR`
		`+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,`
		`+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE`
		`+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER`
		`+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,`
		`+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`+ # SOFTWARE.`
		`+ #`
		`+ # Written by Jan Kaluza <jkaluza@redhat.com>`
		`+`
		`+ import solv`
		`+ from module_build_service import log, conf`
		`+ import itertools`
		`+`
		`+ class MMDResolver(object):`
		`+ """`
		`+ Resolves dependencies between Module metadata objects.`
		`+ """`
		`+`
		`+ def __init__(self):`
		`+ self.pool = solv.Pool()`
		`+ self.pool.setarch("x86_64")`
ignatenkobrain commented 6 years ago sounds like a workaround ;) You probably need to check all combinations against all arches separately. But let's leave this for now.
		`+ self.build_repo = self.pool.add_repo("build")`
		`+ self.available_repo = self.pool.add_repo("available")`
		`+`
		`+ self.solvables_per_name = {}`
		`+ self.alternatives_whitelist = set()`
		`+`
		`+ def _create_solvable(self, repo, mmd):`
		`+ """`
		+ Creates libsolv Solvable object in repo `repo` based on the Modulemd
		+ metadata `mmd`.
		`+`
		`+ This fills in all the provides/requires/conflicts of Solvable.`
		`+`
		`+ :rtype: solv.Solvable`
		`+ :return: Solvable object.`
		`+ """`
		`+ solvable = repo.add_solvable()`
		`+ solvable.name = "%s:%s:%d:%s" % (mmd.get_name(), mmd.get_stream(),`
mprahl commented 6 years ago Bike shedding: You could replace this with a join instead. For instance: solvable.name = ':'.join(mmd.get_name(), mmd.get_stream(), mmd.get_version(), mmd.get_context())
mprahl commented 6 years ago The context won't be set on the modulemd we are trying to solve yet when this is called since the context is done from the resulting buildrequires and requires we end up allowing.
		`+ mmd.get_version(), mmd.get_context())`
		`+ solvable.evr = "%s-%d" % (mmd.get_stream(), mmd.get_version())`
ignatenkobrain commented 6 years ago after some thinking, I would go with just version in evr (without stream) so it won't crack through comparing streams
mprahl commented 6 years ago @ignatenkobrain but a version by itself doesn't determine if a module is newer or not, you need the stream as well. What are the repercussions/benefits if stream is dropped from the evr?
ignatenkobrain commented 6 years ago stream is in the name. whatever is in version must be comparable..
		`+ solvable.arch = "x86_64"`
		`+`
		`+ # Provides`
		`+ solvable.add_provides(`
		`+ self.pool.Dep("module(%s)" % mmd.get_name()).Rel(`
		`+ solv.REL_EQ, self.pool.Dep(solvable.evr)))`
		`+ solvable.add_provides(`
		`+ self.pool.Dep("module(%s)" % solvable.name).Rel(`
		`+ solv.REL_EQ, self.pool.Dep(solvable.evr)))`
		`+`
		`+ # Requires`
		`+ for deps in mmd.get_dependencies():`
mprahl commented 6 years ago The modulemd spec says "Each list item describes a combination of dependencies this module can be built or run against". That means there could be repeat dependencies in each list item. Will this code handle that?
ignatenkobrain commented 6 years ago it won't, we should create multiple solvables.
		`+ for name, streams in deps.get_requires().items():`
		`+ requires = None`
		`+ for stream in streams.get():`
		`+ require = self.pool.Dep("module(%s)" % name)`
		`+ require = require.Rel(solv.REL_EQ, self.pool.Dep(stream))`
mprahl commented 6 years ago I don't understand this line. In particular, why is the relationship done with the stream? Can you please explain it?
mprahl commented 6 years ago Ah I see, so you are saying that the module depends on this build dependency of a specific stream.
		`+ if requires:`
		`+ requires = requires.Rel(solv.REL_OR, require)`
		`+ else:`
		`+ requires = require`
		`+ solvable.add_requires(requires)`
		`+`
		`+ # Build-requires in case we are in build_repo.`
		`+ if repo == self.build_repo:`
mprahl commented 6 years ago Seems like most of the code is the same as the requires loop. You could likely avoid duplicating the code by using a for loop around this and dynamically determine the function to call.
		`+ for deps in mmd.get_dependencies():`
		`+ for name, streams in deps.get_buildrequires().items():`
		`+ requires = None`
		`+ for stream in streams.get():`
		`+ require = self.pool.Dep("module(%s)" % name)`
		`+ require = require.Rel(solv.REL_EQ, self.pool.Dep(stream))`
		`+ if requires:`
		`+ requires = requires.Rel(solv.REL_OR, require)`
ignatenkobrain commented 6 years ago this part seems wrong. does it req `(A or A)`?
mprahl commented 6 years ago What part seems wrong here? I don't think it is "A" or "A" since we assume each stream in the list is unique. Can you further explain?
		`+ else:`
		`+ requires = require`
		`+ solvable.add_requires(requires)`
		`+ self.alternatives_whitelist.add(name)`
		`+`
		`+ # Conflicts`
		`+ if mmd.get_name() not in self.solvables_per_name:`
		`+ self.solvables_per_name[mmd.get_name()] = []`
		`+ for other_solvable in self.solvables_per_name[mmd.get_name()]:`
		`+ other_solvable.add_conflicts(`
		`+ self.pool.Dep("module(%s)" % solvable.name).Rel(`
		`+ solv.REL_EQ, self.pool.Dep(solvable.evr)))`
		`+ solvable.add_conflicts(`
		`+ self.pool.Dep("module(%s)" % other_solvable.name).Rel(`
ignatenkobrain commented 6 years ago no, just `module($name)`
		`+ solv.REL_EQ, self.pool.Dep(other_solvable.evr)))`
		`+ self.solvables_per_name[mmd.get_name()].append(solvable)`
		`+`
		`+ return solvable`
		`+`
		`+ def add_available_module(self, mmd):`
		`+ """`
		`+ Adds module available for dependency solving.`
		`+ """`
		`+ self._create_solvable(self.available_repo, mmd)`
		`+`
		`+ def _solve(self, module_name, alternative_with=None):`
		`+ """`
		+ Solves the dependencies of module `module_name`. If there is an
		`+ alternative solution to dependency solving, it will prefer the one`
		+ which brings in the package in `alternative_with` list (if set).
		`+`
		`+ :rtype: solv.Solver`
		`+ :return: Solver object with dependencies resolved.`
		`+ """`
		`+ solver = self.pool.Solver()`
		`+ # Try to select the module we are interested in.`
		`+ flags = solv.Selection.SELECTION_PROVIDES`
		`+ sel = self.pool.select("module(%s)" % module_name, flags)`
		`+ if sel.isempty():`
		`+ raise ValueError(`
		`+ "Cannot find module %s while resolving "`
		`+ "dependencies" % module_name)`
		`+ # Prepare the job including the solution for problems from previous calls.`
		`+ jobs = sel.jobs(solv.Job.SOLVER_INSTALL)`
		`+`
		`+ if alternative_with:`
		`+ for name in alternative_with:`
		`+ sel = self.pool.select("module(%s)" % name, flags)`
		`+ if sel.isempty():`
		`+ raise ValueError(`
		`+ "Cannot find module %s while resolving "`
		`+ "dependencies" % name)`
		`+ jobs += sel.jobs(solv.Job.SOLVER_FAVOR)`
		`+ # Try to solve the dependencies.`
		`+ problems = solver.solve(jobs)`
		`+ # In case we have some problems, return early here with the problems.`
		`+ if len(problems) != 0:`
		`+ # TODO: Add more info.`
		`+ raise ValueError(`
		`+ "Dependencies between modules are not satisfied")`
		`+ return solver`
		`+`
		`+ def _solve_recurse(self, solvable, alternatives=None, alternatives_tried=None):`
		`+ """`
		+ Solves dependencies of module defined by `solvable` object and all its
		`+ alternatives recursively.`
		`+`
		`+ :return: set of frozensets of n:s:v:c of modules which satisfied the`
		`+ dependency solving.`
		`+ """`
		`+ if not alternatives:`
		`+ alternatives = set()`
		`+ if not alternatives_tried:`
		`+ alternatives_tried = set()`
		`+`
		`+ solver = self._solve(solvable.name, alternatives)`
		`+ if not solver:`
		`+ return set([])`
		`+`
		`+ ret = set([])`
		`+ ret.add(`
		`+ frozenset([s.name for s in solver.transaction().newsolvables()`
		`+ if s.name != solvable.name]))`
		`+`
		`+ choices = []`
		`+ for alt in solver.all_alternatives():`
		`+ l = []`
		`+ for alt_choice in alt.choices():`
		`+ if alt_choice.name.split(":")[0] in self.alternatives_whitelist:`
		`+ l.append(alt_choice.name)`
		`+ if l:`
		`+ choices.append(l)`
		`+`
		`+ choices_combinations = list(itertools.product(*choices))`
		`+ for choices_combination in choices_combinations:`
		`+ if choices_combination not in alternatives_tried:`
		`+ alternatives_tried.add(choices_combination)`
		`+ ret = ret.union(self._solve_recurse(`
		`+ solvable, choices_combination, alternatives_tried))`
		`+`
		`+ return ret`
		`+`
		`+ def solve(self, mmd):`
		`+ """`
		+ Solves dependencies of module defined by `mmd` object. Returns set
		`+ containing frozensets with all the possible combinations which`
		`+ satisfied dependencies.`
		`+`
		`+ :return: set of frozensets of n:s:v:c of modules which satisfied the`
		`+ dependency solving.`
		`+ """`
		`+ solvable = self._create_solvable(self.build_repo, mmd)`
		`+ self.pool.createwhatprovides()`
		`+`
		`+ alternatives = self._solve_recurse(solvable)`
		`+ return alternatives`

tests/test_mmd_resolver.py

file added

+144

		`@@ -0,0 +1,144 @@`
		`+ # Copyright (c) 2017 Red Hat, Inc.`
		`+ #`
		`+ # Permission is hereby granted, free of charge, to any person obtaining a copy`
		`+ # of this software and associated documentation files (the "Software"), to deal`
		`+ # in the Software without restriction, including without limitation the rights`
		`+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell`
		`+ # copies of the Software, and to permit persons to whom the Software is`
		`+ # furnished to do so, subject to the following conditions:`
		`+ #`
		`+ # The above copyright notice and this permission notice shall be included in all`
		`+ # copies or substantial portions of the Software.`
		`+ #`
		`+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR`
		`+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,`
		`+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE`
		`+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER`
		`+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,`
		`+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`+ # SOFTWARE.`
		`+ #`
		`+ # Written by Ralph Bean <rbean@redhat.com>`
		`+`
		`+ import gi`
		`+ gi.require_version('Modulemd', '1.0') # noqa`
		`+ from gi.repository import Modulemd`
		`+`
		`+ import pytest`
		`+ from mock import patch`
		`+`
		`+ from module_build_service.mmd_resolver import MMDResolver`
		`+`
		`+`
		`+ class TestMMDResolver:`
		`+`
		`+ def setup_method(self, test_method):`
		`+ self.mmd_resolver = MMDResolver()`
		`+`
		`+ def teardown_method(self, test_method):`
		`+ pass`
		`+`
		`+ def _make_mmd(self, nsvc, requires, build_requires):`
		`+ name, stream, version, context = nsvc.split(":")`
		`+ mmd = Modulemd.Module()`
		`+ mmd.set_mdversion(2)`
		`+ mmd.set_name(name)`
		`+ mmd.set_stream(stream)`
		`+ mmd.set_version(int(version))`
		`+ mmd.set_context(context)`
		`+ mmd.set_summary("foo")`
		`+ mmd.set_description("foo")`
		`+ licenses = Modulemd.SimpleSet()`
		`+ licenses.add("GPL")`
		`+ mmd.set_module_licenses(licenses)`
		`+`
		`+ deps = Modulemd.Dependencies()`
		`+ for req_name, req_streams in requires.items():`
		`+ deps.add_requires(req_name, req_streams)`
		`+ for req_name, req_streams in build_requires.items():`
		`+ deps.add_buildrequires(req_name, req_streams)`
		`+ mmd.set_dependencies((deps, ))`
		`+ return mmd`
		`+`
		`+ def test_solve_tree(self):`
		`+ mmds = []`
		`+ mmds.append(self._make_mmd("app:1:0:c1", {}, {"gtk": ["1", "2"]}))`
		`+ mmds.append(self._make_mmd("gtk:1:0:c2", {"font": ["a", "b"], "platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:1:0:c3", {"font": ["a", "b"], "platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:2:0:c4", {"font": ["a", "b"], "platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:2:0:c5", {"font": ["a", "b"], "platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("font:a:0:c6", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("font:a:0:c7", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("font:b:0:c8", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("font:b:0:c9", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("platform:f28:0:c10", {}, {}))`
		`+ mmds.append(self._make_mmd("platform:f29:0:c11", {}, {}))`
		`+`
		`+ for mmd in mmds[1:]:`
		`+ self.mmd_resolver.add_available_module(mmd)`
		`+ expanded = self.mmd_resolver.solve(mmds[0])`
		`+`
		`+ expected = set([`
		`+ frozenset(["gtk:1:0:c2", "platform:f28:0:c10", "font:b:0:c8"]),`
		`+ frozenset(["gtk:1:0:c3", "platform:f29:0:c11", "font:b:0:c9"]),`
		`+ frozenset(["gtk:2:0:c4", "platform:f28:0:c10", "font:b:0:c8"]),`
		`+ frozenset(["gtk:2:0:c5", "platform:f29:0:c11", "font:b:0:c9"]),`
		`+ ])`
		`+`
		`+ assert expanded == expected`
		`+`
		`+ def test_solve_tree_buildrequire_platform(self):`
		`+ mmds = []`
		`+ mmds.append(self._make_mmd("app:1:0:c1", {}, {"gtk": ["1", "2"], "platform": ["f28"]}))`
		`+ mmds.append(self._make_mmd("gtk:1:0:c2", {"font": ["a", "b"], "platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:1:0:c3", {"font": ["a", "b"], "platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:2:0:c4", {"font": ["a", "b"], "platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:2:0:c5", {"font": ["a", "b"], "platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("font:a:0:c6", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("font:a:0:c7", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("font:b:0:c8", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("font:b:0:c9", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("platform:f28:0:c10", {}, {}))`
		`+ mmds.append(self._make_mmd("platform:f29:0:c11", {}, {}))`
		`+`
		`+ for mmd in mmds[1:]:`
		`+ self.mmd_resolver.add_available_module(mmd)`
		`+ expanded = self.mmd_resolver.solve(mmds[0])`
		`+`
		`+ expected = set([`
		`+ frozenset(["gtk:1:0:c2", "platform:f28:0:c10", "font:b:0:c8"]),`
		`+ frozenset(["gtk:2:0:c4", "platform:f28:0:c10", "font:b:0:c8"]),`
		`+ ])`
		`+`
		`+ assert expanded == expected`
		`+`
		`+ def test_solve_tree_multiple_build_requires(self):`
		`+ mmds = []`
		`+ mmds.append(self._make_mmd("app:1:0:c1", {}, {"gtk": ["1", "2"], "foo": ["1", "2"]}))`
		`+ mmds.append(self._make_mmd("gtk:1:0:c2", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:1:0:c3", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:2:0:c4", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("gtk:2:0:c5", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("foo:1:0:c2", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("foo:1:0:c3", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("foo:2:0:c4", {"platform": ["f28"]}, {}))`
		`+ mmds.append(self._make_mmd("foo:2:0:c5", {"platform": ["f29"]}, {}))`
		`+ mmds.append(self._make_mmd("platform:f28:0:c10", {}, {}))`
		`+ mmds.append(self._make_mmd("platform:f29:0:c11", {}, {}))`
		`+`
		`+ for mmd in mmds[1:]:`
		`+ self.mmd_resolver.add_available_module(mmd)`
		`+ expanded = self.mmd_resolver.solve(mmds[0])`
		`+`
		`+ expected = set([`
		`+ frozenset(['foo:2:0:c5', 'gtk:1:0:c3', 'platform:f29:0:c11']),`
		`+ frozenset(['foo:2:0:c4', 'gtk:2:0:c4', 'platform:f28:0:c10']),`
		`+ frozenset(['foo:1:0:c2', 'gtk:2:0:c4', 'platform:f28:0:c10']),`
		`+ frozenset(['foo:2:0:c5', 'gtk:2:0:c5', 'platform:f29:0:c11']),`
		`+ frozenset(['foo:1:0:c3', 'gtk:2:0:c5', 'platform:f29:0:c11']),`
		`+ frozenset(['foo:1:0:c2', 'gtk:1:0:c2', 'platform:f28:0:c10']),`
		`+ frozenset(['foo:2:0:c4', 'gtk:1:0:c2', 'platform:f28:0:c10']),`
		`+ frozenset(['foo:1:0:c3', 'gtk:1:0:c3', 'platform:f29:0:c11'])`
		`+ ])`
		`+`
		`+ assert expanded == expected`

tox.ini

file modified

		`@@ -11,6 +11,7 @@`

		`[testenv]`
		`usedevelop = true`
		`+ sitepackages = true`
		`deps = -r{toxinidir}/test-requirements.txt`
		`commands =`
		`py.test -v {posargs}`

no initial comment

ignatenkobrain commented on line 37 of module_build_service/mmd_resolver.py 6 years ago

sounds like a workaround ;) You probably need to check all combinations against all arches separately. But let's leave this for now.

ignatenkobrain commented on line 57 of module_build_service/mmd_resolver.py 6 years ago

after some thinking, I would go with just version in evr (without stream) so it won't crack through comparing streams

ignatenkobrain commented on line 104 of module_build_service/mmd_resolver.py 6 years ago

no, just module($name)

ignatenkobrain commented on line 90 of module_build_service/mmd_resolver.py 6 years ago

this part seems wrong. does it req (A or A)?

mprahl commented on line 55 of module_build_service/mmd_resolver.py 6 years ago

Bike shedding: You could replace this with a join instead.

For instance:

solvable.name = ':'.join(mmd.get_name(), mmd.get_stream(), mmd.get_version(), mmd.get_context())

mprahl commented on line 69 of module_build_service/mmd_resolver.py 6 years ago

The modulemd spec says "Each list item describes a combination of dependencies this module can be built or run against". That means there could be repeat dependencies in each list item. Will this code handle that?

mprahl commented on line 57 of module_build_service/mmd_resolver.py 6 years ago

@ignatenkobrain but a version by itself doesn't determine if a module is newer or not, you need the stream as well. What are the repercussions/benefits if stream is dropped from the evr?

mprahl commented on line 82 of module_build_service/mmd_resolver.py 6 years ago

Seems like most of the code is the same as the requires loop. You could likely avoid duplicating the code by using a for loop around this and dynamically determine the function to call.

mprahl commented on line 74 of module_build_service/mmd_resolver.py 6 years ago

I don't understand this line. In particular, why is the relationship done with the stream? Can you please explain it?

mprahl commented on line 74 of module_build_service/mmd_resolver.py 6 years ago

Ah I see, so you are saying that the module depends on this build dependency of a specific stream.

mprahl commented on line 90 of module_build_service/mmd_resolver.py 6 years ago

What part seems wrong here? I don't think it is "A" or "A" since we assume each stream in the list is unique. Can you further explain?

mprahl commented on line 55 of module_build_service/mmd_resolver.py 6 years ago

The context won't be set on the modulemd we are trying to solve yet when this is called since the context is done from the resulting buildrequires and requires we end up allowing.

mprahl commented 6 years ago

Nice job on this @jkaluza!