Sublime Text build 3189 ignores plugin_loaded() functions from .pyc modules

[guillermooo]

Description

Sublime Text never passes .pyc modules to sublime_plugin.reload_module(module), not in build 3189 and not in previous builds. If .pyc modules were loaded just as .py modules, the issue reported here wouldn't happen. The different ways in which .py and .pyc modules are loaded may be an issue in itself or intended behavior.

Before build 3189, sublime_plugin.py used to iterate through sys.modules.values() and call all plugin_loaded() functions. This way, .pyc modules would get a chance to initialize.

As of build 3189, sublime_plugin.py ignores plugin_loaded() functions from .pyc modules because they are never loaded through sublime_plugin.py and sys.modules.values() is never queried for available plugins.

This is the breaking change in sublime_plugin.py:

 def synthesize_on_activated_async():
     if not api_ready:
@@ -277,12 +293,12 @@
     global api_ready
     api_ready = True

-    for m in list(sys.modules.values()):
-        if "plugin_loaded" in m.__dict__:
-            try:
-                m.plugin_loaded()
-            except:
-                traceback.print_exc()
+    for plc in deferred_plugin_loadeds:
+        try:
+            plc()
+        except:
+            traceback.print_exc()
+    deferred_plugin_loadeds.clear()

A workaround follows that may or may not work for all .pyc modules in a Sublime Text plugin folder (it works for my case and I stopped investigating):

# some_plugin.pyc

def plugin_loaded():
    print('initializing my plugin...')

if not sublime_plugin.api_ready:
    print("[DEBUG] request deferred loading of", __name__)
    sublime_plugin.deferred_plugin_loadeds.append(plugin_loaded)

It would be great to know if Sublime HQ intends to fix this issue or if they consider it a feature. Currently, it's breaking the Six Vim emulator.

Steps to reproduce

1. Install Sublime Text build 3188
2. Install Six package
3. Verify that Six loads correctly
4. Install Sublime Text build 3189
5. Verify that Six doesn't load correctly

Expected behavior

From the Sublime Text release notes for build 3189:

API: Various minor improvements related to plugin module loading and unloading

This indicates that a breaking change wasn't intended.

Actual behavior

Plugins including compiled .pyc modules don't initialize properly as of Sublime Text build 3189.

Environment

• Sublime Text build 3189
• This seems to be a cross-platform bug/feature; confirmed on Windows 10.

[Thom1729]

The root cause seems to be (as you say) that .pyc modules are treated differently from .py modules. Before #2585, plugin_loaded would be called on all modules, not just plugins (i.e. modules at the top level of a package). This behavior was incorrect, but it did partially cover up the original bug. (As you mention, reloading would still be broken.)

My guess is that some piece of code in Sublime is looking for files with a .py extension when it should be looking for .pyc as well. With luck, this might be an easy fix. If not, you may be able to work around the bug by calling sublime_plugin.load_module(module). This is a new function also added in 3189 (see #2590), so you can call it if it exists and do nothing if it doesn't.

Incidentally, looking over the ZipLoader code I see .py hardcoded in several places. Does Sublime load .pyc modules in zipped packages? If not, is that why Six declares .no-sublime-package? I'm sure that this could be corrected. I'd send a PR if I could. 😛

[deathaxe]

From what I understand, the changes in the _sublimeplugin.py just ensure to call the plugin_loaded() handler not earlier than the API is ready. Hence those changes are not causing binaries (*.pyc) to be ignored. Using the _sublimeplugin.py of ST3184 does not fix the issue.

I've created a little dummy my_plugin.pyc with a plugin_loaded() handler for testing. It looks like ST does not even load the plugin. As a result it does not call any handler in it.

The expected line reloading plugin PycDummy.dummy.pyc is completely missing in ST's console.

It doesn't matter whether the plugin is zipped or not.

[wbond]

We don’t currently load .pyc from .sublime-package files, and it is a bug I’ve been tracking internally, I just haven’t taken the time to fix it yet.

This issue is definitely a bug and I will get it addressed with the next build. It is kind of like the linegap bug identified in 3189 with DirectWrite that exposed the issue we aren’t sending theme font options to the filter label controls used in the quick panel. 😀 At least they are helping expose underlying issues!

[Thom1729]

From what I understand, the changes in the sublime_plugin.py just ensure to call the plugin_loaded() handler not earlier than the API is ready.

When a plugin is loaded via the new load_module or the existing reload_plugin, Sublime will run plugin_loaded if the API is ready. If the API isn't ready, the behavior changed in 3189.

Before 3189, on_api_ready would iterate over sys.modules and call plugin_loaded on any module that has such a method. This was incorrect; plugin_loaded should only be called on plugins.

In 3189, if the API is not ready, then reload_plugin/load_module will add its plugin_loaded to a list, and then on_api_ready will run all of those callbacks. But because reload_plugin isn't called on .pyc plugins (the root bug), this new approach will not call their plugin_loaded callbacks.

We don’t currently load .pyc from .sublime-package files, and it is a bug I’ve been tracking internally, I just haven’t taken the time to fix it yet.

Are you accepting PRs code pasted into issues?

[deathaxe]

@Thom1729 : This is basically true, but nevertheless my debugging package PycDummy.dummy.pyc does not exist in sys.modules as the module is not loaded like @wbond described and thus the contained plugin_loaded() is not executed.

With regards to Six this means, the plugin_loaded() method would most likely be defined in one of the modules loaded by the one and only text based python module main.py. ST3188- wouldn't have loaded any binary file as well.

As all of those modules are imported from the lib package, I'd say it is expected behavior to break the plugin as it does not define the plugin_loaded() in the correct place.

Six would need to add a plugin_loaded() to the main.py and call all the plugin_loaded() functions manually.

To verify this behavior I added an main.py to my PycDummy package, which imports the binary module.

from . import my_plugin_loaded   # binary .pyc module

Now ST3188 runs the plugin_loaded() from the binary module and ST3189 does not.

The issue is fixed by:

from . import my_plugin_loaded   # binary .pyc module

def plugin_loaded():
    my_plugin_loaded.plugin_loaded()

ST 3189 behaves as expected: The issue needs to be fixed in the Six plugin.

[guillermooo]

@Thom1729

Incidentally, looking over the ZipLoader code I see .py hardcoded in several places. Does Sublime load .pyc modules in zipped packages? If not, is that why Six declares .no-sublime-package? I'm sure that this could be corrected. I'd send a PR if I could. 😛

I missed you were asking about Six here.

The exact reason why I use .no-sublime-package has been lost in the fog of time, but it's probably because of what you say, although I'm sure at the time it was an empirical realization that dawned on me in a moment of despair, and not some sober insight I gleaned from looking at sublime_plugin.py. I do remember Six started as a .sublime-package and it will go back to one soon* if it's feasible.

I'd send a PR if I could.

Six will *most likely** end life as an open source project, but not just yet (or in the near future)...

* Definition only available through my lawyer

[FichteFoll]

So, is the root cause of this issue that .pyc files in the package root aren't loaded as plugins by ST and thus plugin_loaded isn't called on them? Or was it just that Six didn't bubble up its plugin_loaded functions from submodules that aren't plugins?

.pyc not being loaded in .sublime-package is a separate issue.

[guillermooo]

So, is the root cause of this issue that .pyc files in the package root aren't loaded as plugins by ST and thus plugin_loaded isn't called on them?

I think that's the issue, yes.

[wbond]

Right, so we don't load .pyc as plugins. They can be used to store valid Python, but neither in loose or packed configurations would .pyc ever be scanned for commands or event handlers, with the exception that plugin_loaded() would be called on every module in sys.modules.

Upon further investigation, it would seem to me that trying to scan .pyc for plugins may break packages or cause unintended consequences.

To preserve previous behavior, we would need to handle .pyc files that define plugin_loaded(). That said, this feels messy since .pyc files can't define other plugin code and have it properly handled. Additionally, .pyc files currently can't be loaded from .sublime-package files.

Considering all of the shortcomings of .pyc, but the potential for breakage, it would be nicer to not execute plugin_loaded() on .pyc files and leave the possible enhancement of supporting them properly for a more major version or a new Python environment. Would it be much of an imposition to change Six to define (or call the existing function) in a .py file @guillermooo?

[guillermooo]

@wbond

To preserve previous behavior, we would need to handle .pyc files that define plugin_loaded(). That said, this feels messy since .pyc files can't define other plugin code and have it properly handled. Additionally, .pyc files currently can't be loaded from .sublime-package files.

Agreed. It seems an oddity that .pyc files participate only partially as plugins.

Considering all of the shortcomings of .pyc, but the potential for breakage, it would be nicer to not execute plugin_loaded() on .pyc files and leave the possible enhancement of supporting them properly for a more major version or a new Python environment. Would it be much of an imposition to change Six to define (or call the existing function) in a .py file @guillermooo?

I think it's fine to completely ignore .pyc during plugin loading. For one, manually calling sublime_plugin.reload_plugin() from a .pyc module works fine. However, wouldn't the change lead to different loading order of modules during plugin loading and therefore, potentially, to more breakage? (I'm talking from memory, but I've been meaning to test this.)

[deathaxe]

However, wouldn't the change lead to different loading order of modules during plugin loading and therefore, potentially, to more breakage?

1. A package should provide only one plugin_loaded() in one main top-level module and make sure to call subsequent internal plugin-loaded-handlers if the order is important.

2. If more than one plugin_loaded() functions exist in different modules, they must be able to run independendly.

This statement is true for older builds of ST as well. The main change of ST3189 is to ensure to only call the plugin_loaded() function from "plugins". A plugin is a python module which must be placed in the root of a package (e.g.: Six/main.py).

Running all plugin_loaded() functions from all modules found in any sub directory of a package was actually a bug which might cause issues with 3rd-party libraries included in a package whose probably exposed plugin_loaded() is not meant to be called by ST.

[guillermooo]

Running all plugin_loaded() functions from all modules found in any sub directory of a package was actually a bug which might cause issues with 3rd-party libraries included in a package whose probably exposed plugin_loaded() is not meant to be called by ST.

I don't think anyone is disputing that? But I think it's fair to point out that fixing a long-standing bug may break packages that over the years have relied on the bug out of ignorance or sloppiness (both of which may not be desirable qualities but they aren't in short supply).

[deathaxe]

But I think it's fair to point out that fixing a long-standing bug may break packages ...

That's true, but should not lead to a situation which prevents bug fixing at all.