Open evandrocoan opened 5 years ago
Are you editing dep1
or dep2
? If you are editing dep2
and the code of dep1
is untouched, there is no need to reload dep1
.
On the other hand, if you are editing dep1
, I believe that we don’t currently reload dep2
as only dependent regular packages are reloaded.
I had actually edited both dep1
and dep2
, and I was reloading dep2
. Indeed, there is not reason to reload dep1
when I am reloading dep2
. My bad.
Now, I tried reloading dep1
and it got stuck on this screen:
[Package Reloader] unloading dependency dep2
[Package Reloader] unloading dependency dep1
[Package Reloader] begin ======================================================
Then, I tried to run the command again, it it got stuck a little further:
[Package Reloader] unloading dependency dep2
[Package Reloader] unloading dependency dep1
[Package Reloader] error: WrapPlus is not loaded.
[Package Reloader] begin ======================================================
reloading plugin RememberCommandPaletteInput.input_history
[Package Reloader] reloading |-- RememberCommandPaletteInput
[Package Reloader] reloading |-- RememberCommandPaletteInput.input_history
[Package Reloader] end --------------------------------------------------------
[Package Reloader] begin ======================================================
I tried run it more times, but the result was always the last result above.
Then, I restarted Sublime Text, and the results had gone a little more beyond:
``` [Package Reloader] unloading dependency dep1 [Package Reloader] unloading dependency dep2 [Package Reloader] begin ====================================================== reloading plugin StudioChannel.commands [Package Reloader] reloading |-- StudioChannel [Package Reloader] reloading |-- StudioChannel.commands [Package Reloader] reloading | |-- StudioChannel.settings reloading plugin StudioChannel.settings [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin HighlightWords.HighlightWords [Package Reloader] reloading |-- HighlightWords [Package Reloader] reloading |-- HighlightWords.HighlightWords [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin OverrideUnpackedPackages.override_unpacked_packages [Package Reloader] reloading |-- OverrideUnpackedPackages [Package Reloader] reloading |-- OverrideUnpackedPackages.override_unpacked_packages [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin BufferScroll.BufferScroll [Package Reloader] reloading |-- BufferScroll [Package Reloader] reloading |-- BufferScroll.BufferScroll [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== ```
https://github.com/wbond/package_control/pull/1392
Technically, this package does not reload dependencies, it only unloads them. Packages need to be explicitly loaded so that commands and such will be properly registered. Dependencies can be (and typically are) loaded lazily, when an import
statement requires code from one. (Package Control's default dependency "loader" only adds the dependency to the path.) So I don't think we have to worry about dependency load order.
PR #24 (in master, not yet released) checks the dependencies.json
file in a dependency and recursively unloads all necessary dependencies, then reloads the necessary packages. I would expect that calling the reload command on debugtools should unload both debugtools and channelmanager, then reload AmxxChannel; and that reloading AmxxChannel should probably cause both dependencies to be loaded.
Are you using master?
Yes. I am using AutomaticPackageReloader master branch and on all repositories listed, I am also using their master branch. Although, for some of them, the master branch could have unpushed things, i.e., things I did today.
Technically, this package does not reload dependencies, it only unloads them.
Indeed. Everything seems to be working fine. The only unexplained thing is why not all packages which use the dependency are not being reloaded, i.e., when reloading the dependency debugtools
only up to 4 packages were being reloaded, while I got 13 packages using the debugtools
dependency:
What's the output of the following console command?
from AutomaticPackageReloader.reloader.reloader import resolve_dependencies; resolve_dependencies('debugtools')
It should be a tuple of two sets: respectively, dependencies and packages to reload.
>>> from AutomaticPackageReloader.reloader.reloader import resolve_dependencies; resolve_dependencies('debugtools')
({'channelmanager', 'debugtools'}, {'AmxxChannel', 'StudioChannel', 'UnitTesting', 'HighlightWords', 'RememberCommandPaletteInput', 'PlantUmlDiagrams', 'LSP', 'AllAutocomplete', 'AmxxEditor', 'WrapPlus', 'BufferScroll', 'OverrideUnpackedPackages'})
Everything is there.
The process of reloading is hanging. For example, I just ran the command now, the it stopped right here:
``` [Package Reloader] unloading dependency channelmanager [Package Reloader] unloading dependency debugtools [Package Reloader] begin ====================================================== reloading plugin AmxxChannel.commands [Package Reloader] reloading |-- AmxxChannel [Package Reloader] reloading |-- AmxxChannel.commands [Package Reloader] reloading | |-- AmxxChannel.settings reloading plugin AmxxChannel.settings [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin StudioChannel.commands [Package Reloader] reloading |-- StudioChannel [Package Reloader] reloading |-- StudioChannel.commands [Package Reloader] reloading | |-- StudioChannel.settings reloading plugin StudioChannel.settings [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin UnitTesting.ut [Package Reloader] reloading |-- UnitTesting [Package Reloader] reloading |-- UnitTesting.ut [Package Reloader] reloading | |-- UnitTesting.unittesting [Package Reloader] reloading | | |-- UnitTesting.unittesting.core [Package Reloader] reloading | | | |-- UnitTesting.unittesting.core.st3 [Package Reloader] reloading | | | | |-- UnitTesting.unittesting.core.st3.case [Package Reloader] reloading | | | | | |-- UnitTesting.unittesting.utils [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.json_file [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.output_panel [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.progress_bar [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.reloader [Package Reloader] reloading | | | | | | | |-- UnitTesting.unittesting.utils.stack_meter [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.stdio_splitter [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.isiterable [Package Reloader] reloading | | | | |-- UnitTesting.unittesting.core.st3.runner [Package Reloader] reloading | | | | |-- UnitTesting.unittesting.core.st3.suite [Package Reloader] reloading | | | |-- UnitTesting.unittesting.core.st3.legacy_runner [Package Reloader] reloading | | | |-- UnitTesting.unittesting.core.loader [Package Reloader] reloading | | |-- UnitTesting.unittesting.scheduler [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_package [Package Reloader] reloading | | | |-- UnitTesting.unittesting.mixin [Package Reloader] reloading | | | |-- UnitTesting.unittesting.const [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_coverage [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_current [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_syntax [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_color_scheme [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin HighlightWords.HighlightWords [Package Reloader] reloading |-- HighlightWords [Package Reloader] reloading |-- HighlightWords.HighlightWords [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin RememberCommandPaletteInput.input_history [Package Reloader] reloading |-- RememberCommandPaletteInput [Package Reloader] reloading |-- RememberCommandPaletteInput.input_history [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== reloading plugin PlantUmlDiagrams.diagram_plugin [Package Reloader] reloading |-- PlantUmlDiagrams [Package Reloader] reloading |-- PlantUmlDiagrams.diagram_plugin [Package Reloader] reloading | |-- PlantUmlDiagrams.diagram [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.plantuml [Package Reloader] reloading | | | |-- PlantUmlDiagrams.diagram.base [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.sublime3 [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.quicklook [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.preview [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.eog [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.freedesktop_default [Package Reloader] reloading | | |-- PlantUmlDiagrams.diagram.windows [Package Reloader] end -------------------------------------------------------- [Package Reloader] begin ====================================================== ```
I'm looking over the code trying to find something, and I did notice the following:
plugin_unloaded
relies on a dependency.sublime_plugin.unload_module
on all modules in the package. We should only call this on top-level modules. (We should still remove all modules from sys.modules
.) This could cause an error if a non-top-level module defines a method named 'plugin_unloadedthat is not safe to call (e.g. if it's used in a top-level
plugin_unloaded` and it's not safe to call twice).In short, the procedure is:
sys.modules
.sublime_plugin.unload_module
.sys.modules
.But it should be:
sublime_plugin.unload_module
.sys.modules
.I don't know whether this discrepancy is relevant to the observed problem, but nothing else jumps out at me. It does look like a possible source of problems when reloading multiple dependencies and packages.
I'd like to remedy this in either case. It will probably be a big code change, but mostly moving things around that are already there.
@evandrocoan try the refactor-reload
branch and see if that helps.
I checkout on your branch, restarted Sublime Text, and tried to reload debugtools
:
``` [Package Reloader] begin ====================================================== reloading plugin BufferScroll.BufferScroll [Package Reloader] reloading |-- BufferScroll [Package Reloader] reloading |-- BufferScroll.BufferScroll reloading plugin AmxxChannel.commands [Package Reloader] reloading |-- AmxxChannel [Package Reloader] reloading |-- AmxxChannel.commands [Package Reloader] reloading | |-- AmxxChannel.settings reloading plugin AmxxChannel.settings reloading plugin UnitTesting.ut [Package Reloader] reloading |-- UnitTesting [Package Reloader] reloading |-- UnitTesting.ut [Package Reloader] reloading | |-- UnitTesting.unittesting [Package Reloader] reloading | | |-- UnitTesting.unittesting.core [Package Reloader] reloading | | | |-- UnitTesting.unittesting.core.st3 [Package Reloader] reloading | | | | |-- UnitTesting.unittesting.core.st3.case [Package Reloader] reloading | | | | | |-- UnitTesting.unittesting.utils [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.json_file [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.output_panel [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.progress_bar [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.reloader [Package Reloader] reloading | | | | | | | |-- UnitTesting.unittesting.utils.stack_meter [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.stdio_splitter [Package Reloader] reloading | | | | | | |-- UnitTesting.unittesting.utils.isiterable [Package Reloader] reloading | | | | |-- UnitTesting.unittesting.core.st3.runner [Package Reloader] reloading | | | | |-- UnitTesting.unittesting.core.st3.suite [Package Reloader] reloading | | | |-- UnitTesting.unittesting.core.st3.legacy_runner [Package Reloader] reloading | | | |-- UnitTesting.unittesting.core.loader [Package Reloader] reloading | | |-- UnitTesting.unittesting.scheduler [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_package [Package Reloader] reloading | | | |-- UnitTesting.unittesting.mixin [Package Reloader] reloading | | | |-- UnitTesting.unittesting.const [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_coverage [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_current [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_syntax [Package Reloader] reloading | | |-- UnitTesting.unittesting.test_color_scheme reloading plugin WrapPlus.py_textwrap [Package Reloader] reloading |-- WrapPlus [Package Reloader] reloading |-- WrapPlus.py_textwrap reloading plugin WrapPlus.wrap_plus [Package Reloader] reloading |-- WrapPlus.wrap_plus reloading plugin LSP.boot [Package Reloader] reloading |-- LSP [Package Reloader] reloading |-- LSP.boot [Package Reloader] reloading | |-- LSP.plugin [Package Reloader] reloading | |-- LSP.plugin.core [Package Reloader] reloading | |-- LSP.plugin.core.main [Package Reloader] reloading | | |-- LSP.plugin.core.settings [Package Reloader] reloading | | | |-- LSP.plugin.core.types [Package Reloader] reloading | | |-- LSP.plugin.core.events [Package Reloader] reloading | | |-- LSP.plugin.core.registry [Package Reloader] reloading | | | |-- LSP.plugin.core.diagnostics [Package Reloader] reloading | | | | |-- LSP.plugin.core.url [Package Reloader] reloading | | | | |-- LSP.plugin.core.protocol [Package Reloader] reloading | | | | |-- LSP.plugin.core.views [Package Reloader] reloading | | | | |-- LSP.plugin.core.windows [Package Reloader] reloading | | | | | |-- LSP.plugin.core.sessions [Package Reloader] reloading | | | | | | |-- LSP.plugin.core.transports [Package Reloader] reloading | | | | | | |-- LSP.plugin.core.rpc [Package Reloader] reloading | | | | | | | |-- LSP.plugin.core.process [Package Reloader] reloading | | | | | |-- LSP.plugin.core.workspace [Package Reloader] reloading | | | |-- LSP.plugin.core.configurations [Package Reloader] reloading | | | |-- LSP.plugin.core.clients [Package Reloader] reloading | | | |-- LSP.plugin.core.handlers [Package Reloader] reloading | | |-- LSP.plugin.core.panels [Package Reloader] reloading | |-- LSP.plugin.core.documents [Package Reloader] reloading | |-- LSP.plugin.core.edit [Package Reloader] reloading | |-- LSP.plugin.completion [Package Reloader] reloading | |-- LSP.plugin.diagnostics [Package Reloader] reloading | |-- LSP.plugin.configuration [Package Reloader] reloading | |-- LSP.plugin.formatting [Package Reloader] reloading | |-- LSP.plugin.highlights [Package Reloader] reloading | |-- LSP.plugin.definition [Package Reloader] reloading | |-- LSP.plugin.hover [Package Reloader] reloading | | |-- LSP.plugin.core.popups [Package Reloader] reloading | |-- LSP.plugin.references [Package Reloader] reloading | |-- LSP.plugin.signature_help [Package Reloader] reloading | |-- LSP.plugin.code_actions [Package Reloader] reloading | | |-- LSP.plugin.core.helpers [Package Reloader] reloading | |-- LSP.plugin.symbols [Package Reloader] reloading | |-- LSP.plugin.rename [Package Reloader] reloading | |-- LSP.plugin.execute_command [Package Reloader] reload failed. --------------------------------------------- ```
Wait. I think I know the problem, the debugtools
dependency cannot be completley unloaded/deleted from memory as explained on https://github.com/SublimeText/UnitTesting/issues/145#issuecomment-452364578
Then, I just cannot use AutomaticPackageReloader
to reload it. Unless there is a setting/flag which I can set on the debugtools
dependency, marking it to not be deleted from memory by AutomaticPackageReloader. But as you explained, it seems that AutomaticPackageReloader does not reload the dependency files, it just delete the dependency from the memory and reload the packages which use the dependency.
I did some tricks to allow the debugtools
to be reloaded, it just cannot be completely unloaded/deleted from memory. Then, when developing, to test my changes, I just do on the console of Sublime Text a command like import imp; import debugtools.all.debug_tools.stderr_replacement; imp.reload( debugtools.all.debug_tools.stderr_replacement )
to reload it on the fly, but without completing deleting it from memory.
Ah, that makes sense.
This should be solvable (after #28). Whenever a package causes sys.stdout
to be patched, it should ensure that the patch will be undone in plugin_unloaded
. (This is probably the correct behavior anyway.) Then, when debugtools is unloaded, sys.stdout
won't be patched, so there should be no problem.
If multiple packages may share the same patch, then the patch can keep a count of users, and the unsubscribe function can decrement the count and unpatch if the count is zero.
@Thom1729 I am thinking of moving the core reloader to sublime_lib, and UnitTesting would depend on sublime_lib for the reloader code. This package will depend on sublime_lib and remain as the front interface for the reloader code. It will make the maintenance a bit easier.
I think the reloader logic definitely makes sense for a dependency. I am of two minds about whether sublime_lib is the right place versus a new dedicated dependency (hypothetically, "package_util").
The upside of combining them is that we'd probably want to take advantage of some sublime_lib features anyway, particularly ResourcePath, so if they were separate then "package_util" would have to depend on sublime_lib (and as it stands, users would have to explicitly declare both dependencies).
The downside is that sublime_lib would then depend on Package Control -- even though the reloading works without PC, its behavior is partly determined by the internals of PC. This is a discrete step beyond merely abstracting the Sublime API. We could remove this dependency by reimplementing is_dependency and such, though that comes with its own complications.
A separate dependency might be more "opinionated" with things like the PC dependency, logging, and so on. It would also be a good opportunity to expose more features:
resolve dependencies
).I'm in favor of a separate dependency rather than incorporating into sublime_lib.
When dependency dependencies get added to pc, that new dep can depend on sublime_lib officially. Until then, packages that use it can just depend on both.
My dependency got dependencies, and when reloading it, it just start reloading the packages.
You can know the dependency load order by analyzing the Package Control
dependencies.json
because they should be ordered by the dependency load order. For example, consider a dependency with thisdependencies.json
:It means
dep1
is required bydep2
anddep1
should be loaded first.You can also analyse each dependency
.sublime-dependency
file and get their load order, then, starting loading them from the lowest to the highest load order.