Skip to content

Commit

Permalink
Add CI Plugin: NestedPackageCheck (microsoft#509)
Browse files Browse the repository at this point in the history
## Description

Adds a CI Plugin that verifies there is no nested package conflicts in
the target package.

This will eventually replace the check that occurs everytime Edk2Path is
instantiated in edk2-pytool-extensions.

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

Verified that manually adding nested packages are:

1. Detected and reported for the target package only
2. Reported but not build breaking when AuditOnly is true.

## Integration Instructions

N/A

---------

Signed-off-by: Joey Vagedes <[email protected]>
Co-authored-by: Michael Kubacki <[email protected]>
  • Loading branch information
2 people authored and kenlautner committed Dec 18, 2023
1 parent 2aa8e69 commit ded54d9
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 0 deletions.
61 changes: 61 additions & 0 deletions .pytool/Plugin/NestedPackageCheck/NestedPackageCheck.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# @file NestedPackageCheck.py
#
# A CI Check that verifies there are no nested packages in the package being tested.
# Review the readme for a description of nested packages and why they are not allowed.
#
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
from edk2toolext.environment.plugintypes.ci_build_plugin import ICiBuildPlugin
from edk2toolext.environment.var_dict import VarDict
from pathlib import Path


class NestedPackageCheck(ICiBuildPlugin):
"""CiBuildPlugin that finds all nested packages in the workspace and fails if any are found in
the target package.
Configuration Options:
"NestedPackageCheck": {
"AuditOnly": False,
}
"""
def GetTestName(self, packagename: str, environment: VarDict) -> tuple:
return (f"Check for nested packages in {packagename}", f"{packagename}.nestedpackagecheck")

def RunBuildPlugin(self, packagename, edk2path, pkgconfig, env, PLM, PLMHelper, tc, output_stream=None) -> int:
target_package = Path(edk2path.GetAbsolutePathOnThisSystemFromEdk2RelativePath(packagename))

# Get all packages in the entire workspace
package_path_packages = {}
for package_path in edk2path.PackagePathList:
package_path_packages[package_path] = \
[p.parent for p in Path(package_path).glob('**/*.dec')]

# Find any nested packages in the workspace
nested_packages = []
for package_path, packages_to_check in package_path_packages.items():
for i, package in enumerate(packages_to_check):
for j in range(i + 1, len(packages_to_check)):
comp_package = packages_to_check[j]
if package.is_relative_to(comp_package) or comp_package.is_relative_to(package):
nested_packages.append((package_path, package, comp_package))

# Record only nested packages in the target package
failed = 0
for conflict in nested_packages:
if target_package in conflict:
tc.LogStdError(f"Nested package detected: {conflict[1]} and {conflict[2]}")
failed += 1

# Don't fail if in AuditOnly mode. Just skip the test.
if failed > 0:
if pkgconfig.get("AuditOnly", False):
tc.SetSkipped()
return -1
else:
tc.SetFailed(f"Nested Packages Check {packagename} failed. Errors: {failed}", "CHECK_FAILED")
return failed

tc.SetSuccess()
return 0
12 changes: 12 additions & 0 deletions .pytool/Plugin/NestedPackageCheck/NestedPackageCheck_plug_in.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## @file
# CiBuildPlugin used to check for nested packages in the repo.
# Ignores packages coming from submodules / subrepos
#
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
{
"scope": "cibuild",
"name": "Nested Package Check",
"module": "NestedPackageCheck"
}
60 changes: 60 additions & 0 deletions .pytool/Plugin/NestedPackageCheck/Readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Nested Package Check Plugin

This CiBuildPlugin detects nested packages and fails if the nested package is due to the currently tested package

## What is a Nested Package?

As the name suggests, a nested package is when one package is located in a subdirectory of another package. Depending on
multiple factors (such as package layouts, the `PACKAGES_PATH` environment variable, and the order of paths in the
`PACKAGES_PATH ` environment variable), nested packages can cause hard to diagnose, or silent, build issues due to
incorrect path resolution. Due to this, the DEC Specification requires that EDK II packages cannot be nested within
other EDKII Packages.

[DEC Specification v1.27](https://tianocore-docs.github.io/edk2-DecSpecification/release-1.27/2_dec_file_overview/#2-dec-file-overview)
\- *"An EDK II Package (directory) is a directory that contains an EDK II package declaration (DEC) file. Only one DEC
file is permitted per directory. EDK II Packages cannot be nested within other EDK II Packages."*

### Nested Package Example

```cmd
# An invalid layout of packages due to nested packages
c:/src/EDK2 # WORKSPACE
├── MdePkg # PACKAGE - Valid
│ ├── MdePkg.dec
│ ├── MdeModulePkg # PACKAGE - Nested
│ │ └── MdeModulePkg.dec
│ └── Include
│ └── CryptoPkg # PACKAGE - Nested
│ └── CryptoPkg.dec
└── NetworkPkg
└── NetworkPkg.dec # PACKAGE - Valid
# A valid layout of packages
c:/src/EDK2 # WORKSPACE
├── MdePkg
│ └── MdePkg.dec
├── MdeModulePkg
│ └── MdeModulePkg.dec
├── CryptoPkg
│ └── CryptoPkg.dec
└── NetworkPkg
└── NetworkPkg.dec
```

## Plugin Configuration

``` yaml
"NestedPackageCheck": {
"AuditOnly": False,
}

```

### AuditOnly

Boolean - Default is False.
If True, run the test in an Audit only mode which will log all errors but instead of failing the
build, it will set the test as skipped. This allows visibility into failures without breaking the
build.

0 comments on commit ded54d9

Please sign in to comment.