MerginMaps / qgis-plugin

QGIS plugin for managing Mergin Maps projects
GNU General Public License v3.0
34 stars 13 forks source link

Error when upgrading the plugin #504

Open erpas opened 1 year ago

erpas commented 1 year ago

On Windows, users get the following when upgrading to version 2023.2.

Plugin installation failed: Failed to unzip plugin package. Probably it's broken or missing from the repository. You may also want to make sure that you have write permission to the plugin directory: C:/Users/<user>/AppData/Roaming/QGIS/QGIS3/profiles/default/python/plugins

obraz

When trying to remove the plugin using the Plugins Manager, users get:

Plugin uninstall failed: Failed to remove the directory: C:/Users/<user>/AppData/Roaming/QGIS/QGIS3/profiles/default/python/plugins/Mergin

The problematic file in use is:

C:\Users\\AppData\Roaming\QGIS\QGIS3\profiles\default\python\plugins\Mergin\mergin\deps\pygeodiff\pygeodiff-2.0.1-python.pyd

A workaround to this is to try to uninstall the plugin (and get the permissions error) restart QGIS and install the plugin again.

PeterPetrik commented 10 months ago

I think this would need QGIS fix? It looks like unload is not called and the folder is just removed ? https://github.com/qgis/QGIS/blob/94b62accf2a1cfc892bc9097cd4dd5f8a1f60ab0/python/pyplugin_installer/qgsplugininstallerinstallingdialog.py#L155

wonder-sk commented 10 months ago

Summary of the findings so far:

By the way, it is also possible to trigger the problem by clicking reinstall plugin (no need to downgrade + upgrade for testing)

alexbruy commented 10 months ago

I also can reproduce issue with upgrading with old Mergin versions, e.g. 2022.3.x. The geodiff *.pyd file is used and can't be removed.

tomasMizera commented 9 months ago

Summary of the findings so far:

  • things with upgrade failures got worse most likely when we added functionality to show changes last year. Since then, we load geodiff library on plugin start, so the upgrade never works (previously upgrade would only fail after opening project sync dialog) - this should get fixed in don't instantiate geodiff library at the module level, create it inside function (refs #504) #532
  • on Windows we need to unload pygeodiff DLL (.pyd file) to fix this to release system's write lock on the .pyd file - we should do that on plugin unload (using _ctypes.FreeLibrary())
  • unfortunately plugin installer first removes plugin directory before unloading it and reloading - this is a bug in QGIS worth fixing

By the way, it is also possible to trigger the problem by clicking reinstall plugin (no need to downgrade + upgrade for testing)

Hi @wonder-sk, given that thee first and the third points are now addressed, does it mean that the only remaining problem is the second point?

wonder-sk commented 9 months ago

Hi @wonder-sk, given that thee first and the third points are now addressed, does it mean that the only remaining problem is the second point?

Correct. This is indended in #534, but we got kinda stuck there and gave up for the time being.

And it would be good to implement https://github.com/MerginMaps/geodiff/issues/205 as well, so that a solution like #534 could do a proper API call to free the library rather something very hacky.

erpas commented 1 month ago

The issue is still here and affecting Windows users when they upgrade to 2024.2.

image