Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when upgrading the plugin #504

Open
erpas opened this issue Jul 17, 2023 · 6 comments
Open

Error when upgrading the plugin #504

erpas opened this issue Jul 17, 2023 · 6 comments
Labels
bug Something isn't working windows

Comments

@erpas
Copy link
Contributor

erpas commented Jul 17, 2023

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

@erpas erpas added bug Something isn't working windows labels Oct 13, 2023
@PeterPetrik
Copy link
Contributor

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
Copy link
Contributor

wonder-sk commented Oct 17, 2023

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)

@wonder-sk wonder-sk changed the title Error when upgrading the plugin to 2023.2 Error when upgrading the plugin Oct 17, 2023
@alexbruy
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 MerginMaps/geodiff#205 as well, so that a solution like #534 could do a proper API call to free the library rather something very hacky.

@erpas
Copy link
Contributor Author

erpas commented Jul 9, 2024

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

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working windows
Projects
None yet
Development

No branches or pull requests

6 participants