From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 528587803D1 for ; Sun, 18 Feb 2024 21:00:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=MV3tixKvUwMfMorjhsktuG6O9cJ198hGUbybYmVB+BE=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1708290002; v=1; b=rpx4M5uGczh+X2lyaLATqilP31MatIQXllBwWkUIyV+5aDaPTRsr++FRqyrkF0hMR9QmzYn1 5rZUlhuHpxsFHWHMA7zBaE8MvZTDXTBSSrY6xlcfcWpMktTBHyN/7vC10d06v2ZRwk3wuv4J9Py MOBGiKXzqs7ZxVY65jhzzQpM= X-Received: by 127.0.0.2 with SMTP id QGmnYY7687511xcGhuBiG0Lb; Sun, 18 Feb 2024 13:00:02 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.18]) by mx.groups.io with SMTP id smtpd.web11.26742.1708289997485769267 for ; Sun, 18 Feb 2024 12:59:58 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10988"; a="2235025" X-IronPort-AV: E=Sophos;i="6.06,169,1705392000"; d="scan'208";a="2235025" X-Received: from orviesa009.jf.intel.com ([10.64.159.149]) by fmvoesa112.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Feb 2024 12:59:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.06,169,1705392000"; d="scan'208";a="4351433" X-Received: from mdkinney-mobl.amr.corp.intel.com ([10.209.68.113]) by orviesa009-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Feb 2024 12:59:58 -0800 From: "Michael D Kinney" To: devel@edk2.groups.io Cc: Rebecca Cran , Liming Gao , Bob Feng , Yuwei Chen , Michael Kubacki , Ard Biesheuvel , Leif Lindholm Subject: [edk2-devel] [Patch 4/4] BaseTools/Scripts/PatchCheck: Error if commit modifies multiple packages Date: Sun, 18 Feb 2024 12:59:51 -0800 Message-Id: <20240218205951.497-5-michael.d.kinney@intel.com> In-Reply-To: <20240218205951.497-1-michael.d.kinney@intel.com> References: <20240218205951.497-1-michael.d.kinney@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: g8KR2C8BUA3OOjsOvMrdJNqxx7686176AA= Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=rpx4M5uG; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io 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 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 Cc: Rebecca Cran Cc: Liming Gao Cc: Bob Feng Cc: Yuwei Chen Cc: Michael Kubacki Cc: Ard Biesheuvel Cc: Leif Lindholm Signed-off-by: Michael D Kinney --- BaseTools/Scripts/PatchCheck.py | 77 ++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py index 415198e3824e..a3b812fb7324 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 + 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) -- 2.40.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115572): https://edk2.groups.io/g/devel/message/115572 Mute This Topic: https://groups.io/mt/104434585/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-