pcbnew Action Plugins not reloaded on "Refresh Plugin"

[Antoine Pintout]

Action Plugins are not refreshed after hitting "Refresh Plugin". Restarting pcbnew does not reload the plugins, only a complete KiCad restart reloads the plugins.

Removing __pycache__ or .pyc files from the plugin folder does not help when trying to refresh the plugins.

The plugin I'm trying to refresh is in ~/.kicad_plugins

When an exception is thrown in the script, the line number where the exception occurs is the cached version but the line content is the new content. This was very confusing at first ;)

Here's my version infos :

Application: kicad
Version: 6.0.0-unknown-589a37b~86~ubuntu18.04.1, release build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.58.0 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.4 libpsl/0.19.1 (+libidn2/2.0.4) nghttp2/1.30.0 librtmp/2.3
Platform: Linux 4.15.0-49-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 3.22
    Boost: 1.65.1
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.58.0
    Compiler: GCC 7.4.0 with C++ ABI 1011

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=ON
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=ON
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

EDITED: added more info concerning the location of the plugin I'm trying to refresh

[Antoine Pintout (procsynth)]

Steps to reproduce :

- Close all instances of KiCad
- In `~/.kicad_plugins`, create the file `test_bug1828595.py` with the following content :

```
# test_bug1828595.py

import pcbnew

class SimplePlugin(pcbnew.ActionPlugin):
    def defaults(self):
        self.name = "Reproduce bug #1828595"
        self.description = "Reproducing bug #1828595"
        self.category = "Testing"

    def Run(self):
        # The entry function of the plugin that is executed on user action
        print(self.name)

SimplePlugin().register() # Instantiate and register to Pcbnew

```
- Run KiCad with the command line
- Start pcbnew
- In "Tools > External Plugins..." click "Reproduce bug #1828595"
- The string "Reproduce bug #1828595" appears in the command line
- Modify `test_bug1828595.py` by changing the value of self.name :
```
        self.name = "AAAAAAAA"
```
- Save `test_bug1828595.py`
- In "Tools > External Plugins..." click "Refresh Plugins"
- In "Tools > External Plugins..." the name of the plugin is still "Reproduce bug #1828595", should be "AAAAAAAA"
- In "Tools > External Plugins..." click "Reproduce bug #1828595"
- The string "Reproduce bug #1828595" appears in the command line, should be "AAAAAAAA"

[Ian McInerney (imcinerney)]

Antoine,

I am trying to reproduce this behavior in my kicad built off of the most recent master, and I am seeing that the plugin is actually being imported under the new name ("AAAAAAAA"). This leads to me having 2 plugins in the list (the original "Reproduce bug #1828595" and the new "AAAAAAAA"). Do you have this second plugin in the list?

[Ian McInerney (imcinerney)]

Ok, disregard the last comment I made. It appears I had tested the wrong version on my machine (python 2 instead of python 3). This does lead to the conclusion that this issue is only with the python 3 build, since python 2 refreshes properly.

[Antoine Pintout (procsynth)]

Hi Ian,

I still can reproduce the bug with the last nightly version and my current setup.

Application: Pcbnew
Version: 6.0.0-unknown-3afea91~86~ubuntu18.04.1, release build
Platform: Linux 4.15.0-51-generic x86_64, 64 bit, Little endian, wxGTK

$ python --version give me "Python 2.7.15+", how does KiCad choose the version to use ?
$ python3 --version give me Python 3.6.8

[Ian McInerney (imcinerney)]

The attached patch fixes the reload call when using python 3. reload() was moved into a new module called importlib in 3.4, so the old call just to reload() was failing in python3. I tested this with both python 3.6 and 2.7 on master and 5.1 and it reloads properly.

[Ian McInerney (imcinerney)]

Antoine,

Your kicad was built with support for python3, so it should use python3 for the scripting. My initial comment was because I had accidentally used my python2 build instead of a python3 build. That actually leads to a different bug that I will file a new report on (if you change a plugin's name the old plugin is still in the list alongside the new plugin).

[Wayne Stambaugh (stambaughw)]

@Ian, your patch seems to reload the script as expected but it doesn't remove the previous script name from the menu so you end up with an extra menu entry that no longer has the correct script name. Did you not see this in your testing?

[Ian McInerney (imcinerney)]

@Wayne, I saw that in the testing originally when I was reproducing the issue (see comment #2) and also at the end (see comment #6), but that is not what I thought the main issue in this bug was about. When I reproduced this on python 3, no plugins were being reloaded (even if only the code inside run() changed not the name). This meant that the entire reload mechanism was broken.

I did spend a little time tracing down the extra menu entry, and its root cause is different from the root cause of the failed reloading. The extra entries are due to the plugin module not having a mechanism for detecting when a plugin is no longer available during its refresh. I was going to make a new bug report tonight to track the double menu entry since it isn't as blocking as this issue and has a different root cause that will probably require a larger change set to the plugin system. If you would like it to be included in this issue, I can do that instead.

[Andrew Lutsenko (qu1ck)]

Please note that you patch only fixes the issue for single file plugins.
Same issue exists (both on py2 and py3) for complex plugins that are imported as modules, see LoadOneSubdirPlugin().
It would be good to address that too.

[Ian McInerney (imcinerney)]

@Andrew, do you have a complex plugin that is failing to reload that you could attach for me to test with? (I don't have an example handy) Looking through the code inside LoadOneSubdirPlugin() it appears that is uses the __import__ statement which works on both python 2 and 3 when used in the LoadOnePlugin() function.

[Andrew Lutsenko (qu1ck)]

Yes, issue is __import__ does not reimport a module if it is already in namespace.
Potential solution would be to first del(ModuleName) and then __import__() but I have not tested it.

An example of a complex plugin to test is attached. Make sure you drop the whole folder in plugin dir.
You can edit the message of the popup and see that it does not get picked up by kicad without full restart.

[Andrew Lutsenko (qu1ck)]

I did a bit more research.

Something like this should work:

if SubDirname in sys.modules:
  reload(SubDirname)
else:
  __import__(SubDirname, locals(), globals())

where reload has same conditionals as in your patch, probably extracting it into helper function is good idea.

[Ian McInerney (imcinerney)]

Here is a patch that fixes the reload of both the complex and simple plugins. It also unifies the import function to simplify the code.

[Seth Hillbrand (sethh)]

Looks good. Thank you for the contribution to KiCad!

[Ian McInerney (imcinerney)]

It looks like the janitor missed this. Fixed in commit d8e7892081554c62edaaa973dd1fd83f793c2d2a.

https://git.launchpad.net/kicad/commit/?id=d8e7892081554c62edaaa973dd1fd83f793c2d2a