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

Download, update and install plugins from UI #420

Merged
merged 38 commits into from Sep 27, 2015

Conversation

Projects
None yet
3 participants
@zas
Copy link
Collaborator

commented Sep 1, 2015

This is a rework of #334

I'll try to include all changes after comments, and most notably, it implements the zipimport suggestion made by @mineo, which permits to import zipped plugins without unzipping them, simplifying installation (just save the .zip in user plugins directory).

Also it gets rid of the need of a temporary directory for downloaded files.

http://tickets.musicbrainz.org/browse/PICARD-691

zas added some commits Aug 31, 2015

Reformat plugin details
Drop file list for now.
Implement update/download plugins from the web directly from UI
- plugin installation code is modified to support zip archives
- USER_DOWNLOADS_DIR is introduced to store downloaded files
- based on the work of Shadab Zafar
Installation of zipped modules is now just a copy of the zip file
No need to unzip anything since we now support loading of zipped modules
@zas

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 1, 2015

Update is buggy, i got weird error:
error: Error -5 while decompressing data: incomplete or truncated stream
But if i quit Picard, the plugin is installed and loaded...
Still investigating, no clue atm.

zas added some commits Sep 2, 2015

Update plugins at next run when replacing files is needed
Replacing during runtime is leading to many issues, either with .zip plugins or
on Windows platform.

So, when it is necessary, require the user to restart Picard to update the module.
@zas

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 4, 2015

Update is buggy, i got weird error:
error: Error -5 while decompressing data: incomplete or truncated stream
But if i quit Picard, the plugin is installed and loaded...
Still investigating, no clue atm.

I finally concluded that it will be hard to make runtime updates, so until we find a reliable way to do it, i went the "restart Picard" to update way.
Basically, when new plugin is installed, and conflicting files exists, it saves new version with extension .update, and when Picard is restarted it takes care of those, deleting any previous files/directories matching the same plugin, and finally install it.
To give you an idea, we can have a plugin in contrib/PLUGIN.py (version 0.1) which is overrided by config/PLUGIN.py (version 0.2), and user can update this one with PLUGIN.zip (version 0.3) ... or even dropping a directory PLUGIN (version 0.4) ... i'm not sure yet all cases are properly handled, but basic functionnality is there: user can update/install plugins from JSON API

@mineo

This comment has been minimized.

Copy link
Member

commented Sep 6, 2015

Could you describe why we can't update plugins at runtime? I thought we already did that in case of version conflicts.

if error:
self.tagger.window.set_statusbar_message(
N_("Error loading plugins list: %(error)s"),
{'error': unicode(error)},

This comment has been minimized.

Copy link
@mineo

mineo Sep 6, 2015

Member

error is only a number, reply.errorString() returns a textual representation of the error that's useful even for users.

This comment has been minimized.

Copy link
@zas

zas Sep 6, 2015

Author Collaborator

I didn't change this part of code, but you're right, i'll fix it.

This comment has been minimized.

Copy link
@zas

zas Sep 6, 2015

Author Collaborator

Done in 5c58c1c

@@ -98,7 +98,7 @@
</sizepolicy>
</property>
<property name="text">
<string>Download plugins</string>
<string>Reload available plugins</string>

This comment has been minimized.

Copy link
@mineo

mineo Sep 6, 2015

Member

I think this is ambiguous - it could mean both "re-import all plugins" or "reload the information about available and installed plugins".

This comment has been minimized.

Copy link
@zas

zas Sep 6, 2015

Author Collaborator

How should i name it ?
It is "reload the information about available and installed plugins", so something like "Reload plugin infos" could do ?

This comment has been minimized.

Copy link
@mineo

mineo Sep 27, 2015

Member

The only software I have around that offers a similar function is IntelliJ, which uses "Reload List of Plugins", which seems fine to me, as does "Reload/Refresh [the] plugin list"

This comment has been minimized.

Copy link
@zas

zas Sep 27, 2015

Author Collaborator

Ok, done in 7284ca7

self.ui.plugins.sortByColumn(0, QtCore.Qt.AscendingOrder)
self.ui.plugins.setCurrentItem(self.ui.plugins.topLevelItem(0))

def reload_available_plugins(self):

This comment has been minimized.

Copy link
@mineo

mineo Sep 6, 2015

Member

This should block clicking the button while the download is in progress, otherwise it's possible to click the button fast enough to cause multiple requests at the same time, which then lead to duplicate entries in the plugin list.

/edit: And another thing: while the reload is in progress, the details section will show the details of a random plugin while the list itself is empty.

This comment has been minimized.

Copy link
@zas

zas Sep 6, 2015

Author Collaborator

Solved in 4f1bad1

@zas

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 6, 2015

Could you describe why we can't update plugins at runtime? I thought we already did that in case of version conflicts.

Well, i don't know, installing .zip or updating share the same code, but when a module of the same name was loaded, zip import fails with uncompressing error ... i try a lot of things, and didn't find where was the issue (check version zas@33e2459 to test if you want).
The sole thing i found is that zip import doesn't support module reloading (check zipimport doc), and perhaps this is the reason it fails.
I thought it could be a corruption of the zip file, but when it fails, restarting picard after the error, shows the zip was installed and loaded as it should.
Frankly, i have no clue about this issue, hence the dirty workaround.

@zas

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 26, 2015

@mineo , @phw : can you test and check the issue i had to workaround (#420 (comment)) ? I perhaps missed something, i would like to merge this feature but i'm not totally happy with the current behavior when updating plugins (it works, but it is a bit cumbersome). If you don't notice anything obvious enough to improve it, i'll merge it as is and we'll improve later. Just tell me what you think about it.

zas added some commits Sep 27, 2015

Download -> Install
Since we have Update -> Updated, it makes more sense to have Install -> Installed, instead of Download -> Installed

zas added a commit that referenced this pull request Sep 27, 2015

Merge pull request #420 from zas/download_plugins_all
Download, update and install plugins from UI

@zas zas merged commit 6e208ca into metabrainz:master Sep 27, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zas zas deleted the zas:download_plugins_all branch Sep 27, 2015

@fdemmer

This comment has been minimized.

Copy link

commented Oct 24, 2015

just fyi: zipimport does not work with zip files containing a comment. that is documented and will not be fixed in python2 (https://bugs.python.org/issue5950).

github seems to generate zip files with a comment on it's release page for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.