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 1F96B940EC3 for ; Sat, 24 Feb 2024 01:27:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=jJKppDbZWkeVnypUvNsD2lNwjoCPvJU1vAJVC/eIWAI=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1708738022; v=1; b=nvOsXGmhEaYyD9l6NE6nWtCaw5fIqXtliyDNyoZuLB5tzPZ390Bafoun7Jh0iwlf23eseh0U j3cPeq9FFFuPEdfMUM5bx9V2vU2M3U6OImX+TtJRs5PcBeCmYTF533K6zdbd/QBiOycfTQZmojf niVBzznLlMm3C84msuJw4dR4= X-Received: by 127.0.0.2 with SMTP id xaoQYY7687511x9ciN0p0Qkl; Fri, 23 Feb 2024 17:27:02 -0800 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.11335.1708738022069122241 for ; Fri, 23 Feb 2024 17:27:02 -0800 X-Received: from [10.0.0.154] (unknown [20.39.63.0]) by linux.microsoft.com (Postfix) with ESMTPSA id 0B4A120B74C0; Fri, 23 Feb 2024 17:27:00 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 0B4A120B74C0 Message-ID: <84d4277c-40fb-4a5a-bf63-abdebd5f4805@linux.microsoft.com> Date: Fri, 23 Feb 2024 20:27:00 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [Patch 4/4] BaseTools/Scripts/PatchCheck: Error if commit modifies multiple packages To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Rebecca Cran , Liming Gao , Bob Feng , Yuwei Chen , Ard Biesheuvel , Leif Lindholm References: <20240218205951.497-1-michael.d.kinney@intel.com> <20240218205951.497-5-michael.d.kinney@intel.com> From: "Michael Kubacki" In-Reply-To: <20240218205951.497-5-michael.d.kinney@intel.com> 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,mikuback@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: IB0sg3l3o6NlcrmUiiZWwH0px7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=nvOsXGmh; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.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 Reviewed-by: Michael Kubacki On 2/18/2024 3:59 PM, Michael D Kinney wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4679 >=20 > 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 >=20 > Modifications to files in the root of the repository are not > evaluated. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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: >=20 > Continuous-integration-options: PatchCheck.ignore-multi-package >=20 > 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: >=20 > git ls-files *.dec >=20 > The set of files added/modified by each commit is found using the > following git command: >=20 > git diff-tree --no-commit-id --name-only --diff-filter=3DAM -r >=20 > The set of files deleted by each commit is found using the > following git command: >=20 > git diff-tree --no-commit-id --name-only --diff-filter=3DD -r >=20 > 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(-) >=20 > diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchChe= ck.py > index 415198e3824e..a3b812fb7324 100755 > --- a/BaseTools/Scripts/PatchCheck.py > +++ b/BaseTools/Scripts/PatchCheck.py > @@ -28,6 +28,7 @@ class Verbose: > =20 > class PatchCheckConf: > ignore_change_id =3D False > + ignore_multi_package =3D False > =20 > class EmailAddressCheck: > """Checks an email address.""" > @@ -98,6 +99,7 @@ class CommitMessageCheck: > =20 > def __init__(self, subject, message, author_email): > self.ok =3D True > + self.ignore_multi_package =3D False > =20 > 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() > =20 > url =3D 'https://github.com/tianocore/tianocore.github.io/wiki/Comm= it-Message-Format' > @@ -324,6 +327,15 @@ class CommitMessageCheck: > self.error('\"%s\" found in commit message:' % cid) > return > =20 > + def check_ci_options_format(self): > + cio=3D'Continuous-integration-options:' > + for line in self.msg.splitlines(): > + if not line.startswith(cio): > + continue > + options =3D line.split(':', 1)[1].split() > + if 'PatchCheck.ignore-multi-package' in options: > + self.ignore_multi_package =3D True > + > (START, PRE_PATCH, PATCH) =3D range(3) > =20 > class GitDiffCheck: > @@ -561,6 +573,7 @@ class CheckOnePatch: > =20 > msg_check =3D CommitMessageCheck(self.commit_subject, self.comm= it_msg, self.author_email) > msg_ok =3D msg_check.ok > + self.ignore_multi_package =3D msg_check.ignore_multi_package > =20 > diff_ok =3D True > if self.diff is not None: > @@ -671,6 +684,7 @@ class CheckGitCommits: > """ > =20 > def __init__(self, rev_spec, max_count): > + dec_files =3D self.read_dec_files_from_git() > commits =3D self.read_commit_list_from_git(rev_spec, max_count) > if len(commits) =3D=3D 1 and Verbose.level > Verbose.ONELINE: > commits =3D [ rev_spec ] > @@ -686,10 +700,66 @@ class CheckGitCommits: > email =3D self.read_committer_email_address_from_git(commit= ) > self.ok &=3D EmailAddressCheck(email, 'Committer').ok > patch =3D self.read_patch_from_git(commit) > - self.ok &=3D CheckOnePatch(commit, patch).ok > + check_patch =3D CheckOnePatch(commit, patch) > + self.ok &=3D check_patch.ok > + ignore_multi_package =3D check_patch.ignore_multi_package > + if PatchCheckConf.ignore_multi_package: > + ignore_multi_package =3D True > + prefix =3D 'WARNING: ' if ignore_multi_package else '' > + check_parent =3D self.check_parent_packages (dec_files, comm= it, prefix) > + if not ignore_multi_package: > + self.ok &=3D check_parent > + > if not commits: > print("Couldn't find commit matching: '{}'".format(rev_spec= )) > =20 > + def check_parent_packages(self, dec_files, commit, prefix): > + ok =3D True > + modified =3D 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 =3D False > + deleted =3D self.get_parent_packages (dec_files, commit, 'D') > + if len (deleted) > 1: > + print("{}The commit deletes files from multiple packages:".f= ormat(prefix)) > + print(" *", '\n * '.join(deleted)) > + ok =3D False > + return ok > + > + def get_parent_packages(self, dec_files, commit, filter): > + filelist =3D self.read_files_modified_from_git (commit, filter) > + parents =3D set() > + for file in filelist: > + dec_found =3D False > + for dec_file in dec_files: > + if os.path.commonpath([dec_file, file]): > + dec_found =3D 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 =3D 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 =3D self.run_git('diff-tree', '--no-commit-id', '--name-only= ', > + '--diff-filter=3D' + 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 =3D [ 'rev-list', '--abbrev-commit', '--no-walk' ] > @@ -800,6 +870,9 @@ class PatchCheckApp: > group.add_argument("--ignore-change-id", > action=3D"store_true", > help=3D"Ignore the presence of 'Change-Id:' = tags in commit message") > + group.add_argument("--ignore-multi-package", > + action=3D"store_true", > + help=3D"Ignore if commit modifies files in mu= ltiple packages") > self.args =3D parser.parse_args() > if self.args.oneline: > Verbose.level =3D Verbose.ONELINE > @@ -807,6 +880,8 @@ class PatchCheckApp: > Verbose.level =3D Verbose.SILENT > if self.args.ignore_change_id: > PatchCheckConf.ignore_change_id =3D True > + if self.args.ignore_multi_package: > + PatchCheckConf.ignore_multi_package =3D True > =20 > if __name__ =3D=3D "__main__": > sys.exit(PatchCheckApp().retval) -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115903): https://edk2.groups.io/g/devel/message/115903 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-