AutomaticPackageReloader icon indicating copy to clipboard operation
AutomaticPackageReloader copied to clipboard

Published 20 hours ago • verified publisher icon randy3k

Reload first dependency's dependencies when reloading a dependency

Open evandrocoan opened this issue 6 years ago • 14 comments

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 this dependencies.json:

{
    "*": {
        "*": [
            "dep1",
            "dep2"
        ]
    }
}

It means dep1 is required by dep2 and dep1 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.

evandrocoan avatar Jan 10 '19 13:01 evandrocoan

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.

randy3k avatar Jan 10 '19 16:01 randy3k

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 ======================================================

But, it still got stuck on the last begin, even after running it more times. There are more packages missing on the above list which requires 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.

At least you are unloading dep2 when reloading dep1, as the log shows at the beginning:

[Package Reloader] unloading dependency dep2
[Package Reloader] unloading dependency dep1
[Package Reloader] begin ======================================================

The dependencies used are:

  1. https://github.com/evandrocoan/debugtools (as dep1) requires nobody
  2. https://github.com/evandrocoan/channelmanager (as dep2) requires debugtools 
    {
    "*": {
        "*": [
            "debugtools"
        ]
    }
}

This is a package which requires both dependencies:

  1. https://github.com/evandrocoan/AmxxChannel 
    {
    "*": {
        "*": [
            "debugtools",
            "channelmanager"
        ]
    }
}
    Currently this package directly requires dep1 and dep2 because Package Control does not yet support a dependency requiring dependencies.

evandrocoan avatar Jan 10 '19 16:01 evandrocoan

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?

Thom1729 avatar Jan 10 '19 18:01 Thom1729

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:

  1. AllAutocomplete
  2. AmxxChannel
  3. AmxxEditor
  4. BufferScroll
  5. channelmanager
  6. HighlightWords
  7. LSP
  8. OverrideUnpackedPackages
  9. PlantUmlDiagrams
  10. QuickSettings
  11. RememberCommandPaletteInput
  12. StudioChannel
  13. WrapPlus

evandrocoan avatar Jan 10 '19 20:01 evandrocoan

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.

Thom1729 avatar Jan 10 '19 21:01 Thom1729 

>>> 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 ======================================================

This last begin was the last thing outputted. This time 8 packages were reloaded.

evandrocoan avatar Jan 10 '19 21:01 evandrocoan

I'm looking over the code trying to find something, and I did notice the following:

  • When reloading a dependency, we unload all dependencies before reloading packages. This could cause an error if a package's plugin_unloaded relies on a dependency.
  • When unloading a package, we call 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-levelplugin_unloaded` and it's not safe to call twice).

In short, the procedure is:

  • For each dependency:
    • For each module:
      • Remove the module from sys.modules.
  • For each package:
    • For each module:
      • Call sublime_plugin.unload_module.
      • Remove the module from sys.modules.
    • Reload the package.

But it should be:

  • For each package:
    • For each top-level module:
      • Call sublime_plugin.unload_module.
  • For each package or dependency:
    • For each module:
      • Remove the module from sys.modules.
  • For each package:
    • Reload the package.

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.

Thom1729 avatar Jan 10 '19 22:01 Thom1729

@evandrocoan try the refactor-reload branch and see if that helps.

Thom1729 avatar Jan 11 '19 14:01 Thom1729

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. ---------------------------------------------

Now, the last it output is [Package Reloader] reload failed. ---------------------------------------------. With the version the master branch, it would be [Package Reloader] begin ======================================================.

evandrocoan avatar Jan 11 '19 14:01 evandrocoan

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.

evandrocoan avatar Jan 11 '19 15:01 evandrocoan

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 avatar Jan 11 '19 15:01 Thom1729

@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.

randy3k avatar Jan 11 '19 19:01 randy3k

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:

  • Recursive dependency/dependent resolution (like resolve dependencies).
  • A "temporary package" class implemented using tempfile.
  • Possibly, some features "borrowed" from OverrideAudit.

Thom1729 avatar Jan 11 '19 22:01 Thom1729

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.

FichteFoll avatar Jan 12 '19 15:01 FichteFoll