summaryrefslogtreecommitdiff
path: root/BaseTools
diff options
context:
space:
mode:
authorMichael D Kinney <michael.d.kinney@intel.com>2024-02-18 12:59:51 -0800
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2024-02-27 19:28:19 +0000
commit6d571c0070161b1b96049410f8584c0955d73536 (patch)
treeed523f545840e069ddab705127536c8c0a093045 /BaseTools
parent0bbec15b549032f239f0b05a725598f719d74325 (diff)
downloadedk2-6d571c0070161b1b96049410f8584c0955d73536.zip
edk2-6d571c0070161b1b96049410f8584c0955d73536.tar.gz
edk2-6d571c0070161b1b96049410f8584c0955d73536.tar.bz2
BaseTools/Scripts/PatchCheck: Error if commit modifies multiple packages
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4679 Update PatchCheck.py to evaluate all the files modified in each commit and generate an error if: * A commit adds/modifies files in multiple package directories * A commit adds/modifies files in multiple non-package directories * A commit adds/modifies files in both a package and a non-package directory * A commit deletes files from multiple package directories * A commit deletes files from multiple non-package directories * A commit deletes files from both a package and a non-package directory Modifications to files in the root of the repository are not evaluated. This check is skipped if PatchCheck.py is run on a patch file or input from stdin because this multiple package commit check depends on information from a git repository. If --ignore-multi-package option is set, then reduce the multiple package commit check from an error to a warning for all commits in the commit range provided to PatchCheck.py. Add check for a 'Continuous-integration-options:' commit message tag that allows one or more options to be specified at the individual commit scope to enable/disable continuous integration checks. This tag must start at the beginning of a commit message line and may appear more than once in a commit message. Add support for a Continuous-integration-options tag value of 'PatchCheck.ignore-multi-package' that reduces the multiple package commit check from an error to a warning for the specific commits that specify this option. Example: Continuous-integration-options: PatchCheck.ignore-multi-package The set of packages are found by searching for DEC files in a git repository. The list of DEC files in a git repository is collected with the following git command: git ls-files *.dec The set of files added/modified by each commit is found using the following git command: git diff-tree --no-commit-id --name-only --diff-filter=AM -r <commit> The set of files deleted by each commit is found using the following git command: git diff-tree --no-commit-id --name-only --diff-filter=D -r <commit> Cc: Rebecca Cran <rebecca@bsdio.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Bob Feng <bob.c.feng@intel.com> Cc: Yuwei Chen <yuwei.chen@intel.com> Cc: Michael Kubacki <mikuback@linux.microsoft.com> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Leif Lindholm <quic_llindhol@quicinc.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
Diffstat (limited to 'BaseTools')
-rwxr-xr-xBaseTools/Scripts/PatchCheck.py77
1 files changed, 76 insertions, 1 deletions
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 415198e..a3b812f 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -28,6 +28,7 @@ class Verbose:
class PatchCheckConf:
ignore_change_id = False
+ ignore_multi_package = False
class EmailAddressCheck:
"""Checks an email address."""
@@ -98,6 +99,7 @@ class CommitMessageCheck:
def __init__(self, subject, message, author_email):
self.ok = True
+ self.ignore_multi_package = False
if subject is None and message is None:
self.error('Commit message is missing!')
@@ -120,6 +122,7 @@ class CommitMessageCheck:
self.check_overall_format()
if not PatchCheckConf.ignore_change_id:
self.check_change_id_format()
+ self.check_ci_options_format()
self.report_message_result()
url = 'https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format'
@@ -324,6 +327,15 @@ class CommitMessageCheck:
self.error('\"%s\" found in commit message:' % cid)
return
+ def check_ci_options_format(self):
+ cio='Continuous-integration-options:'
+ for line in self.msg.splitlines():
+ if not line.startswith(cio):
+ continue
+ options = line.split(':', 1)[1].split()
+ if 'PatchCheck.ignore-multi-package' in options:
+ self.ignore_multi_package = True
+
(START, PRE_PATCH, PATCH) = range(3)
class GitDiffCheck:
@@ -561,6 +573,7 @@ class CheckOnePatch:
msg_check = CommitMessageCheck(self.commit_subject, self.commit_msg, self.author_email)
msg_ok = msg_check.ok
+ self.ignore_multi_package = msg_check.ignore_multi_package
diff_ok = True
if self.diff is not None:
@@ -671,6 +684,7 @@ class CheckGitCommits:
"""
def __init__(self, rev_spec, max_count):
+ dec_files = self.read_dec_files_from_git()
commits = self.read_commit_list_from_git(rev_spec, max_count)
if len(commits) == 1 and Verbose.level > Verbose.ONELINE:
commits = [ rev_spec ]
@@ -686,10 +700,66 @@ class CheckGitCommits:
email = self.read_committer_email_address_from_git(commit)
self.ok &= EmailAddressCheck(email, 'Committer').ok
patch = self.read_patch_from_git(commit)
- self.ok &= CheckOnePatch(commit, patch).ok
+ check_patch = CheckOnePatch(commit, patch)
+ self.ok &= check_patch.ok
+ ignore_multi_package = check_patch.ignore_multi_package
+ if PatchCheckConf.ignore_multi_package:
+ ignore_multi_package = True
+ prefix = 'WARNING: ' if ignore_multi_package else ''
+ check_parent = self.check_parent_packages (dec_files, commit, prefix)
+ if not ignore_multi_package:
+ self.ok &= check_parent
+
if not commits:
print("Couldn't find commit matching: '{}'".format(rev_spec))
+ def check_parent_packages(self, dec_files, commit, prefix):
+ ok = True
+ modified = self.get_parent_packages (dec_files, commit, 'AM')
+ if len (modified) > 1:
+ print("{}The commit adds/modifies files in multiple packages:".format(prefix))
+ print(" *", '\n * '.join(modified))
+ ok = False
+ deleted = self.get_parent_packages (dec_files, commit, 'D')
+ if len (deleted) > 1:
+ print("{}The commit deletes files from multiple packages:".format(prefix))
+ print(" *", '\n * '.join(deleted))
+ ok = False
+ return ok
+
+ def get_parent_packages(self, dec_files, commit, filter):
+ filelist = self.read_files_modified_from_git (commit, filter)
+ parents = set()
+ for file in filelist:
+ dec_found = False
+ for dec_file in dec_files:
+ if os.path.commonpath([dec_file, file]):
+ dec_found = True
+ parents.add(dec_file)
+ if not dec_found and os.path.dirname (file):
+ # No DEC file found and file is in a subdir
+ # Covers BaseTools, .github, .azurepipelines, .pytool
+ parents.add(file.split('/')[0])
+ return list(parents)
+
+ def read_dec_files_from_git(self):
+ # run git ls-files *.dec
+ out = self.run_git('ls-files', '*.dec')
+ # return list of .dec files
+ try:
+ return out.split()
+ except:
+ return []
+
+ def read_files_modified_from_git(self, commit, filter):
+ # run git diff-tree --no-commit-id --name-only -r <commit>
+ out = self.run_git('diff-tree', '--no-commit-id', '--name-only',
+ '--diff-filter=' + filter, '-r', commit)
+ try:
+ return out.split()
+ except:
+ return []
+
def read_commit_list_from_git(self, rev_spec, max_count):
# Run git to get the commit patch
cmd = [ 'rev-list', '--abbrev-commit', '--no-walk' ]
@@ -800,6 +870,9 @@ class PatchCheckApp:
group.add_argument("--ignore-change-id",
action="store_true",
help="Ignore the presence of 'Change-Id:' tags in commit message")
+ group.add_argument("--ignore-multi-package",
+ action="store_true",
+ help="Ignore if commit modifies files in multiple packages")
self.args = parser.parse_args()
if self.args.oneline:
Verbose.level = Verbose.ONELINE
@@ -807,6 +880,8 @@ class PatchCheckApp:
Verbose.level = Verbose.SILENT
if self.args.ignore_change_id:
PatchCheckConf.ignore_change_id = True
+ if self.args.ignore_multi_package:
+ PatchCheckConf.ignore_multi_package = True
if __name__ == "__main__":
sys.exit(PatchCheckApp().retval)