From 4bff2d214d0f2d45815e6712c4a11aa63d3ee526 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Tue, 10 Apr 2018 11:18:16 -0400 Subject: [PATCH] remove optimization for values unchanged from the branch When a requirement setting is changed, require that all requirements follow the rules. Without this change, it is possible to partially update the dependency list for a project in a way that leaves some of the dependencies out of compliance. With this change, all dependencies must be compliant in order to update any of them. Change-Id: I154245339a36618ac2e9a5922bc37121d44bca29 Signed-off-by: Doug Hellmann --- openstack_requirements/check.py | 9 +-- openstack_requirements/tests/test_check.py | 66 ------------------- .../files/project-requirements-change.py | 18 +---- 3 files changed, 4 insertions(+), 89 deletions(-) diff --git a/openstack_requirements/check.py b/openstack_requirements/check.py index cd4b49d550..a30ac2394b 100644 --- a/openstack_requirements/check.py +++ b/openstack_requirements/check.py @@ -131,12 +131,8 @@ def get_global_reqs(content): return global_reqs -def _validate_one(name, reqs, branch_reqs, blacklist, global_reqs): +def _validate_one(name, reqs, blacklist, global_reqs): "Returns True if there is a failure." - if (name in branch_reqs.reqs and - reqs == branch_reqs.reqs[name]): - # Unchanged [or a change that preserves a current value] - return False if name in blacklist: # Blacklisted items are not synced and are managed # by project teams as they see fit, so no further @@ -171,7 +167,7 @@ def _validate_one(name, reqs, branch_reqs, blacklist, global_reqs): return False -def validate(head_reqs, branch_reqs, blacklist, global_reqs): +def validate(head_reqs, blacklist, global_reqs): failed = False # iterate through the changing entries and see if they match the global # equivalents we want enforced @@ -182,7 +178,6 @@ def validate(head_reqs, branch_reqs, blacklist, global_reqs): _validate_one( name, reqs, - branch_reqs, blacklist, global_reqs, ) diff --git a/openstack_requirements/tests/test_check.py b/openstack_requirements/tests/test_check.py index 7e8d7ad7eb..d2413b7862 100644 --- a/openstack_requirements/tests/test_check.py +++ b/openstack_requirements/tests/test_check.py @@ -131,17 +131,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.2,!=1.4')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertFalse( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -153,17 +147,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.2,!=1.4')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertFalse( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse('name'), global_reqs=global_reqs, ) @@ -176,17 +164,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.5')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertFalse( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse('name'), global_reqs=global_reqs, ) @@ -198,17 +180,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.2,!=1.4')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': 'name>=1.2,!=1.4'}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('') self.assertTrue( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -220,17 +196,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.2,!=1.4')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertFalse( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -243,17 +213,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.1,!=1.4')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertFalse( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -266,17 +230,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.2,!=1.4,!=1.5')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertTrue( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -289,17 +247,11 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse('name>=1.2')['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs('name>=1.2,!=1.4') self.assertFalse( check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -317,11 +269,6 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse(r_content)['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs(textwrap.dedent(""" name>=1.5;python_version=='3.5' name>=1.2,!=1.4;python_version=='2.6' @@ -330,7 +277,6 @@ class TestValidateOne(testtools.TestCase): check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -347,11 +293,6 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse(r_content)['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs(textwrap.dedent(""" name>=1.5;python_version=='3.5' name>=1.2,!=1.4;python_version=='2.6' @@ -360,7 +301,6 @@ class TestValidateOne(testtools.TestCase): check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) @@ -378,11 +318,6 @@ class TestValidateOne(testtools.TestCase): r for r, line in requirement.parse(r_content)['name'] ] - branch_reqs = check.RequirementsList( - 'testproj', - {'requirements': {'requirements.txt': ''}}, - ) - branch_reqs.process(False) global_reqs = check.get_global_reqs(textwrap.dedent(""" name>=1.5;python_version=='3.5' name>=1.2,!=1.4;python_version=='2.6' @@ -391,7 +326,6 @@ class TestValidateOne(testtools.TestCase): check._validate_one( 'name', reqs=reqs, - branch_reqs=branch_reqs, blacklist=requirement.parse(''), global_reqs=global_reqs, ) diff --git a/playbooks/files/project-requirements-change.py b/playbooks/files/project-requirements-change.py index d82ca15de5..ca0a9aa12e 100755 --- a/playbooks/files/project-requirements-change.py +++ b/playbooks/files/project-requirements-change.py @@ -136,21 +136,7 @@ def main(): head_strict = not branch.startswith('stable/') head_reqs.process(strict=head_strict) - if not args.local: - # build a list of requirements already in the target branch, - # so that we can create a diff and identify what's being changed - run_command("git checkout HEAD^1") - branch_proj = project.read(cwd) - - # switch back to the proposed change now - run_command("git checkout %s" % branch) - else: - branch_proj = {'root': cwd} - branch_reqs = check.RequirementsList(branch, branch_proj) - # Don't error on the target branch being broken. - branch_reqs.process(strict=False) - - failed = check.validate(head_reqs, branch_reqs, blacklist, global_reqs) + failed = check.validate(head_reqs, blacklist, global_reqs) failed = ( check.validate_lower_constraints( @@ -162,7 +148,7 @@ def main(): ) # report the results - if failed or head_reqs.failed or branch_reqs.failed: + if failed or head_reqs.failed: print("*** Incompatible requirement found!") print("*** See http://docs.openstack.org/developer/requirements") sys.exit(1)