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 36ACAAC0EDA for ; Wed, 14 Feb 2024 07:11:57 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=kb0YHRCDSh7hu3/3QiJs8faMN1rjJyIvSgBSo+dquKg=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1707894716; v=1; b=eBEHGSmHjTDEjmAl6x5WWip30zzI4xW5sPyG1hF+nj7N3zZKEvpjxQqsjleNCIg+9FkeRjfY JArm81D5IepwBSu3qWQQiu5d26Y2XYhyqhpDfy/ICiR+9OLe5Vi35uvfEXYTVScDj5GShCgrEsJ WqAwJsPHeDWudb8/04Rl6Bf0= X-Received: by 127.0.0.2 with SMTP id yPalYY7687511xZuU2k7fp2t; Tue, 13 Feb 2024 23:11:56 -0800 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web10.36610.1707894715472468996 for ; Tue, 13 Feb 2024 23:11:56 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 847F1CE1C2A for ; Wed, 14 Feb 2024 07:11:52 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id C08C4C433A6 for ; Wed, 14 Feb 2024 07:11:51 +0000 (UTC) X-Received: by mail-lj1-f173.google.com with SMTP id 38308e7fff4ca-2d0cd9871b3so3867791fa.1 for ; Tue, 13 Feb 2024 23:11:51 -0800 (PST) X-Gm-Message-State: RkE1cI4g5XGdeB7AHGLwVU5sx7686176AA= X-Google-Smtp-Source: AGHT+IEmm6SIXmzbSh9LshOk89D2onK88GFk9H/D6gatk/1dcPA9C4js3fesXBlwvrDm3MfeOu0sspqnjQIN73RPLIk= X-Received: by 2002:ac2:5b8f:0:b0:511:8c6b:bc78 with SMTP id o15-20020ac25b8f000000b005118c6bbc78mr366348lfn.2.1707894709797; Tue, 13 Feb 2024 23:11:49 -0800 (PST) MIME-Version: 1.0 References: <20240214011751.2529-1-michael.d.kinney@intel.com> In-Reply-To: <20240214011751.2529-1-michael.d.kinney@intel.com> From: "Ard Biesheuvel" Date: Wed, 14 Feb 2024 08:11:38 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [Patch 1/1] 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 , Michael Kubacki 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,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=eBEHGSmH; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (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 On Wed, 14 Feb 2024 at 02:18, Michael D Kinney wrote: > > 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 single commit modifies files in multiple packages > * A single commit modifies files in multiple non-package dirs > * A single commit modifies files in both a package and a > non-package dir. > > Modifications to files in the root of the repository are not > evaluated. > > The set of packages are found by search for DEC files in the > repository. The list of DEC files in the 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 > Signed-off-by: Michael D Kinney Will this trigger pre-merge CI failures if the patch touches more than one package? If so, I will need an opt-out/override for this. > --- > BaseTools/Scripts/PatchCheck.py | 49 +++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py > index 1675dcbd7321..988f152e38d7 100755 > --- a/BaseTools/Scripts/PatchCheck.py > +++ b/BaseTools/Scripts/PatchCheck.py > @@ -665,6 +665,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 ] > @@ -681,9 +682,57 @@ class CheckGitCommits: > self.ok &= EmailAddressCheck(email, 'Committer').ok > patch = self.read_patch_from_git(commit) > self.ok &= CheckOnePatch(commit, patch).ok > + self.ok &= self.check_parent_packages (dec_files, commit) > + > if not commits: > print("Couldn't find commit matching: '{}'".format(rev_spec)) > > + 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 check_parent_packages(self, dec_files, commit): > + modified = self.get_parent_packages (dec_files, commit, 'AM') > + if len (modified) > 1: > + print("The commit adds/modifies files in multiple packages:\n *", > + '\n * '.join(modified)) > + self.ok = False > + deleted = self.get_parent_packages (dec_files, commit, 'D') > + if len (deleted) > 1: > + print("The commit deletes files from multiple packages:\n *", > + '\n * '.join(deleted)) > + self.ok = False > + return self.ok > + > + 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' ] > -- > 2.40.1.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115429): https://edk2.groups.io/g/devel/message/115429 Mute This Topic: https://groups.io/mt/104345509/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-