From: Eric Ball Date: Thu, 18 Feb 2021 23:48:42 +0000 (-0800) Subject: Check dco_signoffs files for missing DCO hashes X-Git-Tag: v0.35.2~7 X-Git-Url: https://gerrit.linuxfoundation.org/infra/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F31%2F66831%2F4;p=releng%2Flftools.git Check dco_signoffs files for missing DCO hashes When checking for missing DCOs, any hashes found to be documented in the dco_signoffs (or a user-specified alternative) directory will no longer be included in the missing list. Some formatting improvements have also been made to the relevant files, particularly removing unnecessary #noqa and #nosec directives, which were not affecting pylint scores. Issue: RELENG-3396 Change-Id: I7cdac03fb406c6ce862b997c01fb0fab7cfecd76 Signed-off-by: Eric Ball --- diff --git a/lftools/cli/dco.py b/lftools/cli/dco.py index e6181cd8..aa36c40d 100644 --- a/lftools/cli/dco.py +++ b/lftools/cli/dco.py @@ -27,37 +27,61 @@ def dco(ctx): @click.command() @click.argument("repo-path", required=False) +@click.option( + "--signoffs", + type=str, + required=False, + default="dco_signoffs", + help="Specify a directory to check for DCO signoff text files", +) @click.pass_context -def check(ctx, repo_path): +def check(ctx, repo_path, signoffs): """Check repository for commits missing DCO. This check will exclude merge commits and empty commits. It operates in your current working directory which has to be a git repository. Alternatively, you can opt to pass in the path to a git repo. + + By default, this will also check for DCO signoff files in a directory + named "dco_signoffs". To check in a different directory, use the + --signoffs option. To ignore signoff files, an empty string can be passed. + Refer to https://developercertificate.org/ """ if not repo_path: repo_path = "." - status = dco_checker.check(repo_path) + status = dco_checker.check(repo_path, signoffs) sys.exit(status) @click.command() @click.argument("repo-path", required=False) +@click.option( + "--signoffs", + type=str, + required=False, + default="dco_signoffs", + help="Specify a directory to check for DCO signoff text files", +) @click.pass_context -def match(ctx, repo_path): +def match(ctx, repo_path, signoffs): """Check for commits whose DCO does not match the commit author's email. This check will exclude merge commits and empty commits. It operates in your current working directory which has to be a git repository. Alternatively, you can opt to pass in the path to a git repo. + + By default, this will also check for DCO signoff files in a directory + named "dco_signoffs". To check in a different directory, use the + --signoffs option. To ignore signoff files, an empty string can be passed. + Refer to https://developercertificate.org/ """ if not repo_path: repo_path = "." - status = dco_checker.match(repo_path) + status = dco_checker.match(repo_path, signoffs) sys.exit(status) diff --git a/lftools/shell/dco.py b/lftools/shell/dco.py index 5377d562..1a1a06c8 100644 --- a/lftools/shell/dco.py +++ b/lftools/shell/dco.py @@ -13,10 +13,11 @@ __author__ = "DW Talton" import logging +import os from os import chdir from os import getcwd import re -import subprocess # nosec +import subprocess log = logging.getLogger(__name__) @@ -30,7 +31,7 @@ def get_branches(path=getcwd(), invert=False): chdir(path) try: branches = ( - subprocess.check_output("git branch -r | grep -v origin/HEAD", shell=True) # nosec + subprocess.check_output("git branch -r | grep -v origin/HEAD", shell=True) .decode(encoding="UTF-8") .splitlines() ) @@ -38,8 +39,8 @@ def get_branches(path=getcwd(), invert=False): for branch in branches: branch = branch.strip() hashes = ( - subprocess.check_output( # nosec - 'git log {} --no-merges --pretty="%H %ae" --grep "Signed-off-by" {}'.format(branch, invert), # noqa + subprocess.check_output( + 'git log {} --no-merges --pretty="%H %ae" --grep "Signed-off-by" {}'.format(branch, invert), shell=True, ) .decode(encoding="UTF-8") @@ -57,7 +58,7 @@ def get_branches(path=getcwd(), invert=False): exit(1) -def check(path=getcwd()): +def check(path=getcwd(), signoffs_dir="dco_signoffs"): """Check repository for commits missing DCO.""" chdir(path) try: @@ -70,9 +71,21 @@ def check(path=getcwd()): if commit: missing.append(commit.split(" ")[0]) - if missing: - # de-dupe the commit list - missing_list = list(dict.fromkeys(missing)) + if not missing: + exit(0) + + # de-dupe the commit list + missing_list = list(dict.fromkeys(missing)) + + # Check for dco_signoffs file + if os.path.isdir(signoffs_dir): + missing_list = [ + commit + for commit in missing_list + if subprocess.run(["grep", "-Ri", commit, signoffs_dir], capture_output=True).returncode + ] + + if missing_list: for commit in missing_list: log.info("{}".format(commit)) exit(1) @@ -81,7 +94,7 @@ def check(path=getcwd()): exit(1) -def match(path=getcwd()): +def match(path=getcwd(), signoffs_dir="dco_signoffs"): """Check for commits where DCO does not match the commit author's email.""" chdir(path) try: @@ -89,30 +102,44 @@ def match(path=getcwd()): exit_code = 0 if not hashes: exit(exit_code) - else: - for commit in hashes: - commit_id = commit.split(" ")[0] - if commit_id: - commit_log_message = subprocess.check_output( # nosec - "git log --format=%B -n 1 {}".format(commit_id), shell=True - ).decode(encoding="UTF-8") - commit_author_email = ( - subprocess.check_output("git log --format='%ae' {}^!".format(commit_id), shell=True) # nosec - .decode(encoding="UTF-8") - .strip() - ) - sob_email_regex = "(?=Signed\-off\-by: )*[\<](.*)[\>]" # noqa - sob_results = re.findall(sob_email_regex, commit_log_message) - - if commit_author_email in sob_results: + + # de-dupe the commit list + hashes_list = list(dict.fromkeys(hashes)) + + for commit in hashes_list: + commit_id = commit.split(" ")[0] + if commit_id: + commit_log_message = subprocess.check_output( + "git log --format=%B -n 1 {}".format(commit_id), shell=True + ).decode(encoding="UTF-8") + commit_author_email = ( + subprocess.check_output("git log --format='%ae' {}^!".format(commit_id), shell=True) + .decode(encoding="UTF-8") + .strip() + ) + sob_email_regex = r"(?=Signed\-off\-by: ).*[\<](.*)[\>]" + sob_results = re.findall(sob_email_regex, commit_log_message) + + if not commit_author_email in sob_results and os.path.isdir(signoffs_dir): + dco_file = subprocess.run( + ["grep", "-Rli", commit_author_email, signoffs_dir], capture_output=True + ).stdout + # If commit_author_email and commit_id are in the same file, do not print info + if ( + dco_file + and not subprocess.run( + ["grep", "-li", commit_id, dco_file.strip()], capture_output=True + ).returncode + ): continue - else: - log.info( - "For commit ID {}: \n\tCommitter is {}" - "\n\tbut commit is signed off by {}\n".format(commit_id, commit_author_email, sob_results) - ) - exit_code = 1 - exit(exit_code) + + log.info( + "For commit ID {}: \n\tCommitter is {}" + "\n\tbut commit is signed off by {}\n".format(commit_id, commit_author_email, sob_results) + ) + exit_code = 1 + + exit(exit_code) except subprocess.CalledProcessError as e: log.error(e) exit(1) diff --git a/releasenotes/notes/dco-signoffs-feature-e0f65249153942fb.yaml b/releasenotes/notes/dco-signoffs-feature-e0f65249153942fb.yaml new file mode 100644 index 00000000..eb479802 --- /dev/null +++ b/releasenotes/notes/dco-signoffs-feature-e0f65249153942fb.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + ``lftools dco check`` will now include a check for signoff files to remove + commits from the "missing DCO" list. By default, this will check the + directory "dco_signoffs", but the ``--signoffs`` option can be used to + specify a different directory. diff --git a/requirements.txt b/requirements.txt index 64823cf2..0d02590d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -31,7 +31,7 @@ PyGithub==1.46 PyJWT==1.7.1 pyrsistent==0.15.7 python-jenkins==1.7.0 -PyYAML==5.3 +PyYAML requests==2.23.0 rsa==4.0 ruamel.yaml==0.16.10