From 90fb2a07521e0520a3bd24df066008b71846c9a6 Mon Sep 17 00:00:00 2001 From: Sean Brogan Date: Tue, 13 Oct 2020 16:14:54 -0700 Subject: [PATCH] [MDLINT_PLUGIN] Add MarkdownLint checker as a CI Plugin --- .../markdownlint-check-prereq-steps.yml | 18 ++ .azurepipelines/templates/pr-gate-steps.yml | 154 +++++++++++++++ .../MarkdownLintCheck/MarkdownLintCheck.py | 177 ++++++++++++++++++ .../MarkdownLintCheck_plug_in.yaml | 11 ++ .pytool/Plugin/MarkdownLintCheck/Readme.md | 68 +++++++ BaseTools/Source/Python/README.md | 8 +- MdeModulePkg/MdeModulePkg.ci.yaml | 7 + .../UnitTestFrameworkPkg.ci.yaml | 10 +- 8 files changed, 448 insertions(+), 5 deletions(-) create mode 100644 .azurepipelines/templates/markdownlint-check-prereq-steps.yml create mode 100644 .azurepipelines/templates/pr-gate-steps.yml create mode 100644 .pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck.py create mode 100644 .pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck_plug_in.yaml create mode 100644 .pytool/Plugin/MarkdownLintCheck/Readme.md diff --git a/.azurepipelines/templates/markdownlint-check-prereq-steps.yml b/.azurepipelines/templates/markdownlint-check-prereq-steps.yml new file mode 100644 index 00000000000..5af643faf51 --- /dev/null +++ b/.azurepipelines/templates/markdownlint-check-prereq-steps.yml @@ -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()) diff --git a/.azurepipelines/templates/pr-gate-steps.yml b/.azurepipelines/templates/pr-gate-steps.yml new file mode 100644 index 00000000000..6f45f656fd9 --- /dev/null +++ b/.azurepipelines/templates/pr-gate-steps.yml @@ -0,0 +1,154 @@ +## @file +# File templates/pr-gate-steps.yml +# +# template file containing the steps to build +# +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +parameters: + tool_chain_tag: '' + build_pkgs: '' + build_targets: '' + build_archs: '' + usePythonVersion: '' + extra_install_step: [] + +steps: +- bash: | + echo "##vso[task.prependpath]${HOME}/.local/bin" + echo "new PATH=${PATH}" + displayName: Set PATH + condition: eq('${{ parameters.tool_chain_tag }}', 'GCC5') + +- checkout: self + clean: true + fetchDepth: 1 + +- task: UsePythonVersion@0 + inputs: + versionSpec: ${{ parameters.usePythonVersion }} + architecture: "x64" + condition: ne('${{ parameters.usePythonVersion }}', '') + +- script: pip install -r pip-requirements.txt --upgrade + displayName: 'Install/Upgrade pip modules' + +# Set default +- bash: | + echo "##vso[task.setvariable variable=pkgs_to_build]${{ parameters.build_pkgs }}" + echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}" + +# Fetch the target branch so that pr_eval can diff them. +# Seems like azure pipelines/github changed checkout process in nov 2020. +- script: git fetch origin $(System.PullRequest.targetBranch) + displayName: fetch target branch + condition: eq(variables['Build.Reason'], 'PullRequest') + +- ${{ parameters.extra_install_step }} + +# trim the package list if this is a PR +- task: CmdLine@1 + displayName: Check if ${{ parameters.build_pkgs }} need testing + inputs: + filename: stuart_pr_eval + arguments: -c .pytool/CISettings.py -p ${{ parameters.build_pkgs }} --pr-target origin/$(System.PullRequest.targetBranch) --output-csv-format-string "##vso[task.setvariable variable=pkgs_to_build;isOutpout=true]{pkgcsv}" --output-count-format-string "##vso[task.setvariable variable=pkg_count;isOutpout=true]{pkgcount}" + condition: eq(variables['Build.Reason'], 'PullRequest') + +# 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}} + inputs: + filename: stuart_setup + arguments: -c .pytool/CISettings.py -p $(pkgs_to_build) -t ${{ parameters.build_targets}} -a ${{ parameters.build_archs}} TOOL_CHAIN_TAG=${{ parameters.tool_chain_tag}} + condition: and(gt(variables.pkg_count, 0), succeeded()) + +- task: CmdLine@1 + displayName: Update ${{ parameters.build_pkgs }} ${{ parameters.build_archs}} + inputs: + filename: stuart_update + arguments: -c .pytool/CISettings.py -p $(pkgs_to_build) -t ${{ parameters.build_targets}} -a ${{ parameters.build_archs}} TOOL_CHAIN_TAG=${{ parameters.tool_chain_tag}} + condition: and(gt(variables.pkg_count, 0), succeeded()) + +# build basetools +# do this after setup and update so that code base dependencies +# are all resolved. +- template: basetools-build-steps.yml + parameters: + tool_chain_tag: ${{ parameters.tool_chain_tag }} + +- task: CmdLine@1 + displayName: Build and Test ${{ parameters.build_pkgs }} ${{ parameters.build_archs}} + inputs: + filename: stuart_ci_build + arguments: -c .pytool/CISettings.py -p $(pkgs_to_build) -t ${{ parameters.build_targets}} -a ${{ parameters.build_archs}} TOOL_CHAIN_TAG=${{ parameters.tool_chain_tag}} + condition: and(gt(variables.pkg_count, 0), succeeded()) + +# Publish Test Results to Azure Pipelines/TFS +- task: PublishTestResults@2 + displayName: 'Publish junit test results' + continueOnError: true + condition: and( succeededOrFailed(),gt(variables.pkg_count, 0)) + inputs: + testResultsFormat: 'JUnit' # Options: JUnit, NUnit, VSTest, xUnit + testResultsFiles: 'Build/TestSuites.xml' + #searchFolder: '$(System.DefaultWorkingDirectory)' # Optional + mergeTestResults: true # Optional + testRunTitle: $(System.JobName) # Optional + #buildPlatform: # Optional + #buildConfiguration: # Optional + publishRunAttachments: true # Optional + +# Publish Test Results to Azure Pipelines/TFS +- task: PublishTestResults@2 + displayName: 'Publish host based test results for $(System.JobName)' + continueOnError: true + condition: and( succeededOrFailed(), gt(variables.pkg_count, 0)) + inputs: + testResultsFormat: 'JUnit' # Options: JUnit, NUnit, VSTest, xUnit + testResultsFiles: 'Build/**/*.result.xml' + #searchFolder: '$(System.DefaultWorkingDirectory)' # Optional + mergeTestResults: false # Optional + testRunTitle: ${{ parameters.build_pkgs }} # Optional + #buildPlatform: # Optional + #buildConfiguration: # Optional + publishRunAttachments: true # Optional + +# Copy the build logs to the artifact staging directory +- task: CopyFiles@2 + displayName: "Copy build logs" + inputs: + targetFolder: '$(Build.ArtifactStagingDirectory)' + SourceFolder: 'Build' + contents: | + BUILDLOG_*.txt + BUILDLOG_*.md + CI_*.txt + CI_*.md + CISETUP.txt + SETUPLOG.txt + UPDATE_LOG.txt + PREVALLOG.txt + TestSuites.xml + **/BUILD_TOOLS_REPORT.html + **/OVERRIDELOG.TXT + coverage.xml + flattenFolders: true + condition: succeededOrFailed() + +# Publish build artifacts to Azure Artifacts/TFS or a file share +- task: PublishBuildArtifacts@1 + continueOnError: true + displayName: "Publish build logs" + inputs: + pathtoPublish: '$(Build.ArtifactStagingDirectory)' + artifactName: 'Build Logs $(System.JobName)' + condition: succeededOrFailed() diff --git a/.pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck.py b/.pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck.py new file mode 100644 index 00000000000..56b32075341 --- /dev/null +++ b/.pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck.py @@ -0,0 +1,177 @@ +# @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 .. + """ + 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) + + # Newer versions of markdownlint don't understand backslashes + relpath = relpath.replace(os.path.sep, '/') + + # + # 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() diff --git a/.pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck_plug_in.yaml b/.pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck_plug_in.yaml new file mode 100644 index 00000000000..53d8172e180 --- /dev/null +++ b/.pytool/Plugin/MarkdownLintCheck/MarkdownLintCheck_plug_in.yaml @@ -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" +} diff --git a/.pytool/Plugin/MarkdownLintCheck/Readme.md b/.pytool/Plugin/MarkdownLintCheck/Readme.md new file mode 100644 index 00000000000..1196ebe1790 --- /dev/null +++ b/.pytool/Plugin/MarkdownLintCheck/Readme.md @@ -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: +* markdownlint-cli: + * Src available: + +## 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 + + +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: +Linter Rules are described here: + +## 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 diff --git a/BaseTools/Source/Python/README.md b/BaseTools/Source/Python/README.md index 8c4d9e73bbe..75f64c5242a 100644 --- a/BaseTools/Source/Python/README.md +++ b/BaseTools/Source/Python/README.md @@ -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 . 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: 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: diff --git a/MdeModulePkg/MdeModulePkg.ci.yaml b/MdeModulePkg/MdeModulePkg.ci.yaml index 85c3287643f..90a9b9a0f50 100644 --- a/MdeModulePkg/MdeModulePkg.ci.yaml +++ b/MdeModulePkg/MdeModulePkg.ci.yaml @@ -122,5 +122,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 } } diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml index 293e4482258..4e0229bef5b 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml @@ -127,6 +127,14 @@ "Library/CmockaLib/cmocka/**", "Library/GoogleTestLib/googletest/**", "Library/SubhookLib/subhook/**" - ] + ], + }, + + ## options defined .pytool/Plugin/MarkdownLintCheck + "MarkdownLintCheck": { + "IgnoreFiles": [ + "Library/CmockaLib/cmocka", # cmocka is submodule outside of control + "Library/GoogleTestLib/googletest" # googletest is submodule outside of control + ] # package root relative file, folder, or glob pattern to ignore } }