Subject Check was added to PatchCheck.py to enforce that if a package
was touched in a commit that it be referenced in the subject line.
However, this is impractical for multipackage commits with many
packages, e.g. when stack cookies were added, every package was
touched, but in a rote way, and it is not reasonable to put every
package in a subject line.
This updates PatchCheck.py to check if ignore_multi_package is set
and if so only require that package names be included in the subject
if there are fewer than 3 packages touched. Otherwise, PatchCheck will
require the message to start with `Global:` to indicate it touches
more than 3 packages.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
PatchCheck.py currently parses the CI options as the last
step it does before reporting results. This means that the
other checking logic cannot use any of the CI options that
are passed in.
This updates the order of operations to process CI options
before running other checks so that they can be used in
performing checks.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Commit b55530ad44
("BaseTools/PatchCheck.py: verify commit message lists package(s)")
introduced a check for the requirement to enumerate all modified packages
in the commit subject line. But it did leave the maximum line length at
75 characters (for non-CVE commits), which can get a bit cramped for
changes to several packages.
Introduce a new arbitrary "at least 20 characters after the :" limit.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
Use a temporary variable for max subject line length and log
result of test in one location.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
Before doing the subject line length check, the script checks that the
number of lines in the commit message (including subject) is not less
than or equal to zero - and returns if it is.
However, then the test for whether the subject line starts with a CVE
tag inexplicably also checks for whether the number of lines are
greater than or equal to one. This is just clutter, so drop it.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
Verify that the subject line matches the basic
{Package}[,Package]:
format _or_ the
Revert "<subject of commit to revert>"
format.
Non-package top-level directories are treated as packages.
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
In order to enable subject line format compliance checking in following
patches, pass through a list of modified packages to CommitMessageCheck.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
The get_parent_packages function in CheckGitCommits returns the path of
non-package directories, but in fact returns the path of the .dec file
for actual packages.
Align the handling to be more consistent and return only directory names,
regarding of how it was found.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
PatchCheck error messages can be improved by adding the line number.
The line itself may consist of only whitespace for some errors.
Adding the line number can help better locate the error source.
Signed-off-by: Gary Beihl <garybeihl@microsoft.com>
BaseTools was moved out to a separate repo and consumed as a pip
module by edk2 CI. This process has not led to the desired goals
of doing so, so this patch removes the pip based BaseTools from
edk2 CI.
The original goal of moving BaseTools to a pip module was
primarily to speed up the development process, as the old edk2
mailing list was slow. However, with edk2 moving to PRs, it now
actually slows the BaseTools development process to have to do
a PR in another repo, publish the module, and then make a PR
in edk2 to consume the new BaseTools. It also holds up using
the features in a new BaseTools in other PRs.
There were other goals of moving, such as allowing projects to
use the BaseTools outside of edk2. This can still be accomplished
outside of this PR, this PR simply stops edk2 CI from using the
pip module.
Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Fix logic error that changes the commit range checked depending
on the verbosity level set.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
The commit message format requirements have been updated for
GitHub PR based code reviews and no longer required Cc: tags
for the maintainers and reviewers. Remove the Cc: tag check
from PatchCheck.py.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
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>
Code review tools like gerrit might use a 'Change-id' tag to track
the evolution of patches. This tag should be removed before
submitting a patch to the mailing-list.
It has been observed that contributors sometimes forget to remove
this tag. Add a check in PatchCheck.py to automate this.
Also add a '--ignore-change-id' command line parameter to ignore
the above check.
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
Acked-by: Pedro Falcato <pedro.falcato@gmail.com>
Reviewed-by: Yuwei Chen <yuwei.chen@intel.com>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
Allow .rtf files created by applications such as Notepad to be committed
as-is without further manual editing by skipping the requirements for
CRLF, no tabs and no trailing whitespace.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Acked-by: Laszlo Ersek <lersek@redhat.com>
New code should use the C99 macro __func__ instead of the pre-Standard
macro __FUNCTION__. Update PatchCheck.py to reject patches with the
latter.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Purdue Compiler Construction Tool Set (PCCTS) source code was copied/
pasted into BaseTools/Source/C/VfrCompile/Pccts/.
The code contains tab characters instead of spaces.
PatchCheck.py gives an error on modifications to files that
contain tabs.
The goal of my upcoming change there is not to mix tabs and spaces
but to fix a bug while preserving its current formatting characters.
This change adds that directory to the pre-existing list of
directories in which tab checks are ignored in PatchCheck.py
and also updates the check for makefiles to check for *.makefile:
this allows {header,footer,app,lib}.makefile in
BaseTools/Source/C/Makefiles to be detected and avoid having
PatchCheck.py complain about tab characters.
The check for "Makefile" is updated to be case-insensitive since
there are some Makefiles named 'makefile' instead of 'Makefile'.
Co-authored-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
With the removal of RVCT support and the related Bin/CYGWIN_NT-5.1-i686
and Darwin-i386 directories, remove a leftover reference to
CYGWIN_NT-5.1-i686 from Scripts/PatchCheck.py.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Acked-by: Bob Feng <bob.c.feng@intel.com>
The syntax for Makefiles requires that indented lines s
tart with a tab, but not a space.
This change of PatchCheck.py make the patch for Makefile/GNUmakefile
pass the PatchCheck.py.
Signed-off-by: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
