Skip to content

Commit

Permalink
[MDLINT_PLUGIN] Add MarkdownLint checker as a CI Plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
spbrogan authored and kenlautner committed May 4, 2023
1 parent 2f5b4f5 commit c599ba5
Show file tree
Hide file tree
Showing 12 changed files with 319 additions and 7 deletions.
18 changes: 18 additions & 0 deletions .azurepipelines/templates/markdownlint-check-prereq-steps.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## @file
# File templates/markdownlint-check-prereq-steps.yml
#
# template file used to install markdownlint checking prerequisite
# depends on spell-check-prereq-steps to run first to set the node version
#
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
##

parameters:
none: ''

steps:

- script: npm install -g markdownlint-cli@0.31.1
displayName: "Install markdown linter"
condition: and(gt(variables.pkg_count, 0), succeeded())
4 changes: 4 additions & 0 deletions .azurepipelines/templates/pr-gate-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ steps:
# install spell check prereqs
- template: spell-check-prereq-steps.yml

# MU_CHANGE - Add MarkdownLint
# install markdownlint check prereqs
- template: markdownlint-check-prereq-steps.yml

# Build repo
- task: CmdLine@1
displayName: Setup ${{ parameters.build_pkgs }} ${{ parameters.build_archs}}
Expand Down
14 changes: 14 additions & 0 deletions .markdownlint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
## @file
# markdownlint configuration
#
# Copyright (c) Microsoft Corporation
# SPDX-License-Identifier: BSD-2-Clause-Patent
##

# Rules can be found here: https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md
# Config info: https://github.com/DavidAnson/markdownlint#configuration
{
"default": true,
"MD013": {"line_length": 120, "code_blocks": false, "tables": false},
"MD033": {"allowed_elements": ["br"]}
}
5 changes: 5 additions & 0 deletions .markdownlintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Ignore build directory
/Build/

# Ignore external dependencies
*_extdep/
174 changes: 174 additions & 0 deletions .pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
# @file MarkdownLintCheck.py
#
# An edk2-pytool based plugin wrapper for markdownlint
#
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
import logging
import json
import yaml
from io import StringIO
import os
from typing import List
from edk2toolext.environment.plugintypes.ci_build_plugin import ICiBuildPlugin
from edk2toollib.utility_functions import RunCmd
from edk2toolext.environment.var_dict import VarDict
from edk2toolext.environment import version_aggregator


class MarkdownLintCheck(ICiBuildPlugin):
"""
A CiBuildPlugin that uses the markdownlint-cli node module to scan the files
from the package being tested for linter errors.
The linter config file (.markdownlint.yaml) must be present in one of the defined
locations otherwise the test will be skipped. These locations were picked to also
align with how editors and tools will find the config file.
1st priority location - At the package root
2nd Priority location - At the workspace root of the build (suggested location unless override needed)
Configuration options:
"MarkdownLintCheck": {
"AuditOnly": False, # If True, log all errors and then mark as skipped
"IgnoreFiles": [] # package root relative file, folder, or glob pattern to ignore
}
"""

CONFIG_FILE_NAME = ".markdownlint.yaml"

def GetTestName(self, packagename: str, environment: VarDict) -> tuple:
""" Provide the testcase name and classname for use in reporting
Args:
packagename: string containing name of package to build
environment: The VarDict for the test to run in
Returns:
a tuple containing the testcase name and the classname
(testcasename, classname)
testclassname: a descriptive string for the testcase can include whitespace
classname: should be patterned <packagename>.<plugin>.<optionally any unique condition>
"""
return ("Lint Markdown files in " + packagename, packagename + ".markdownlint")

##
# External function of plugin. This function is used to perform the task of the CiBuild Plugin
#
# - package is the edk2 path to package. This means workspace/packagepath relative.
# - edk2path object configured with workspace and packages path
# - PkgConfig Object (dict) for the pkg
# - EnvConfig Object
# - Plugin Manager Instance
# - Plugin Helper Obj Instance
# - Junit Logger
# - output_stream the StringIO output stream from this plugin via logging

def RunBuildPlugin(self, packagename, Edk2pathObj, pkgconfig, environment, PLM, PLMHelper, tc, output_stream=None):
Errors = []

abs_pkg_path = Edk2pathObj.GetAbsolutePathOnThisSystemFromEdk2RelativePath(
packagename)

if abs_pkg_path is None:
tc.SetSkipped()
tc.LogStdError("No package {0}".format(packagename))
return -1

# check for node
return_buffer = StringIO()
ret = RunCmd("node", "--version", outstream=return_buffer)
if (ret != 0):
tc.SetSkipped()
tc.LogStdError("NodeJs not installed. Test can't run")
logging.warning("NodeJs not installed. Test can't run")
return -1
node_version = return_buffer.getvalue().strip() # format vXX.XX.XX
tc.LogStdOut(f"Node version: {node_version}")

# Check for markdownlint-cli
return_buffer = StringIO()
ret = RunCmd("markdownlint", "--version", outstream=return_buffer)
if (ret != 0):
tc.SetSkipped()
tc.LogStdError("markdownlint not installed. Test can't run")
logging.warning("markdownlint not installed. Test can't run")
return -1
mdl_version = return_buffer.getvalue().strip() # format XX.XX.XX
tc.LogStdOut(f"MarkdownLint version: {mdl_version}")
version_aggregator.GetVersionAggregator().ReportVersion(
"MarkDownLint", mdl_version, version_aggregator.VersionTypes.INFO)

# Get relative path for the root of package to use with ignore and path parameters
relpath = os.path.relpath(abs_pkg_path)

#
# check for any package specific ignore patterns defined by package config
#
Ignores = []
if("IgnoreFiles" in pkgconfig):
for i in pkgconfig["IgnoreFiles"]:
Ignores.append(f"{relpath}/{i}")

#
# Make the path string to check
#
path_to_check = f'{relpath}/**/*.md'

# get path to config file -

# Currently there is support for two different config files
# If the config file is not found then the test case is skipped
#
# 1st - At the package root
# 2nd - At the workspace root of the build
config_file_path = None

# 1st check to see if the config file is at package root
if os.path.isfile(os.path.join(abs_pkg_path, MarkdownLintCheck.CONFIG_FILE_NAME)):
config_file_path = os.path.join(abs_pkg_path, MarkdownLintCheck.CONFIG_FILE_NAME)

# 2nd check to see if at workspace root
elif os.path.isfile(os.path.join(Edk2pathObj.WorkspacePath, MarkdownLintCheck.CONFIG_FILE_NAME)):
config_file_path = os.path.join(Edk2pathObj.WorkspacePath, MarkdownLintCheck.CONFIG_FILE_NAME)

# If not found - skip test
else:
tc.SetSkipped()
tc.LogStdError(f"{MarkdownLintCheck.CONFIG_FILE_NAME} not found. Skipping test")
logging.warning(f"{MarkdownLintCheck.CONFIG_FILE_NAME} not found. Skipping test")
return -1


# Run the linter
results = self._check_markdown(path_to_check, config_file_path, Ignores)
for r in results:
tc.LogStdError(r.strip())

# add result to test case
overall_status = len(results)
if overall_status != 0:
if "AuditOnly" in pkgconfig and pkgconfig["AuditOnly"]:
# set as skipped if AuditOnly
tc.SetSkipped()
return -1
else:
tc.SetFailed("Markdown Lint Check {0} Failed. Errors {1}".format(
packagename, overall_status), "CHECK_FAILED")
else:
tc.SetSuccess()
return overall_status

def _check_markdown(self, rel_file_to_check: os.PathLike, abs_config_file_to_use: os.PathLike, Ignores: List) -> []:
output = StringIO()
param = f"--config {abs_config_file_to_use}"
for a in Ignores:
param += f' --ignore "{a}"'
param += f' "{rel_file_to_check}"'

ret = RunCmd(
"markdownlint", param, outstream=output)
if ret == 0:
return []
else:
return output.getvalue().strip().splitlines()
11 changes: 11 additions & 0 deletions .pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck_plug_in.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## @file
# CiBuildPlugin used to lint repository documentation in markdown files
#
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
{
"scope": "cibuild",
"name": "Markdown Lint Test",
"module": "MarkdownLintCheck"
}
68 changes: 68 additions & 0 deletions .pytool/Plugin/MarkdownLintCheck/Readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Markdown Lint Plugin

This CiBuildPlugin scans all the markdown files in a given package and
checks for linter errors.

## Requirements

The test case in this plugin will be skipped if the requirements are not met.

1. NodeJs installed and on your path
2. `markdownlint-cli` NodeJs package installed
3. a `.markdownlint.yaml` config file either at your repository root or package root.

* NodeJS: <https://nodejs.org/en/>
* markdownlint-cli: <https://www.npmjs.com/package/markdownlint-cli>
* Src available: <https://github.com/igorshubovych/markdownlint-cli>

## Configuration

It is desired to use standard configuration methods so that both local editors (vscode, etc)
and the CI process leverage the same configuration. This mostly works but for ignoring
files there is currently a small discrepancy.

First there is/can be a `.markdownlintignore` file at root of the repository. This
file much like a `.gitignore` is great for broadly ignoring files with patterns. This
works for both editor/ci.

But for package based ignores and to keep the control of which files to ignore within the package
there is no answer that supports both CI and editors. Open question here
<https://github.com/DavidAnson/vscode-markdownlint/issues/130>

For the CI plugin you can use the IgnoreFiles configuration option described in the Plugin Configuration.

## Plugin Configuration

The plugin has only minimal configuration options to support the UEFI codebase.

``` yaml
"MarkdownLintCheck": {
"AuditOnly": False, # If True, log all errors and then mark as skipped
"IgnoreFiles": [] # package root relative file, folder, or glob pattern to ignore
}
```

### 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 the failures without breaking the build.

### IgnoreFiles

This supports package relative files, folders, and glob patterns to ignore.
These are passed to the markdownlint-cli tool as quoted -i parameters.

## Linter Configuration

All configuration options available to the linter can be set in the `.markdownlint.yaml`.
This includes customizing rule options and enforcement.
See more details here: <https://github.com/DavidAnson/markdownlint#configuration>
Linter Rules are described here: <https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md>

## Rule Overrides

There are times when a certain rule should not apply to part of a markdown file.
Markdownlint has numerous ways to configure this.
See the in file Configuration options described at the links above
5 changes: 2 additions & 3 deletions .pytool/Plugin/SpellCheck/cspell.base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@
"ucrtd",
"msvcrtd",
"XIPFLAGS",
"markdownlint",
"bootflow",
"bootup",
"cacheability",
Expand Down Expand Up @@ -290,8 +291,6 @@
"unrecovered",
"cmocka",
"unenrolling",
"unconfigure",
"Loongson",
"LOONGARCH"
"unconfigure"
]
}
8 changes: 4 additions & 4 deletions BaseTools/Source/Python/README.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
# Edk2 Basetools

This folder has traditionally held the source of Python based tools used by EDK2.
The official repo this source has moved to https://github.com/tianocore/edk2-basetools.
The official repo this source has moved to <https://github.com/tianocore/edk2-basetools>.
This folder will remain in the tree until the next stable release (expected 202102).
There is a new folder under Basetools `BinPipWrappers` that uses the pip module rather than this tree for Basetools.
By adding the scope `pipbuild-win` or `pipbuild-unix` (depending on your host system), the SDE will use the
`BinPipWrappers` instead of the regular `BinWrappers`.

## Why Move It?
## Why Move It

The discussion is on the mailing list. The RFC is here: https://edk2.groups.io/g/rfc/topic/74009714#270
The discussion is on the mailing list. The RFC is here: <https://edk2.groups.io/g/rfc/topic/74009714#270>
The benefits allow for the Basetools project to be used separately from EDK2 itself as well as offering it in a
globally accessible manner.
This makes it much easier to build a module using Basetools.
Separating the Basetools into their own repo allows for easier CI and contribution process.
Additional pros, cons, and process can be found on the mailing list.

## How Do I Install It?
## How Do I Install It

By default, EDK2 is tied to and tested with a specific version of the Basetools through `pip-requirements.txt`.
You can simply run:
Expand Down
6 changes: 6 additions & 0 deletions CryptoPkg/CryptoPkg.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@
"LibraryClassCheck": {
"IgnoreHeaderFile": []
},
"MarkdownLintCheck": {
"AuditOnly": False, # If True, log all errors and then mark as skipped
"IgnoreFiles": [
"Library/OpensslLib/openssl"
] # package root relative file, folder, or glob pattern to ignore
},

## options defined ci/Plugin/SpellCheck
"SpellCheck": {
Expand Down
7 changes: 7 additions & 0 deletions MdeModulePkg/MdeModulePkg.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -118,5 +118,12 @@
"canthave"
],
"AdditionalIncludePaths": [] # Additional paths to spell check relative to package root (wildcards supported)
},

## options defined .pytool/Plugin/MarkdownLintCheck
"MarkdownLintCheck": {
"IgnoreFiles": [ "Universal/RegularExpressionDxe/oniguruma", # submodule outside of control
"Library/BrotliCustomDecompressLib/brotli" # submodule outside of control
] # package root relative file, folder, or glob pattern to ignore
}
}
6 changes: 6 additions & 0 deletions UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,11 @@
],
"IgnoreStandardPaths": [], # Standard Plugin defined paths that should be ignore
"AdditionalIncludePaths": [] # Additional paths to spell check (wildcards supported)
},

## options defined .pytool/Plugin/MarkdownLintCheck
"MarkdownLintCheck": {
"IgnoreFiles": [ "Library/CmockaLib/cmocka" # cmocka is submodule outside of control
] # package root relative file, folder, or glob pattern to ignore
}
}

0 comments on commit c599ba5

Please sign in to comment.