Skip to content

Commit

Permalink
Compare Newline only diff.
Browse files Browse the repository at this point in the history
Keep formatted file if unable to diff by code.
Refine error report, so we can at least know the filename in build log.
Let -v same as TestCase StdError, so we check error directly in local build.
  • Loading branch information
yangrongwei committed Apr 6, 2024
1 parent 822b984 commit 51656e3
Showing 1 changed file with 83 additions and 16 deletions.
99 changes: 83 additions & 16 deletions .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,40 @@
from edk2toollib.utility_functions import RunCmd
from io import StringIO
from typing import Any, Dict, List, Tuple
from enum import Enum

class NewLineStyle(Enum):
""" https://en.wikipedia.org/wiki/Newline """
NOTHING = 0 #
POSIX = 1 # \n
DOS = 2 # \r\n
BBC = 3 # \n\r
MAC = 4 # \r

def detect_newline(raw : bytes) -> NewLineStyle:
CR = int.from_bytes(b'\r') # carriage return
LF = int.from_bytes(b'\n') # line feed
first_lf = raw.find(LF)

if -1 == first_lf:
# not found \n
if -1 != raw.find(CR):
return NewLineStyle.MAC
else:
return NewLineStyle.NOTHING
# found \n
if raw[first_lf - 1] == CR:
return NewLineStyle.DOS
elif raw[first_lf + 1] == CR:
return NewLineStyle.BBC
else:
# only \n
return NewLineStyle.POSIX

def detect_newline_of_file(file: str):
with open(file, "rb") as f:
raw = f.read(4 * 1024)
return detect_newline(raw)

#
# Provide more user friendly messages for certain scenarios
Expand Down Expand Up @@ -213,12 +247,14 @@ def _check_for_preexisting_formatted_files(self) -> None:
Existence of such files is an unexpected condition. This might result
from an error that occurred during a previous run or a premature exit from a debug scenario. In any case, the package should be clean before starting a new run.
"""
pre_existing_formatted_file_count = len(
[str(path.resolve()) for path in pathlib.Path(self._abs_package_path).rglob(f'*{UncrustifyCheck.FORMATTED_FILE_EXTENSION}')])
pre_existing_formatted_files = [str(path.resolve()) for path in pathlib.Path(self._abs_package_path).rglob(f'*{UncrustifyCheck.FORMATTED_FILE_EXTENSION}')]
pre_existing_formatted_file_count = len(pre_existing_formatted_files)

if pre_existing_formatted_file_count > 0:
raise UncrustifyStalePluginFormattedFilesException(
f"{pre_existing_formatted_file_count} formatted files already exist. To prevent overwriting these files, please remove them before running this plugin.")
err_str = f"{pre_existing_formatted_file_count} formatted files already exist. \n" \
f"To prevent overwriting these files, please remove them before running this plugin. \n" \
f"{'\n'.join(pre_existing_formatted_files) } \n"
raise UncrustifyStalePluginFormattedFilesException(err_str)

def _cleanup_temporary_directory(self) -> None:
"""
Expand All @@ -236,6 +272,11 @@ def _cleanup_temporary_formatted_files(self) -> None:
This will recursively remove all formatted files generated by Uncrustify
during this execution instance.
"""
# Something wrong with our OutputFileDiffs function, check _process_uncrustify_results(self)
# OR turn off OutputFileDiffs
if hasattr(self, '_keep_formatted_file'):
return

if hasattr(self, '_abs_package_path'):
formatted_files = [str(path.resolve()) for path in pathlib.Path(
self._abs_package_path).rglob(f'*{UncrustifyCheck.FORMATTED_FILE_EXTENSION}')]
Expand All @@ -255,6 +296,7 @@ def _create_temp_working_directory(self) -> None:
except OSError as e:
raise UncrustifyInputFileCreationErrorException(
f"Error creating plugin directory {self._working_dir}.\n\n{repr(e)}.")
logging.info("Working dir: " + self._working_dir)

def _create_uncrustify_file_list_file(self) -> None:
"""
Expand All @@ -266,6 +308,9 @@ def _create_uncrustify_file_list_file(self) -> None:
with open(self._app_input_file_path, 'w', encoding='utf8') as f:
f.writelines(f"\n".join(self._abs_file_paths_to_format))

logging.info("Input file: " + self._app_input_file_path)
logging.info("Files to process: " + "\n".join(self._abs_file_paths_to_format))

def _execute_uncrustify(self) -> None:
"""
Executes Uncrustify with the initialized configuration.
Expand Down Expand Up @@ -449,6 +494,7 @@ def _initialize_config_file_info(self) -> None:
self._app_config_file = os.path.normpath(
os.path.join(self._abs_package_path, self._app_config_file))

logging.info("Using config: " + self._app_config_file)
if not os.path.isfile(self._app_config_file):
raise FileNotFoundError(
errno.ENOENT, os.strerror(errno.ENOENT), self._app_config_file)
Expand All @@ -461,8 +507,7 @@ def _initialize_environment_info(self, package_rel_path: str, edk2_path: Edk2Pat
package_rel_path)
self._abs_workspace_path = edk2_path.WorkspacePath
self._package_config = package_config
self._package_name = os.path.basename(
os.path.normpath(package_rel_path))
self._package_name = os.path.basename(os.path.normpath(package_rel_path))
self._plugin_name = self.__class__.__name__
self._plugin_path = os.path.dirname(os.path.realpath(__file__))
self._rel_package_path = package_rel_path
Expand Down Expand Up @@ -561,19 +606,26 @@ def _log_uncrustify_app_info(self) -> None:
logging.info(f"Uncrustify version: {self._app_version}")
logging.info('\n')

def _dual_out(self, msg : str) -> None:
"""Log to INFO and test case report StdError"""
self._tc.LogStdError(msg)
logging.info(msg)

def _process_uncrustify_results(self) -> None:
"""
Process the results from Uncrustify.
Determines whether formatting errors are present and logs failures.
"""
formatted_files = [str(path.resolve()) for path in pathlib.Path(
self._abs_package_path).rglob(f'*{UncrustifyCheck.FORMATTED_FILE_EXTENSION}')]
formatted_files = [str(path.resolve()) for path in
pathlib.Path(self._abs_package_path).rglob(f'*{UncrustifyCheck.FORMATTED_FILE_EXTENSION}')]

self._formatted_file_error_count = len(formatted_files)

if self._formatted_file_error_count > 0:
logging.warning(f'Uncrustify found {self._formatted_file_error_count} files with formatting errors')
pre_formatted_files = [x[:-len(UncrustifyCheck.FORMATTED_FILE_EXTENSION)] for x in formatted_files]
logging.warning(f"{'\n'.join(pre_formatted_files)}")
self._tc.LogStdError(f"Uncrustify found {self._formatted_file_error_count} files with formatting errors:\n")
logging.critical(
"Visit the following instructions to learn "
Expand All @@ -587,8 +639,10 @@ def _process_uncrustify_results(self) -> None:
for formatted_file in formatted_files:
pre_formatted_file = formatted_file[:-len(UncrustifyCheck.FORMATTED_FILE_EXTENSION)]

self._tc.LogStdError(f"Formatting errors in {os.path.relpath(pre_formatted_file, self._abs_package_path)}\n")
logging.info(f"Formatting errors in {os.path.relpath(pre_formatted_file, self._abs_package_path)}")
self._dual_out(f"Formatting errors in {os.path.relpath(pre_formatted_file, self._abs_package_path)}\n")

logging.info(formatted_file)
logging.info(pre_formatted_file)

if (self._output_file_diffs or
self._file_template_contents is not None or
Expand All @@ -599,20 +653,33 @@ def _process_uncrustify_results(self) -> None:

if (self._file_template_contents is not None and
self._file_template_contents in formatted_file_text):
self._tc.LogStdError(f"File header is missing in {os.path.relpath(pre_formatted_file, self._abs_package_path)}\n")
self._dual_out(f"File header is missing in {os.path.relpath(pre_formatted_file, self._abs_package_path)}\n")

if (self._func_template_contents is not None and
self._func_template_contents in formatted_file_text):
self._tc.LogStdError(f"A function header is missing in {os.path.relpath(pre_formatted_file, self._abs_package_path)}\n")
self._dual_out(f"A function header is missing in {os.path.relpath(pre_formatted_file, self._abs_package_path)}\n")

if self._output_file_diffs:
with open(pre_formatted_file) as pf:
pre_formatted_file_text = pf.read()


content_diff = False
for line in difflib.unified_diff(pre_formatted_file_text.split('\n'), formatted_file_text.split('\n'), fromfile=pre_formatted_file, tofile=formatted_file, n=3):
self._tc.LogStdError(line)

self._tc.LogStdError('\n')
content_diff = True
self._dual_out(line)

if not content_diff:
ff_eol = detect_newline_of_file(formatted_file)
pf_eol = detect_newline_of_file(pre_formatted_file)
self._dual_out(f"EOL is {ff_eol} for {formatted_file}")
self._dual_out(f"EOL is {pf_eol} for {pre_formatted_file}")
if (ff_eol != pf_eol):
self._dual_out("Only Newlines diff.")
else:
self._dual_out("Unable to detect, you may check diff by yourself.")
self._keep_formatted_file = formatted_file

self._dual_out('\n')

def _remove_tree(self, dir_path: str, ignore_errors: bool = False) -> None:
"""
Expand Down

0 comments on commit 51656e3

Please sign in to comment.