From d9b86f01c22c04dffa6861086ff4f23d32e23e31 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Tue, 8 Nov 2022 11:00:51 -0500 Subject: [PATCH 1/4] Fix typos in robot files (#334) Signed-off-by: Michael Kubacki --- integration_test/Shared_Keywords.robot | 4 ++-- integration_test/edk2_stuart_pr_eval.robot | 16 ++++++++-------- integration_test/mu_stuart_pr_eval.robot | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/integration_test/Shared_Keywords.robot b/integration_test/Shared_Keywords.robot index 5ed744250..0f00578d6 100644 --- a/integration_test/Shared_Keywords.robot +++ b/integration_test/Shared_Keywords.robot @@ -159,10 +159,10 @@ Stuart platform build Should Be Equal As Integers ${result.rc} 0 Stuart platform run - [Arguments] ${setting_file} ${arch} ${target} ${tool_chain} ${addtional_flags} ${ws} + [Arguments] ${setting_file} ${arch} ${target} ${tool_chain} ${additional_flags} ${ws} Log to console Stuart Build Run ${result}= Run Process stuart_build - ... -c ${setting_file} -a ${arch} TOOL_CHAIN_TAG\=${tool_chain} TARGET\=${target} --FlashOnly ${addtional_flags} + ... -c ${setting_file} -a ${arch} TOOL_CHAIN_TAG\=${tool_chain} TARGET\=${target} --FlashOnly ${additional_flags} ... cwd=${ws} stdout=stdout.txt stderr=stderr.txt Log Many stdout: ${result.stdout} stderr: ${result.stderr} Should Be Equal As Integers ${result.rc} 0 diff --git a/integration_test/edk2_stuart_pr_eval.robot b/integration_test/edk2_stuart_pr_eval.robot index a41c0e558..c0a81ae67 100644 --- a/integration_test/edk2_stuart_pr_eval.robot +++ b/integration_test/edk2_stuart_pr_eval.robot @@ -168,7 +168,7 @@ Test Stuart PR for Policy 4 module c file changed that platform dsc depends on Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Confirm same contents ${pkgs} OvmfPkg @@ -189,7 +189,7 @@ Test Stuart PR for Policy 4 module c file changed that platform dsc does not dep Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -209,7 +209,7 @@ Test Stuart PR for all policies when a PR contains a deleted file Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -229,7 +229,7 @@ Test Stuart PR for all policies when a PR contains a deleted folder Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -249,7 +249,7 @@ Test Stuart PR for all policies when a PR contains multiple levels of deleted fo Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Confirm same contents ${pkgs} OvmfPkg @@ -270,7 +270,7 @@ Test Stuart PR for all policies when a PR contains file added Stage changed file ${location_to_move} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Confirm same contents ${pkgs} OvmfPkg @@ -291,7 +291,7 @@ Test Stuart PR for all policies when a PR contains directory added Stage changed file ${location_to_move} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -312,7 +312,7 @@ Test Stuart PR for changing a file at the root of repo Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${platform_ci_file} OvmfPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} diff --git a/integration_test/mu_stuart_pr_eval.robot b/integration_test/mu_stuart_pr_eval.robot index 015b95198..f7afc3748 100644 --- a/integration_test/mu_stuart_pr_eval.robot +++ b/integration_test/mu_stuart_pr_eval.robot @@ -195,7 +195,7 @@ Test Stuart PR using ProjectMu for all policies when a PR contains a deleted fol Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${core_ci_file} PcAtChipsetPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -219,7 +219,7 @@ Test Stuart PR using ProjectMu for all policies when a PR contains a deleted fol # Stage changed file ${file_to_modify} ${ws_root} # Commit changes "Changes" ${ws_root} - # # Platform CI test DSC dependnency on implementation file # Policy 4 + # # Platform CI test DSC dependency on implementation file # Policy 4 # ${pkgs}= Stuart pr evaluation ${core_ci_file} SecurityPkg ${default_branch} ${EMPTY} ${ws_root} # Should Be Empty ${pkgs} @@ -240,7 +240,7 @@ Test Stuart PR using ProjectMu for all policies when a PR contains file added Stage changed file ${location_to_move} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${core_ci_file} MdeModulePkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -261,7 +261,7 @@ Test Stuart PR for all policies when a PR contains directory added Stage changed file ${location_to_move} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${core_ci_file} UefiCpuPkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} @@ -282,7 +282,7 @@ Test Stuart PR for changing a file at the root of repo Stage changed file ${file_to_modify} ${ws_root} Commit changes "Changes" ${ws_root} - # Platform CI test DSC dependnency on implementation file # Policy 4 + # Platform CI test DSC dependency on implementation file # Policy 4 ${pkgs}= Stuart pr evaluation ${core_ci_file} MdePkg ${default_branch} ${EMPTY} ${ws_root} Should Be Empty ${pkgs} From fdc72c3c507089a15aa0d89c487dc4668617ebb4 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Tue, 8 Nov 2022 15:31:04 -0800 Subject: [PATCH 2/4] Pydocstyle Updates (#343) Updates pipeline to only run on the edk2toolext folder. Updates developing documentation to specify to the user that they should only run pydocstyle on the edk2toolext folder. Adds documentation, rather then noqa to empty package init files. --- BasicDevTests.py | 8 ++++---- azure-pipelines/templates/build-test-job.yml | 2 ++ azure-pipelines/templates/pydocstyle-test-steps.yml | 6 +++--- docs/developing.md | 2 +- edk2toolext/__init__.py | 4 +++- edk2toolext/bin/__init__.py | 4 +++- edk2toolext/environment/__init__.py | 4 +++- edk2toolext/environment/extdeptypes/__init__.py | 3 +-- edk2toolext/environment/plugintypes/__init__.py | 3 +-- edk2toolext/invocables/__init__.py | 3 +-- edk2toolext/tests/__init__.py | 4 +++- 11 files changed, 25 insertions(+), 18 deletions(-) diff --git a/BasicDevTests.py b/BasicDevTests.py index 0a268221d..517cc59ae 100644 --- a/BasicDevTests.py +++ b/BasicDevTests.py @@ -18,7 +18,7 @@ import re -def TestEncodingOk(apath, encodingValue): # noqa +def TestEncodingOk(apath, encodingValue): try: with open(apath, "rb") as f_obj: f_obj.read().decode(encodingValue) @@ -29,7 +29,7 @@ def TestEncodingOk(apath, encodingValue): # noqa return True -def TestFilenameLowercase(apath): # noqa +def TestFilenameLowercase(apath): if apath != apath.lower(): logging.critical(f"Lowercase failure: file {apath} not lower case path") logging.error(f"\n\tLOWERCASE: {apath.lower()}\n\tINPUTPATH: {apath}") @@ -47,14 +47,14 @@ def PackageAndModuleValidCharacters(apath): return True -def TestNoSpaces(apath): # noqa +def TestNoSpaces(apath): if " " in apath: logging.critical(f"NoSpaces failure: file {apath} has spaces in path") return False return True -def TestRequiredLicense(apath): # noqa +def TestRequiredLicense(apath): licenses = ["SPDX-License-Identifier: BSD-2-Clause-Patent", ] try: with open(apath, "rb") as f_obj: diff --git a/azure-pipelines/templates/build-test-job.yml b/azure-pipelines/templates/build-test-job.yml index 6bb1f8770..004ee01f9 100644 --- a/azure-pipelines/templates/build-test-job.yml +++ b/azure-pipelines/templates/build-test-job.yml @@ -34,6 +34,8 @@ jobs: - template: flake8-test-steps.yml - template: pydocstyle-test-steps.yml + parameters: + root_package_folder: ${{parameters.root_package_folder}} - template: spell-test-steps.yml diff --git a/azure-pipelines/templates/pydocstyle-test-steps.yml b/azure-pipelines/templates/pydocstyle-test-steps.yml index cb30f1bc9..869ebfcdf 100644 --- a/azure-pipelines/templates/pydocstyle-test-steps.yml +++ b/azure-pipelines/templates/pydocstyle-test-steps.yml @@ -7,15 +7,15 @@ ## parameters: - none: '' + root_package_folder: '' steps: -- script: pydocstyle . +- script: pydocstyle ${{parameters.root_package_folder}} displayName: 'Run pydocstyle' condition: succeededOrFailed() # Only capture and archive the lint log on failures. -- script: pydocstyle . > pydocstyle.err.log +- script: pydocstyle ${{parameters.root_package_folder}} > pydocstyle.err.log displayName: 'Capture pydocstyle failures' condition: Failed() diff --git a/docs/developing.md b/docs/developing.md index 0b3161eec..fc0e1c811 100644 --- a/docs/developing.md +++ b/docs/developing.md @@ -97,7 +97,7 @@ out all the different parts. 2. Run a Basic Python docstring Check (using pydocstring) and resolve any issues ``` cmd - pydocstyle . + pydocstyle edk2toolext ``` 3. Run the `BasicDevTests.py` script to check file encoding, file naming, etc diff --git a/edk2toolext/__init__.py b/edk2toolext/__init__.py index 9f73dc662..504057895 100644 --- a/edk2toolext/__init__.py +++ b/edk2toolext/__init__.py @@ -3,5 +3,7 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## +"""This file exists to satisfy pythons packaging requirements. -# noqa \ No newline at end of file +Read more: https://docs.python.org/3/reference/import.html#regular-packages +""" diff --git a/edk2toolext/bin/__init__.py b/edk2toolext/bin/__init__.py index 9f73dc662..504057895 100644 --- a/edk2toolext/bin/__init__.py +++ b/edk2toolext/bin/__init__.py @@ -3,5 +3,7 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## +"""This file exists to satisfy pythons packaging requirements. -# noqa \ No newline at end of file +Read more: https://docs.python.org/3/reference/import.html#regular-packages +""" diff --git a/edk2toolext/environment/__init__.py b/edk2toolext/environment/__init__.py index 9f73dc662..504057895 100644 --- a/edk2toolext/environment/__init__.py +++ b/edk2toolext/environment/__init__.py @@ -3,5 +3,7 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## +"""This file exists to satisfy pythons packaging requirements. -# noqa \ No newline at end of file +Read more: https://docs.python.org/3/reference/import.html#regular-packages +""" diff --git a/edk2toolext/environment/extdeptypes/__init__.py b/edk2toolext/environment/extdeptypes/__init__.py index 9f73dc662..48e3ed736 100644 --- a/edk2toolext/environment/extdeptypes/__init__.py +++ b/edk2toolext/environment/extdeptypes/__init__.py @@ -3,5 +3,4 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## - -# noqa \ No newline at end of file +"""This package contains the different ext_dep types available in the environment.""" diff --git a/edk2toolext/environment/plugintypes/__init__.py b/edk2toolext/environment/plugintypes/__init__.py index 9f73dc662..2f894c4ef 100644 --- a/edk2toolext/environment/plugintypes/__init__.py +++ b/edk2toolext/environment/plugintypes/__init__.py @@ -3,5 +3,4 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## - -# noqa \ No newline at end of file +"""This package contains the different plugin types available in the environment.""" diff --git a/edk2toolext/invocables/__init__.py b/edk2toolext/invocables/__init__.py index a7e3a2a8b..a1e88c2ed 100644 --- a/edk2toolext/invocables/__init__.py +++ b/edk2toolext/invocables/__init__.py @@ -3,5 +3,4 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## - -# noqa +"""This package contains the different invocables available to the developer.""" diff --git a/edk2toolext/tests/__init__.py b/edk2toolext/tests/__init__.py index a7e3a2a8b..504057895 100644 --- a/edk2toolext/tests/__init__.py +++ b/edk2toolext/tests/__init__.py @@ -3,5 +3,7 @@ # # SPDX-License-Identifier: BSD-2-Clause-Patent ## +"""This file exists to satisfy pythons packaging requirements. -# noqa +Read more: https://docs.python.org/3/reference/import.html#regular-packages +""" From 9b422202e614df768a1d604f902566722d4df2c8 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Mon, 7 Nov 2022 08:33:07 -0500 Subject: [PATCH 3/4] Update usage of deprecated loading APIs Fixes #337 Plugins are currently loaded in the `_load()` function in plugin_manager.py by using the `imp` standard library: https://docs.python.org/3/library/imp.html The first sentence of the library documentation states the following: ``` Deprecated since version 3.4, will be removed in version 3.12: The imp module is deprecated in favor of importlib. ``` This means, `imp` will be removed in the next Python release. Therefore, this change updates the code to use `importlib` standard library as recommended by the `imp` documentation. The code follows the recipe recommended by the `importlib` documentation to directly load a Python source file as is the case when loading Python modules based on plugin descriptor information. https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly Signed-off-by: Michael Kubacki --- edk2toolext/environment/plugin_manager.py | 32 ++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/edk2toolext/environment/plugin_manager.py b/edk2toolext/environment/plugin_manager.py index b1699e6af..1d53fcab1 100644 --- a/edk2toolext/environment/plugin_manager.py +++ b/edk2toolext/environment/plugin_manager.py @@ -9,7 +9,7 @@ import sys import os -import imp +import importlib import logging from edk2toolext.environment import shell_environment @@ -82,32 +82,34 @@ def _load(self, PluginDescriptor): PluginDescriptor(PluginDescriptor): the plugin descriptor """ PluginDescriptor.Obj = None - PythonFileName = PluginDescriptor.descriptor["module"] + ".py" - PyModulePath = os.path.join(os.path.dirname(os.path.abspath( - PluginDescriptor.descriptor["descriptor_file"])), PythonFileName) - logging.debug("Loading Plugin from %s", PyModulePath) - try: - with open(PyModulePath, "r") as plugin_file: - _module = imp.load_module( - "UefiBuild_Plugin_" + PluginDescriptor.descriptor["module"], - plugin_file, - PyModulePath, - ("py", "r", imp.PY_SOURCE)) + py_file_path = PluginDescriptor.descriptor["module"] + ".py" + py_module_path = os.path.join(os.path.dirname(os.path.abspath( + PluginDescriptor.descriptor["descriptor_file"])), py_file_path) + py_module_name = "UefiBuild_Plugin_" + PluginDescriptor.descriptor["module"] + + logging.debug("Loading Plugin from %s", py_module_path) + + try: + spec = importlib.util.spec_from_file_location( + py_module_name, py_module_path) + module = importlib.util.module_from_spec(spec) + sys.modules[py_module_name] = module + spec.loader.exec_module(module) except Exception: exc_info = sys.exc_info() logging.error("Failed to import plugin: %s", - PyModulePath, exc_info=exc_info) + py_module_path, exc_info=exc_info) return -1 # Instantiate the plugin try: - obj = getattr(_module, PluginDescriptor.descriptor["module"]) + obj = getattr(module, PluginDescriptor.descriptor["module"]) PluginDescriptor.Obj = obj() except AttributeError: exc_info = sys.exc_info() logging.error("Failed to instantiate plugin: %s", - PyModulePath, exc_info=exc_info) + py_module_path, exc_info=exc_info) return -1 return 0 From 24f080b2953f610bd686fcb739679b3e2f005c0a Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Mon, 7 Nov 2022 08:51:26 -0500 Subject: [PATCH 4/4] Add plugin module paths to system path Closes #338 Adds dynamically loaded plugin module directory paths to the system path so the plugin modules can import other modules relative to their path as they would if they were not dynamically loaded. Signed-off-by: Michael Kubacki --- edk2toolext/environment/plugin_manager.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/edk2toolext/environment/plugin_manager.py b/edk2toolext/environment/plugin_manager.py index 1d53fcab1..da4f7f46e 100644 --- a/edk2toolext/environment/plugin_manager.py +++ b/edk2toolext/environment/plugin_manager.py @@ -95,6 +95,11 @@ def _load(self, PluginDescriptor): py_module_name, py_module_path) module = importlib.util.module_from_spec(spec) sys.modules[py_module_name] = module + + py_module_dir = os.path.dirname(py_module_path) + if py_module_dir not in sys.path: + sys.path.append(py_module_dir) + spec.loader.exec_module(module) except Exception: exc_info = sys.exc_info()