diff --git a/openstack_requirements/check.py b/openstack_requirements/check.py index 6e77663ff2..18deb66e3e 100644 --- a/openstack_requirements/check.py +++ b/openstack_requirements/check.py @@ -19,6 +19,9 @@ import collections from openstack_requirements import project from openstack_requirements import requirement +from packaging import markers +from packaging import specifiers + class RequirementsList(object): def __init__(self, name, project): @@ -181,3 +184,125 @@ def validate(head_reqs, branch_reqs, blacklist, global_reqs): ) return failed + + +def _find_constraint(req, constraints): + """Return the constraint matching the markers for req. + + Given a requirement, find the constraint with matching markers. + If none match, find a constraint without any markers at all. + Otherwise return None. + """ + if req.markers: + req_markers = markers.Marker(req.markers) + for constraint_setting, _ in constraints: + if constraint_setting.markers == req.markers: + return constraint_setting + # NOTE(dhellmann): This is a very naive attempt to check + # marker compatibility that relies on internal + # implementation details of the packaging library. The + # best way to ensure the constraint and requirements match + # is to use the same marker string in the corresponding + # lines. + c_markers = markers.Marker(constraint_setting.markers) + env = { + str(var): str(val) + for var, op, val in c_markers._markers # WARNING: internals + } + if req_markers.evaluate(env): + return constraint_setting + # Try looking for a constraint without any markers. + for constraint_setting, _ in constraints: + if not constraint_setting.markers: + return constraint_setting + return None + + +def validate_lower_constraints(req_list, constraints, blacklist): + """Return True if there is an error. + + :param reqs: RequirementsList for the head of the branch + :param constraints: Parsed lower-constraints.txt or None + + """ + if constraints is None: + return False + + parsed_constraints = requirement.parse(constraints) + + failed = False + + for fname, freqs in req_list.reqs_by_file.items(): + + if fname == 'doc/requirements.txt': + # Skip things that are not needed for unit or functional + # tests. + continue + + print("Validating lower constraints of {}".format(fname)) + + for name, reqs in freqs.items(): + + if name in blacklist: + continue + + if name not in parsed_constraints: + print('Package {!r} is used in {} ' + 'but not in lower-constraints.txt'.format( + name, fname)) + failed = True + continue + + for req in reqs: + spec = specifiers.SpecifierSet(req.specifiers) + # FIXME(dhellmann): This will only find constraints + # where the markers match the requirements list + # exactly, so we can't do things like use different + # constrained versions for different versions of + # python 3 if the requirement range is expressed as + # python_version>3.0. We can support different + # versions if there is a different requirement + # specification for each version of python. I don't + # really know how smart we want this to be, because + # I'm not sure we want to support extremely + # complicated dependency sets. + constraint_setting = _find_constraint( + req, + parsed_constraints[name], + ) + if not constraint_setting: + print('Unable to find constraint for {} ' + 'matching {!r} or without any markers.'.format( + name, req.markers)) + failed = True + continue + print('req', req) + print('constraint_setting', constraint_setting) + + version = constraint_setting.specifiers.lstrip('=') + + if not spec.contains(version): + print('Package {!r} is constrained to {} ' + 'which is incompatible with the settings {} ' + 'from {}.'.format( + name, version, req, fname)) + failed = True + + min = [ + s + for s in req.specifiers.split(',') + if '>' in s + ] + if not min: + # No minimum specified. Ignore this and let some + # other validation trap the error. + continue + + expected = min[0].lstrip('>=') + if version != expected: + print('Package {!r} is constrained to {} ' + 'which does not match ' + 'the minimum version specifier {} in {}'.format( + name, version, expected, fname)) + failed = True + return failed diff --git a/openstack_requirements/project.py b/openstack_requirements/project.py index 995aba5925..777d040313 100644 --- a/openstack_requirements/project.py +++ b/openstack_requirements/project.py @@ -132,13 +132,17 @@ def read(root): target_files = [ 'requirements.txt', 'tools/pip-requires', 'test-requirements.txt', 'tools/test-requires', - 'doc/requirements.txt' + 'doc/requirements.txt', ] for py_version in (2, 3): target_files.append('requirements-py%s.txt' % py_version) target_files.append('test-requirements-py%s.txt' % py_version) for target_file in target_files: _safe_read(result, target_file, output=requirements) + # Read lower-constraints.txt and ensure the key is always present + # in case the file is missing. + result['lower-constraints.txt'] = None + _safe_read(result, 'lower-constraints.txt') return result diff --git a/openstack_requirements/tests/test_check.py b/openstack_requirements/tests/test_check.py index fdbaf0f4e6..7e8d7ad7eb 100644 --- a/openstack_requirements/tests/test_check.py +++ b/openstack_requirements/tests/test_check.py @@ -396,3 +396,253 @@ class TestValidateOne(testtools.TestCase): global_reqs=global_reqs, ) ) + + +class TestValidateLowerConstraints(testtools.TestCase): + + def setUp(self): + super(TestValidateLowerConstraints, self).setUp() + self._stdout_fixture = fixtures.StringStream('stdout') + self.stdout = self.useFixture(self._stdout_fixture).stream + self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.stdout)) + + def test_no_constraints_file(self): + constraints_content = None + project_data = { + 'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_no_min(self): + constraints_content = textwrap.dedent(""" + name==1.2 + """) + project_data = { + 'requirements': {'requirements.txt': 'name!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_matches(self): + constraints_content = textwrap.dedent(""" + name==1.2 + """) + project_data = { + 'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_not_constrained(self): + constraints_content = textwrap.dedent(""" + """) + project_data = { + 'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertTrue( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_mismatch_blacklisted(self): + constraints_content = textwrap.dedent(""" + name==1.2 + """) + project_data = { + 'requirements': {'requirements.txt': 'name>=1.3,!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse('name'), + ) + ) + + def test_lower_bound_lower(self): + constraints_content = textwrap.dedent(""" + name==1.2 + """) + project_data = { + 'requirements': {'requirements.txt': 'name>=1.1,!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertTrue( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_lower_bound_higher(self): + constraints_content = textwrap.dedent(""" + name==1.2 + """) + project_data = { + 'requirements': {'requirements.txt': 'name>=1.3,!=1.4'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertTrue( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_constrained_version_excluded(self): + constraints_content = textwrap.dedent(""" + name==1.2 + """) + project_data = { + 'requirements': {'requirements.txt': 'name>=1.1,!=1.2'}, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertTrue( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_constraints_with_markers(self): + constraints_content = textwrap.dedent(""" + name==1.1;python_version=='2.7' + name==2.0;python_version=='3.5' + name==2.0;python_version=='3.6' + """) + project_data = { + 'requirements': { + 'requirements.txt': textwrap.dedent(""" + name>=1.1,!=1.2;python_version=='2.7' + name>=2.0;python_version=='3.5' + name>=2.0;python_version=='3.6' + """), + }, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_constraints_with_markers_missing_one_req(self): + constraints_content = textwrap.dedent(""" + name==1.1;python_version=='2.7' + name==2.0;python_version=='3.5' + name==2.0;python_version=='3.6' + """) + project_data = { + 'requirements': { + 'requirements.txt': textwrap.dedent(""" + name>=1.1,!=1.2;python_version=='2.7' + name>=2.0;python_version=='3.5' + """), + }, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_constraints_with_markers_missing_one_marker(self): + constraints_content = textwrap.dedent(""" + name==1.1;python_version=='2.7' + name==2.0;python_version=='3.5' + """) + project_data = { + 'requirements': { + 'requirements.txt': textwrap.dedent(""" + name>=1.1,!=1.2;python_version=='2.7' + name>=2.0;python_version=='3.5' + name>=2.0;python_version=='3.6' + """), + }, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertTrue( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) + + def test_complex_marker_evaluation(self): + constraints_content = textwrap.dedent(""" + name===0.8.0;python_version=='2.7' + name===1.0.0;python_version>='3.0' + """) + project_data = { + 'requirements': { + 'requirements.txt': textwrap.dedent(""" + name>=0.8.0;python_version<'3.0' # BSD + name>=1.0.0;python_version>='3.0' # BSD + """), + }, + 'lower-constraints.txt': constraints_content, + } + head_reqs = check.RequirementsList('testproj', project_data) + head_reqs.process(False) + self.assertFalse( + check.validate_lower_constraints( + req_list=head_reqs, + constraints=project_data['lower-constraints.txt'], + blacklist=requirement.parse(''), + ) + ) diff --git a/openstack_requirements/tests/test_project.py b/openstack_requirements/tests/test_project.py index a3739464f7..204f16066c 100644 --- a/openstack_requirements/tests/test_project.py +++ b/openstack_requirements/tests/test_project.py @@ -45,7 +45,8 @@ class TestReadProject(testtools.TestCase): root = self.useFixture(fixtures.TempDir()).path proj = project.read(root) self.expectThat( - proj, matchers.Equals({'root': root, 'requirements': {}})) + proj, matchers.Equals({'root': root, 'requirements': {}, + 'lower-constraints.txt': None})) class TestProjectExtras(testtools.TestCase): diff --git a/playbooks/files/project-requirements-change.py b/playbooks/files/project-requirements-change.py index 713f2cf927..2746e98bb6 100755 --- a/playbooks/files/project-requirements-change.py +++ b/playbooks/files/project-requirements-change.py @@ -146,6 +146,15 @@ def main(): failed = check.validate(head_reqs, branch_reqs, blacklist, global_reqs) + failed = ( + check.validate_lower_constraints( + head_reqs, + head_proj['lower_constraints.txt'], + blacklist, + ) + or failed + ) + # report the results if failed or head_reqs.failed or branch_reqs.failed: print("*** Incompatible requirement found!")