Skip to content

Commit d9cdd88

Browse files
Add Line Ending plugin (#1150)
## Description Cherry-pick the LineEndingCheck plugin in its entirety including the various bug fixes and performance improvement fixes it has received over time. Additionally, all line endings for every file were updated to be CRLF to pass the new pipeline check. The commits included from 202311: 8080124 6ddc3a5 59f3382 24a4f42 418f07b These commits do the following: Revert the temporary commit disabling the ARM and ARM64 pipeline builds for CI and Basetools. This ensures that all normal Mu CI is run after this point. --- Fixed a bug where the plugin would fail if not run from the projects root directory. --- Improves performance for the plugin compared to its initial release --- For each item, place an "x" in between `[` and `]` if true. Example: `[x]`. _(you can also check items in the GitHub UI)_ - [ ] 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 N/A ## Integration Instructions N/A --------- Signed-off-by: Michael Kubacki <[email protected]> Co-authored-by: Michael Kubacki <[email protected]>
1 parent f65edd8 commit d9cdd88

File tree

45 files changed

+5264
-4898
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+5264
-4898
lines changed
Lines changed: 324 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,324 @@
1+
# @file LineEndingCheck.py
2+
#
3+
# An edk2-pytool based plugin that checks line endings.
4+
#
5+
# Copyright (c) Microsoft Corporation.
6+
# SPDX-License-Identifier: BSD-2-Clause-Patent
7+
##
8+
9+
import glob
10+
from io import StringIO
11+
import logging
12+
import os
13+
import shutil
14+
from pathlib import Path
15+
from typing import Any, Callable, Dict, List, Tuple
16+
17+
from edk2toolext.environment.plugin_manager import PluginManager
18+
from edk2toolext.environment.plugintypes.ci_build_plugin import ICiBuildPlugin
19+
from edk2toolext.environment.plugintypes.uefi_helper_plugin import \
20+
HelperFunctions
21+
from edk2toolext.environment.var_dict import VarDict
22+
from edk2toollib.gitignore_parser import parse_gitignore_lines
23+
from edk2toollib.log.junit_report_format import JunitReportTestCase
24+
from edk2toollib.uefi.edk2.path_utilities import Edk2Path
25+
from edk2toollib.utility_functions import RunCmd
26+
from git import Repo
27+
28+
29+
PLUGIN_NAME = "LineEndingCheck"
30+
31+
LINE_ENDINGS = [
32+
b'\r\n',
33+
b'\n\r',
34+
b'\n',
35+
b'\r'
36+
]
37+
38+
ALLOWED_LINE_ENDING = b'\r\n'
39+
40+
#
41+
# Based on a solution for binary file detection presented in
42+
# https://stackoverflow.com/a/7392391.
43+
#
44+
_TEXT_CHARS = bytearray(
45+
{7, 8, 9, 10, 12, 13, 27} | set(range(0x20, 0x100)) - {0x7f})
46+
47+
48+
def _is_binary_string(_bytes: bytes) -> bool:
49+
return bool(_bytes.translate(None, _TEXT_CHARS))
50+
51+
52+
class LineEndingCheckBadLineEnding(Exception):
53+
pass
54+
55+
class LineEndingCheckGitIgnoreFileException(Exception):
56+
pass
57+
58+
class LineEndingCheck(ICiBuildPlugin):
59+
"""
60+
A CiBuildPlugin that checks whether line endings are a certain format.
61+
62+
By default, the plugin runs against all files in a package unless a
63+
specific file or file extension is excluded.
64+
65+
Configuration options:
66+
"LineEndingCheck": {
67+
"IgnoreFiles": [], # File patterns to ignore.
68+
}
69+
"""
70+
71+
def GetTestName(self, packagename: str, environment: VarDict) -> Tuple:
72+
""" Provide the testcase name and classname for use in reporting
73+
74+
Args:
75+
packagename: String containing name of package to build.
76+
environment: The VarDict for the test to run in.
77+
78+
Returns:
79+
A tuple containing the testcase name and the classname
80+
(testcasename, classname)
81+
testclassname: a descriptive string for the testcase can
82+
include whitespace
83+
classname: Should be patterned <packagename>.<plugin>
84+
.<optionally any unique condition>
85+
"""
86+
return ("Check line endings in " + packagename, packagename +
87+
"." + PLUGIN_NAME)
88+
89+
# Note: This function access git via the command line
90+
#
91+
# function to check and warn if git config reports that
92+
# autocrlf is configured to TRUE
93+
def _check_autocrlf(self):
94+
r = Repo(self._abs_workspace_path)
95+
try:
96+
result = r.config_reader().get_value("core", "autocrlf")
97+
if result:
98+
logging.warning(f"git config core.autocrlf is set to {result} "
99+
f"recommended setting is false "
100+
f"git config --global core.autocrlf false")
101+
except Exception:
102+
logging.warning(f"git config core.autocrlf is not set "
103+
f"recommended setting is false "
104+
f"git config --global core.autocrlf false")
105+
return
106+
107+
# Note: This function currently accesses git via the git command to prevent
108+
# introducing a new Python git module dependency in mu_basecore
109+
# on gitpython.
110+
#
111+
# After gitpython is adopted by edk2-pytool-extensions, this
112+
# implementation can be updated to use the gitpython interface.
113+
def _get_git_ignored_paths(self) -> List[Path]:
114+
""""
115+
Gets paths ignored by git.
116+
117+
Returns:
118+
List[str]: A list of file absolute path strings to all files
119+
ignored in this git repository.
120+
121+
If git is not found, an empty list will be returned.
122+
"""
123+
if not shutil.which("git"):
124+
logging.warning(
125+
"Git is not found on this system. Git submodule paths will "
126+
"not be considered.")
127+
return []
128+
129+
outstream_buffer = StringIO()
130+
exit_code = RunCmd("git", "ls-files --other",
131+
workingdir=self._abs_workspace_path,
132+
outstream=outstream_buffer,
133+
logging_level=logging.NOTSET)
134+
if (exit_code != 0):
135+
raise LineEndingCheckGitIgnoreFileException(
136+
f"An error occurred reading git ignore settings. This will "
137+
f"prevent LineEndingCheck from running against the expected "
138+
f"set of files.")
139+
140+
# Note: This will potentially be a large list, but at least sorted
141+
rel_paths = outstream_buffer.getvalue().strip().splitlines()
142+
abs_paths = []
143+
for path in rel_paths:
144+
abs_paths.append(Path(
145+
os.path.normpath(os.path.join(self._abs_workspace_path, path))))
146+
return abs_paths
147+
148+
# Note: This function currently accesses git via the git command to prevent
149+
# introducing a new Python git module dependency in mu_basecore
150+
# on gitpython.
151+
#
152+
# After gitpython is adopted by edk2-pytool-extensions, this
153+
# implementation can be updated to use the gitpython interface.
154+
def _get_git_submodule_paths(self) -> List[Path]:
155+
"""
156+
Gets submodule paths recognized by git.
157+
158+
Returns:
159+
List[str]: A list of directory absolute path strings to the root
160+
of each submodule in the workspace repository.
161+
162+
If git is not found, an empty list will be returned.
163+
"""
164+
if not shutil.which("git"):
165+
logging.warning(
166+
"Git is not found on this system. Git submodule paths will "
167+
"not be considered.")
168+
return []
169+
170+
if os.path.isfile(os.path.join(self._abs_workspace_path, ".gitmodules")):
171+
logging.info(
172+
".gitmodules file found. Excluding submodules in "
173+
"LineEndingCheck.")
174+
175+
outstream_buffer = StringIO()
176+
exit_code = RunCmd("git",
177+
"config --file .gitmodules --get-regexp path",
178+
workingdir=self._abs_workspace_path,
179+
outstream=outstream_buffer,
180+
logging_level=logging.NOTSET)
181+
if (exit_code != 0):
182+
raise LineEndingCheckGitIgnoreFileException(
183+
f".gitmodule file detected but an error occurred reading "
184+
f"the file. Cannot proceed with unknown submodule paths.")
185+
186+
submodule_paths = []
187+
for line in outstream_buffer.getvalue().strip().splitlines():
188+
submodule_paths.append(Path(
189+
os.path.normpath(os.path.join(self._abs_workspace_path, line.split()[1]))))
190+
191+
return submodule_paths
192+
else:
193+
return []
194+
195+
def _get_files_ignored_in_config(self,
196+
pkg_config: Dict[str, List[str]],
197+
base_dir: str) -> Callable[[str], bool]:
198+
""""
199+
Returns a function that returns true if a given file string path is
200+
ignored in the plugin configuration file and false otherwise.
201+
202+
Args:
203+
pkg_config: Dictionary with the package configuration
204+
base_dir: Base directory of the package
205+
206+
Returns:
207+
Callable[[None], None]: A test case function.
208+
"""
209+
ignored_files = []
210+
if pkg_config.get("IgnoreFilesWithNoExtension", False):
211+
ignored_files.extend(['*', '!*.*', '!*/'])
212+
if "IgnoreFiles" in pkg_config:
213+
ignored_files.extend(pkg_config["IgnoreFiles"])
214+
215+
# Pass "Package configuration file" as the source file path since
216+
# the actual configuration file name is unknown to this plugin and
217+
# this provides a generic description of the file that provided
218+
# the ignore file content.
219+
#
220+
# This information is only used for reporting (not used here) and
221+
# the ignore lines are being passed directly as they are given to
222+
# this plugin.
223+
return parse_gitignore_lines(ignored_files,
224+
"Package configuration file",
225+
base_dir)
226+
227+
def RunBuildPlugin(self, package_rel_path: str, edk2_path: Edk2Path,
228+
package_config: Dict[str, List[str]],
229+
environment_config: Any,
230+
plugin_manager: PluginManager,
231+
plugin_manager_helper: HelperFunctions,
232+
tc: JunitReportTestCase, output_stream=None) -> int:
233+
"""
234+
External function of plugin. This function is used to perform the task
235+
of the CiBuild Plugin.
236+
237+
Args:
238+
- package_rel_path: edk2 workspace relative path to the package
239+
- edk2_path: Edk2Path object with workspace and packages paths
240+
- package_config: Dictionary with the package configuration
241+
- environment_config: Environment configuration
242+
- plugin_manager: Plugin Manager Instance
243+
- plugin_manager_helper: Plugin Manager Helper Instance
244+
- tc: JUnit test case
245+
- output_stream: The StringIO output stream from this plugin
246+
(logging)
247+
248+
Returns:
249+
>0 : Number of errors found
250+
0 : Ran successfully
251+
-1 : Skipped due to a missing pre-requisite
252+
"""
253+
self._abs_workspace_path = edk2_path.WorkspacePath
254+
self._check_autocrlf()
255+
self._abs_pkg_path = \
256+
edk2_path.GetAbsolutePathOnThisSystemFromEdk2RelativePath(
257+
package_rel_path)
258+
259+
if self._abs_pkg_path is None:
260+
tc.SetSkipped()
261+
tc.LogStdError(f"Package folder not found {self._abs_pkg_path}")
262+
return 0
263+
264+
ignore_files = set(self._get_git_ignored_paths())
265+
ignore_dirs = set(self._get_git_submodule_paths())
266+
ignore_filter = self._get_files_ignored_in_config(package_config, self._abs_pkg_path)
267+
268+
file_count = 0
269+
line_ending_count = dict.fromkeys(LINE_ENDINGS, 0)
270+
for file in Path(self._abs_pkg_path).rglob('*'):
271+
if file.is_dir():
272+
continue
273+
274+
if any(file.is_relative_to(ignore_dir) for ignore_dir in ignore_dirs):
275+
continue
276+
277+
if ignore_filter(file):
278+
continue
279+
280+
if file in ignore_files:
281+
continue
282+
283+
with open(file.resolve(), 'rb') as fb:
284+
if not fb.readable() or _is_binary_string(fb.read(1024)):
285+
continue
286+
fb.seek(0)
287+
288+
for lineno, line in enumerate(fb):
289+
try:
290+
for e in LINE_ENDINGS:
291+
if line.endswith(e):
292+
line_ending_count[e] += 1
293+
294+
if e is not ALLOWED_LINE_ENDING:
295+
file_path = file.relative_to(
296+
Path(self._abs_workspace_path)).as_posix()
297+
file_count += 1
298+
299+
tc.LogStdError(
300+
f"Line ending on Line {lineno} in "
301+
f"{file_path} is not allowed.\nLine "
302+
f"ending is {e} and should be "
303+
f"{ALLOWED_LINE_ENDING}.")
304+
logging.error(
305+
f"Line ending on Line {lineno} in "
306+
f"{file_path} is not allowed.\nLine "
307+
f"ending is {e} and should be "
308+
f"{ALLOWED_LINE_ENDING}.")
309+
raise LineEndingCheckBadLineEnding
310+
break
311+
except LineEndingCheckBadLineEnding:
312+
break
313+
314+
del line_ending_count[ALLOWED_LINE_ENDING]
315+
316+
if any(line_ending_count.values()):
317+
tc.SetFailed(
318+
f"{PLUGIN_NAME} failed due to {file_count} files with "
319+
f"incorrect line endings.",
320+
"CHECK_FAILED")
321+
else:
322+
tc.SetSuccess()
323+
324+
return sum(line_ending_count.values())
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Line Ending Check Plugin
2+
3+
This CiBuildPlugin scans all the files in a package to verify that the line endings are CRLF.
4+
5+
> _Note:_ If you encounter a line ending issue found by this plugin, update your development environment to avoid
6+
> issues again in the future.
7+
>
8+
> Most problems are caused by `autocrlf=true` in git settings, which will automatically adjust line endings upon
9+
> checkout and commit which distorts the actual line endings from being consistent locally and remotely. In
10+
> other cases, developing within a Linux workspace will natively use LF by default.
11+
>
12+
> It is simplest to set `autocrlf=false` to prevent manipulation of line endings outside of the actual values and set
13+
> up your editor to use CRLF line endings within the project.
14+
15+
## Configuration
16+
17+
The plugin can be configured to ignore certain files.
18+
19+
``` yaml
20+
"LineEndingCheck": {
21+
"IgnoreFiles": []
22+
"IgnoreFilesWithNoExtension": False
23+
}
24+
```
25+
26+
### IgnoreFiles
27+
28+
An **optional** list of git ignore patterns relative to the package root used to exclude files from being checked.
29+
30+
### IgnoreFilesWithNoExtension
31+
32+
An **optional** value that, if True, will insert the gitignore rules necessary to have this check ignore files
33+
that do not contain a file extension. Necessary for binary files and/or POSIX like executables.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
## @file
2+
# CiBuildPlugin used to check line ending format.
3+
#
4+
# Copyright (c) Microsoft Corporation.
5+
# SPDX-License-Identifier: BSD-2-Clause-Patent
6+
##
7+
{
8+
"scope": "cibuild",
9+
"name": "Line Ending Check Test",
10+
"module": "LineEndingCheck"
11+
}

.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import os
1313
import pathlib
1414
import shutil
15-
import stat
1615
import timeit
1716
from edk2toolext.environment import version_aggregator
1817
from edk2toolext.environment.plugin_manager import PluginManager
@@ -495,7 +494,6 @@ def _initialize_file_to_format_info(self) -> None:
495494
for path in rel_file_paths_to_format:
496495
self._abs_file_paths_to_format.extend(
497496
[str(path.resolve()) for path in pathlib.Path(self._abs_package_path).rglob(path)])
498-
499497
# Remove files ignore in the plugin configuration file
500498
plugin_ignored_files = list(filter(self._get_files_ignored_in_config(), self._abs_file_paths_to_format))
501499

@@ -641,7 +639,7 @@ def _remove_readonly(func, path, _):
641639
"""
642640
Private function to attempt to change permissions on file/folder being deleted.
643641
"""
644-
os.chmod(path, stat.S_IWRITE)
642+
os.chmod(path, os.stat.S_IWRITE)
645643
func(path)
646644

647645
for _ in range(3): # retry up to 3 times

0 commit comments

Comments
 (0)