[RFC] EDK II Continuous Integration Phase 1


Michael D Kinney
 

Hello,

This is a proposal for a first step towards continuous
integration for all TianoCore repositories to help
improve to quality of commits and automate testing and
release processes for all EDK II packages and platforms.

This is based on work from a number of EDK II community
members that have provide valuable input and evaluations.

* Rebecca Cran <rebecca@bsdio.com> Jenkins evaluation
* Laszlo Ersek <lersek@redhat.com> GitLab evaluation
* Philippe Mathieu-Daudé <philmd@redhat.com> GitLab evaluation
* Sean Brogan <sean.brogan@microsoft.com> Azure Pipelines and HBFA
* Bret Barkelew <Bret.Barkelew@microsoft.com> Azure Pipelines and HBFA
* Jiewen Yao <jiewen.yao@intel.com> HBFA

The following link is a link to an EDK II WIKI page that
contains a summary of the work to date. Please provide
feedback in the EDK II mailing lists. The WIKI pages will
be updated with input from the entire EDK II community.

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Continuous-Integration

Proposal
========
Phase 1 of adding continuous integration is limited to the
edk2 repository. Additional repositories will be added later.

The following changes are proposed:
* Remove EDK II Maintainers write access to edk2 repository.
Only EDK II Administrators will continue to have write
access, and that should only be used to handle extraordinary
events.
* EDK II Maintainers use a GitHub Pull Request instead of push
to commit a patch series to the edk2 repository. There are
no other changes to the development and review process. The
patch series is prepared in an EDK II maintainer branch with
all commit message requirements met on each patch in the series.
* The edk2 repository only accepts Pull Requests from members
of the EDK II Maintainers team. Pull Requests from anyone else
are rejected.
* Run pre-commit checks using Azure Pipelines
* If all pre-commit checks pass, then the patch series is auto
committed. The result of this commit must match the contents
and commit history that would have occurred using the previous
push operation.
* If any pre-commit checks fail, then notify the submitter.
A typical reason for a failure would be a merge conflict with
another pull request that was just processed.
* Limit pre-commit checks execution time to 10 minutes.
* Provide on-demand builds to EDK II Maintainers that to allow
EDK II Maintainers to submit a branch through for the same
set of pre-commit checks without submitting a pull request.

## Pre-Commit Checks in Phase 1
* Run and pass PatchCheck.py with no errors

=====================================================

The following are some additional pre-commit check ideas
that could be quickly added once the initial version using
PatchCheck.py is fully functional. Please provide feedback
on the ones you like and additional ones you think may
improve the quality of the commits to the edk2 repository.

## Proposed Pre-Commit Checks in Phase 2
* Verify Reviewed-by and Acked-by tags are present with
correct maintainer email addresses
* Verify no non-ASCII characters in modified files
* Verify no binary files in set of modified files
* Verify package dependency rules in modified files

## Proposed Pre-Commit Checks in Phase 3
* Run ECC on modified files
* Verify modified modules/libs build
* Run available host based tests (HBFA) against modified
modules/libs

Best regards,

Mike


Sean
 

Mike, as you mentioned we have been working towards enabling a practical and extensible CI for Edk2 using Azure dev ops and the recently added edk2-pytool infrastructure. We have been using similar CI for Project Mu for the last few years.

Our approach is a little different in that we focus on validating the whole code base rather than just the incoming patch. We do this because we have found unexpected consequences of patches and overall we want all code to be compliant not just new additions. We have found the time to test the whole tree is not much longer than only the parts impacted by a code change (except maybe when running the entire compile test on every package). This obviously comes with an initial tax of needing to get the codebase into compliant form. Anyway we have prepared an RFC in addition to yours and would like to see these two efforts merged together.

We are still working on making a few optimizations. Currently if the full set of tests are run we take about 20 minutes. This is because compiling MdeModulePkg for debug, release, and host based tests take a while. Most other packages are in the 10 minute range. We do have easy ways to disable or limit certain tests as well as expand the matrix to leverage more cloud resources (more parallel builds).


Content is best viewed online with links to helpful content but is also attached below:
https://github.com/spbrogan/edk2-staging/blob/edk2-stuart-ci-latest/Readme-CI-RFC.md

# CI and PR Gates

## Background

Historically, while the TianoCore maintainers and stewards have done a fantastic job of keeping contribution policies consistent and contributions clean and well-documented, there have been few processes that ran to verify the sanity, cleanliness, and efficacy of the codebase, and even fewer that publicly published their results for the community at large. This has caused inconsistancies and issues within the codebase from time to time.

Adding continuous integration (and potentially PR gates) to the checkin process ensures that simple errors like these are caught and can be fixed on a regular basis.

## Strategy

While a number of CI solutions exist, this proposal will focus on the usage of Azure Dev Ops and Build Pipelines. For demonstration, a sample [TianoCore repo](https://github.com/spbrogan/edk2-staging.git) (branch edk2-stuart-ci-latest) and [Dev Ops Pipeline](https://dev.azure.com/tianocore/edk2-ci-play/_build?definitionId=12) have been set up.

Furthermore, this proposal will leverage the TianoCore python tools PIP modules: [library](https://pypi.org/project/edk2-pytool-library/) and [extensions](https://pypi.org/project/edk2-pytool-extensions/) (with repos located [here](https://github.com/tianocore/edk2-pytool-library) and [here](https://github.com/tianocore/edk2-pytool-extensions)).

The primary execution flows can be found in the `azure-pipelines-pr-gate.yml` and `azure-pipelines-pr-gate-linux.yml` files. These YAML files are consumed by the Azure Dev Ops Build Pipeline and dictate what server resources should be used, how they should be configured, and what processes should be run on them. An overview of this schema can be found [here](https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema).

Inspection of these files reveals the EDKII Tools commands that make up the primary processes for the CI build: 'stuart_setup', 'stuart_update', and 'stuart_ci_build'. These commands come from the EDKII Tools PIP modules and are configured as described below. More documentation on the stuart tools can be found [here](https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/using.md) and [here](https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/features/feature_invocables.md).

## Configuration

Configuration of the CI process consists of (in order of precedence):
* command-line arguments passed in via the Pipeline YAML
* a per-package configuration file (e.g. `<package-name>.mu.yaml`) that is detected by the CI system in EDKII Tools.
* a global configuration Python module (e.g. `CISetting.py`) passed in via the command-line

The global configuration file is described in [this readme](https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_settings_manager.md) from the EDKII Tools documentation. This configuration is written as a Python module so that decisions can be made dynamically based on command line parameters and codebase state.

The per-package configuration file can override most settings in the global configuration file, but is not dynamic. This file can be used to skip or customize tests that may be incompatible with a specific package. By default, the global configuration will try to run all tests on all packages.

## CI Test Types

All CI tests are instances of EDKII Tools plugins. Documentation on the plugin system can be found [here](https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/using_plugin_manager.md) and [here](https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/features/feature_plugin_manager.md). Upon invocation, each plugin will be passed the path to the current package under test and a dictionary containing its targeted configuration, as assembled from the command line, per-package configuration, and global configuration.

Note: CI plugins are considered unique from build plugins and helper plugins, even though some CI plugins may execute steps of a build.

In the example, these plugins live alongside the code under test (in the `BaseTools` directory), but may be moved to the 'edk2-test' repo if that location makes more sense for the community.

### Module Inclusion Test - DscCompleteCheck

This test scans all available modules (via INF files) and compares them to the package-level DSC file for the package each module is contained within. The test considers it an error if any module does not appear in the `Components` section of at least one package-level DSC (indicating that it would not be built if the package were built).

### Code Compilation Test - CompilerPlugin

Once the Module Inclusion Test has verified that all modules would be built if all package-level DSCs were built, the Code Compilation Test simply runs through and builds every package-level DSC on every toolchain and for every architecture that is supported. Any module that fails to build is considered an error.

### Host-Based UnitTests - HostUnitTestCompilerPlugin and HostUnitTestDscCompleteCheck

The [Testing RFC doc](Readme-Testing-RFC.md) has much more detail on this, but the basic idea is that host-based unit tests can be compiled against individual modules and libraries and run on the build agent (in this case, the Dev Ops build server). The successful and failing test case results are collected and included in the final build report.

### GUID Uniqueness Test - GuidCheck

This test works on the collection of all packages rather than an individual package. It looks at all FILE_GUIDs and GUIDs declared in DEC files and ensures that they are unique for the codebase. This prevents, for example, accidental duplication of GUIDs when using an existing INF as a template for a new module.

### Cross-Package Dependency Test - DependencyCheck

This test compares the list of all packages used in INFs files for a given package against a list of "allowed dependencies" in plugin configuration for that package. Any module that depends on a disallowed package will cause a test failure.

### Library Declaration Test - LibraryClassCheck

This test looks at all library header files found in a package's `Include/Library` directory and ensures that all files have a matching LibraryClass declaration in the DEC file for the package. Any missing declarations will cause a failure.

### Invalid Character Test - CharEncodingCheck

This test scans all files in a package to make sure that there are no invalid Unicode characters that may cause build errors in some character sets/localizations.

## Next Steps

* Receive community feedback on RFC.
* Determine where this phase makes sense given existing RFCs from other TianoCore contributors.
* Optimize testing beharior.
* Only run a subset of tests on PRs or individual commits.
* Run full testing either once per day or once every several commits.
* Add more tests/capabilities.
* Continue to improve results formatting.
* Continue to improve CI documentation.
* Much of this documentation effort is pending community feedback on which parts are needed and what phases are priorities.

Thanks

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Michael D Kinney via Groups.Io
Sent: Thursday, August 29, 2019 1:23 PM
To: devel@edk2.groups.io; rfc@edk2.groups.io
Subject: [edk2-rfc] [RFC] EDK II Continuous Integration Phase 1

Hello,

This is a proposal for a first step towards continuous integration for all TianoCore repositories to help improve to quality of commits and automate testing and release processes for all EDK II packages and platforms.

This is based on work from a number of EDK II community members that have provide valuable input and evaluations.

* Rebecca Cran <mailto:rebecca@bsdio.com> Jenkins evaluation
* Laszlo Ersek <mailto:lersek@redhat.com> GitLab evaluation
* Philippe Mathieu-Daudé <mailto:philmd@redhat.com> GitLab evaluation
* Sean Brogan <mailto:sean.brogan@microsoft.com> Azure Pipelines and HBFA
* Bret Barkelew <mailto:Bret.Barkelew@microsoft.com> Azure Pipelines and HBFA
* Jiewen Yao <mailto:jiewen.yao@intel.com> HBFA

The following link is a link to an EDK II WIKI page that contains a summary of the work to date. Please provide feedback in the EDK II mailing lists. The WIKI pages will be updated with input from the entire EDK II community.

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Continuous-Integration&;data=02%7C01%7Csean.brogan%40microsoft.com%7C6f67f169a6c746b4288608d72cbea7b6%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637027069686644659&amp;sdata=GR9wN6gP3mJlCTopAaQ2rlzhby1nuF%2BwDVsfFIQAZjA%3D&amp;reserved=0

Proposal
========
Phase 1 of adding continuous integration is limited to the
edk2 repository. Additional repositories will be added later.

The following changes are proposed:
* Remove EDK II Maintainers write access to edk2 repository.
Only EDK II Administrators will continue to have write
access, and that should only be used to handle extraordinary
events.
* EDK II Maintainers use a GitHub Pull Request instead of push
to commit a patch series to the edk2 repository. There are
no other changes to the development and review process. The
patch series is prepared in an EDK II maintainer branch with
all commit message requirements met on each patch in the series.
* The edk2 repository only accepts Pull Requests from members
of the EDK II Maintainers team. Pull Requests from anyone else
are rejected.
* Run pre-commit checks using Azure Pipelines
* If all pre-commit checks pass, then the patch series is auto
committed. The result of this commit must match the contents
and commit history that would have occurred using the previous
push operation.
* If any pre-commit checks fail, then notify the submitter.
A typical reason for a failure would be a merge conflict with
another pull request that was just processed.
* Limit pre-commit checks execution time to 10 minutes.
* Provide on-demand builds to EDK II Maintainers that to allow
EDK II Maintainers to submit a branch through for the same
set of pre-commit checks without submitting a pull request.

## Pre-Commit Checks in Phase 1
* Run and pass PatchCheck.py with no errors

=====================================================

The following are some additional pre-commit check ideas that could be quickly added once the initial version using PatchCheck.py is fully functional. Please provide feedback on the ones you like and additional ones you think may improve the quality of the commits to the edk2 repository.

## Proposed Pre-Commit Checks in Phase 2
* Verify Reviewed-by and Acked-by tags are present with
correct maintainer email addresses
* Verify no non-ASCII characters in modified files
* Verify no binary files in set of modified files
* Verify package dependency rules in modified files

## Proposed Pre-Commit Checks in Phase 3
* Run ECC on modified files
* Verify modified modules/libs build
* Run available host based tests (HBFA) against modified
modules/libs

Best regards,

Mike


Liming Gao
 

Mike:
I add my comments.

-----Original Message-----
From: rfc@edk2.groups.io [mailto:rfc@edk2.groups.io] On Behalf Of Michael
D Kinney
Sent: Friday, August 30, 2019 4:23 AM
To: devel@edk2.groups.io; rfc@edk2.groups.io
Subject: [edk2-rfc] [RFC] EDK II Continuous Integration Phase 1

Hello,

This is a proposal for a first step towards continuous
integration for all TianoCore repositories to help
improve to quality of commits and automate testing and
release processes for all EDK II packages and platforms.

This is based on work from a number of EDK II community
members that have provide valuable input and evaluations.

* Rebecca Cran <rebecca@bsdio.com> Jenkins evaluation
* Laszlo Ersek <lersek@redhat.com> GitLab evaluation
* Philippe Mathieu-Daudé <philmd@redhat.com> GitLab evaluation
* Sean Brogan <sean.brogan@microsoft.com> Azure Pipelines and HBFA
* Bret Barkelew <Bret.Barkelew@microsoft.com> Azure Pipelines and HBFA
* Jiewen Yao <jiewen.yao@intel.com> HBFA

The following link is a link to an EDK II WIKI page that
contains a summary of the work to date. Please provide
feedback in the EDK II mailing lists. The WIKI pages will
be updated with input from the entire EDK II community.

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Continuous-
Integration

Proposal
========
Phase 1 of adding continuous integration is limited to the
edk2 repository. Additional repositories will be added later.

The following changes are proposed:
* Remove EDK II Maintainers write access to edk2 repository.
Only EDK II Administrators will continue to have write
access, and that should only be used to handle extraordinary
events.
* EDK II Maintainers use a GitHub Pull Request instead of push
to commit a patch series to the edk2 repository. There are
no other changes to the development and review process. The
patch series is prepared in an EDK II maintainer branch with
all commit message requirements met on each patch in the series.
Will the maintainer manually provide pull request after the patch passes the review?
Can the script scan the mail list and auto trig pull request once the patch gets
Reviewed-by or Acked-by from Package maintainers?

* The edk2 repository only accepts Pull Requests from members
of the EDK II Maintainers team. Pull Requests from anyone else
are rejected.
* Run pre-commit checks using Azure Pipelines
The maintainer manually trig pre-commit check or auto trig pre-commit?
By default, pre-commit should be auto trigged based on pull request.

* If all pre-commit checks pass, then the patch series is auto
committed. The result of this commit must match the contents
and commit history that would have occurred using the previous
push operation.
* If any pre-commit checks fail, then notify the submitter.
Will Pre-commit check fail send the mail to the patch submitter?
The patch submitter need update the patch and go through review process again.

A typical reason for a failure would be a merge conflict with
another pull request that was just processed.
* Limit pre-commit checks execution time to 10 minutes.
* Provide on-demand builds to EDK II Maintainers that to allow
EDK II Maintainers to submit a branch through for the same
set of pre-commit checks without submitting a pull request.

## Pre-Commit Checks in Phase 1
* Run and pass PatchCheck.py with no errors

=====================================================

The following are some additional pre-commit check ideas
that could be quickly added once the initial version using
PatchCheck.py is fully functional. Please provide feedback
on the ones you like and additional ones you think may
improve the quality of the commits to the edk2 repository.

## Proposed Pre-Commit Checks in Phase 2
* Verify Reviewed-by and Acked-by tags are present with
correct maintainer email addresses
* Verify no non-ASCII characters in modified files
* Verify no binary files in set of modified files
Now, Edk2 has few binary files, like logo.bmp.
I see one BZ to request logo bmp update.
(BZ https://bugzilla.tianocore.org/show_bug.cgi?id=2050)
So, we need the exception way for it.

* Verify package dependency rules in modified files

## Proposed Pre-Commit Checks in Phase 3
* Run ECC on modified files
* Verify modified modules/libs build
* Run available host based tests (HBFA) against modified
modules/libs
I request boot test on Emulator and Ovmf in the daily and weekly scope.
Daily can cover boot to Shell.
Weekly can cover more boot functionality.

Thanks
Liming
Best regards,

Mike







rebecca@...
 

On 2019-08-29 14:22, Michael D Kinney wrote:
This is based on work from a number of EDK II community
members that have provide valuable input and evaluations.

* Rebecca Cran <rebecca@bsdio.com> Jenkins evaluation
* Laszlo Ersek <lersek@redhat.com> GitLab evaluation
* Philippe Mathieu-Daudé <philmd@redhat.com> GitLab evaluation
* Sean Brogan <sean.brogan@microsoft.com> Azure Pipelines and HBFA
* Bret Barkelew <Bret.Barkelew@microsoft.com> Azure Pipelines and HBFA
* Jiewen Yao <jiewen.yao@intel.com> HBFA

The following link is a link to an EDK II WIKI page that
contains a summary of the work to date. Please provide
feedback in the EDK II mailing lists. The WIKI pages will
be updated with input from the entire EDK II community.

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Continuous-Integration


Sorry, I was thinking the evaluation I was doing was having the entire
CI run in Jenkins, not only the boot testing. I don't have the array of
hardware you'd want for boot testing, just mostly large x86 systems. 

So I think I'll drop out of the evaluation.


--

Rebecca Cran


Michael D Kinney
 

Responses below.

Mike

-----Original Message-----
From: Gao, Liming <liming.gao@intel.com>
Sent: Friday, August 30, 2019 1:44 AM
To: rfc@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: RE: [RFC] EDK II Continuous Integration Phase
1

Mike:
I add my comments.

-----Original Message-----
From: rfc@edk2.groups.io [mailto:rfc@edk2.groups.io]
On Behalf Of
Michael D Kinney
Sent: Friday, August 30, 2019 4:23 AM
To: devel@edk2.groups.io; rfc@edk2.groups.io
Subject: [edk2-rfc] [RFC] EDK II Continuous
Integration Phase 1

Hello,

This is a proposal for a first step towards continuous
integration for
all TianoCore repositories to help improve to quality
of commits and
automate testing and release processes for all EDK II
packages and
platforms.

This is based on work from a number of EDK II
community members that
have provide valuable input and evaluations.

* Rebecca Cran <rebecca@bsdio.com> Jenkins evaluation
* Laszlo Ersek <lersek@redhat.com> GitLab evaluation
* Philippe Mathieu-Daudé <philmd@redhat.com> GitLab
evaluation
* Sean Brogan <sean.brogan@microsoft.com> Azure
Pipelines and HBFA
* Bret Barkelew <Bret.Barkelew@microsoft.com> Azure
Pipelines and HBFA
* Jiewen Yao <jiewen.yao@intel.com> HBFA

The following link is a link to an EDK II WIKI page
that contains a
summary of the work to date. Please provide feedback
in the EDK II
mailing lists. The WIKI pages will be updated with
input from the
entire EDK II community.


https://github.com/tianocore/tianocore.github.io/wiki/
EDK-II-Continuous
-
Integration

Proposal
========
Phase 1 of adding continuous integration is limited to
the
edk2 repository. Additional repositories will be
added later.

The following changes are proposed:
* Remove EDK II Maintainers write access to edk2
repository.
Only EDK II Administrators will continue to have
write
access, and that should only be used to handle
extraordinary
events.
* EDK II Maintainers use a GitHub Pull Request instead
of push
to commit a patch series to the edk2 repository.
There are
no other changes to the development and review
process. The
patch series is prepared in an EDK II maintainer
branch with
all commit message requirements met on each patch in
the series.

Will the maintainer manually provide pull request after
the patch passes the review?
Can the script scan the mail list and auto trig pull
request once the patch gets Reviewed-by or Acked-by
from Package maintainers?
The maintainer will do this step manually. After the
required Reviewed-by and Acked-by responses have been
received, the commit messages in the patch series must
be updated to include the Reviewed-by and Acked-by tags.
I do not see an easy way to automate the editing of the
commit messages and guarantee it is always correct.


* The edk2 repository only accepts Pull Requests from
members
of the EDK II Maintainers team. Pull Requests from
anyone else
are rejected.
* Run pre-commit checks using Azure Pipelines
The maintainer manually trig pre-commit check or auto
trig pre-commit?
By default, pre-commit should be auto trigged based on
pull request.
Automatically triggered.

I would be good to provide an on-demand service for maintainers
to run the exact same pre-commit checks on a branch to see
if there are issues that need to be resolved.


* If all pre-commit checks pass, then the patch series
is auto
committed. The result of this commit must match the
contents
and commit history that would have occurred using
the previous
push operation.
* If any pre-commit checks fail, then notify the
submitter.

Will Pre-commit check fail send the mail to the patch
submitter?
Yes.

The patch submitter need update the patch and go
through review process again.
Potentially. If it is a very simple merge conflict with
no functional impacts, then the maintainer should be able
to resolve and resubmit without additional code review.

If there are functional changes to resolve merge conflict,
then another review cycle would be required.


A typical reason for a failure would be a merge
conflict with
another pull request that was just processed.
* Limit pre-commit checks execution time to 10
minutes.
* Provide on-demand builds to EDK II Maintainers that
to allow
EDK II Maintainers to submit a branch through for
the same
set of pre-commit checks without submitting a pull
request.

## Pre-Commit Checks in Phase 1
* Run and pass PatchCheck.py with no errors

=====================================================

The following are some additional pre-commit check
ideas that could be
quickly added once the initial version using
PatchCheck.py is fully
functional. Please provide feedback on the ones you
like and
additional ones you think may improve the quality of
the commits to the
edk2 repository.

## Proposed Pre-Commit Checks in Phase 2
* Verify Reviewed-by and Acked-by tags are present
with
correct maintainer email addresses
* Verify no non-ASCII characters in modified files
* Verify no binary files in set of modified files
Now, Edk2 has few binary files, like logo.bmp.
I see one BZ to request logo bmp update.
(BZ
https://bugzilla.tianocore.org/show_bug.cgi?id=2050)
So, we need the exception way for it.

* Verify package dependency rules in modified files

## Proposed Pre-Commit Checks in Phase 3
* Run ECC on modified files
* Verify modified modules/libs build
* Run available host based tests (HBFA) against
modified
modules/libs
I request boot test on Emulator and Ovmf in the daily
and weekly scope.
Daily can cover boot to Shell.
Weekly can cover more boot functionality.

Thanks
Liming
Best regards,

Mike