From 9b0176d091550a52bc59822da82cf69291b9c6eb Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Wed, 2 Apr 2014 00:02:30 +0000 Subject: [PATCH] Requirements check should tolerate existing cruft * .../slave_scripts/project-requirements-change.py: When parsing requirements files, only perform style checks like missing final newline or duplicate entries on the new/proposed version of a file and ignore them on the original version of the file in the repository. This makes it possible to actually correct errors on imported requirements files, even with the job running this script already voting on the correcting change. Also annotate this with more commentary so people have some hope of understanding what the heck it's doing. Change-Id: I8ea2800ecc8eb24937e69a573e925b4daf64ff3f Closes-Bug: #1300843 --- .../project-requirements-change.py | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/modules/jenkins/files/slave_scripts/project-requirements-change.py b/modules/jenkins/files/slave_scripts/project-requirements-change.py index 56750f081d..996668cf25 100755 --- a/modules/jenkins/files/slave_scripts/project-requirements-change.py +++ b/modules/jenkins/files/slave_scripts/project-requirements-change.py @@ -39,11 +39,12 @@ class RequirementsList(object): self.reqs = {} self.failed = False - def read_requirements(self, fn, ignore_dups=False): + def read_requirements(self, fn, ignore_dups=False, strict=False): + """ Read a requirements file and optionally enforce style.""" if not os.path.exists(fn): return for line in open(fn): - if '\n' not in line: + if strict and '\n' not in line: raise Exception("Requirements file %s does not " "end with a newline." % fn) if '#' in line: @@ -55,13 +56,15 @@ class RequirementsList(object): line.startswith('-f')): continue req = pkg_resources.Requirement.parse(line) - if not ignore_dups and req.project_name.lower() in self.reqs: + if (not ignore_dups and strict and req.project_name.lower() + in self.reqs): print("Duplicate requirement in %s: %s" % (self.name, str(req))) self.failed = True self.reqs[req.project_name.lower()] = req - def read_all_requirements(self, global_req=False, include_dev=False): + def read_all_requirements(self, global_req=False, include_dev=False, + strict=False): """ Read all the requirements into a list. Build ourselves a consolidated list of requirements. If global_req is @@ -71,34 +74,45 @@ class RequirementsList(object): If include_dev is true allow for development requirements, which may be prereleased versions of libraries that would otherwise be listed. This is most often used for olso prereleases. + + If strict is True then style checks should be performed while reading + the file. """ if global_req: - self.read_requirements('global-requirements.txt') + self.read_requirements('global-requirements.txt', strict=strict) else: for fn in ['tools/pip-requires', 'tools/test-requires', 'requirements.txt', 'test-requirements.txt' ]: - self.read_requirements(fn) + self.read_requirements(fn, strict=strict) if include_dev: self.read_requirements('dev-requirements.txt', - ignore_dups=True) + ignore_dups=True, strict=strict) def main(): branch = sys.argv[1] + + # build a list of requirements in the proposed change, + # and check them for style violations while doing so head = run_command("git rev-parse HEAD").strip() head_reqs = RequirementsList('HEAD') - head_reqs.read_all_requirements() + head_reqs.read_all_requirements(strict=True) + # 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 remote update") run_command("git checkout remotes/origin/%s" % branch) branch_reqs = RequirementsList(branch) branch_reqs.read_all_requirements() + # switch back to the proposed change now run_command("git checkout %s" % head) + # build a list of requirements from the global list in the + # openstack/requirements project so we can match them to the changes reqroot = tempfile.mkdtemp() reqdir = os.path.join(reqroot, "requirements") run_command("git clone https://review.openstack.org/p/openstack/" @@ -111,6 +125,8 @@ def main(): os_reqs.read_all_requirements(include_dev=(branch == 'master'), global_req=True) + # iterate through the changing entries and see if they match the global + # equivalents we want enforced failed = False for req in head_reqs.reqs.values(): name = req.project_name.lower() @@ -128,6 +144,7 @@ def main(): "value %s" % (str(req), str(os_reqs.reqs[name]))) failed = True + # clean up and report the results shutil.rmtree(reqroot) if failed or os_reqs.failed or head_reqs.failed or branch_reqs.failed: sys.exit(1)