Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin Loader Updates [Rebase & FF] #339

Merged
merged 2 commits into from Nov 9, 2022

Conversation

makubacki
Copy link
Member

This series makes two main changes:

  1. Update usage of deprecated loading APIs

    Fixes Plugin Loader dependency will be removed in Python 3.12 #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

  2. Add plugin module paths to system path

    Closes [Feature]: Allow plugins to reference relative modules #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.

@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Merging #339 (eb491c6) into master (fdc72c3) will decrease coverage by 0.93%.
The diff coverage is 8.33%.

❗ Current head eb491c6 differs from pull request most recent head b7ee773. Consider uploading reports for the commit b7ee773 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
- Coverage   70.59%   69.66%   -0.94%     
==========================================
  Files          48       42       -6     
  Lines        4792     4790       -2     
==========================================
- Hits         3383     3337      -46     
- Misses       1409     1453      +44     
Flag Coverage Δ
Linux 69.66% <8.33%> (-0.10%) ⬇️
Windows_NT ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
edk2toolext/environment/plugin_manager.py 37.50% <8.33%> (-2.50%) ⬇️
edk2toolext/capsule/signtool_signer.py 12.50% <0.00%> (-44.32%) ⬇️
edk2toolext/edk2_invocable.py 61.07% <0.00%> (-0.68%) ⬇️
edk2toolext/environment/extdeptypes/__init__.py
edk2toolext/__init__.py
edk2toolext/bin/__init__.py
edk2toolext/invocables/__init__.py
edk2toolext/environment/plugintypes/__init__.py
edk2toolext/environment/__init__.py

Copy link
Contributor

@Javagedes Javagedes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with optional request to update how we currently insert variables into strings for logging.

Copy link
Member

@spbrogan spbrogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve with comment about polluting the python path and potential unintended consequences.

@spbrogan spbrogan added the enhancement New feature or request label Nov 8, 2022
Fixes tianocore#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 <michael.kubacki@microsoft.com>
Closes tianocore#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 <michael.kubacki@microsoft.com>
@Javagedes Javagedes added this to the 0.20.0 milestone Nov 9, 2022
@Javagedes Javagedes merged commit 24f080b into tianocore:master Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants