Bug 14657 - Extension Downloader doesn't show latest version
: Extension Downloader doesn't show latest version
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Plugins
: 7.4.0
: PC Other
: P2 normal (vote)
: 7.5.0
Assigned To: Adrian Smith
http://forums.slimdevices.com/showthr...
:
Depends on:
Blocks: 14321
  Show dependency treegraph
 
Reported: 2009-10-07 21:53 UTC by Erland Isaksson
Modified: 2010-04-08 17:25 UTC (History)
3 users (show)

See Also:
Category: ---


Attachments
Need two restarts to install a plugin (3.36 KB, application/octet-stream)
2009-10-13 16:11 UTC, Philip Meyer
Details
potential patch for installing from wrong repo (1.42 KB, patch)
2009-10-17 08:41 UTC, Adrian Smith
Details | Diff
Updated patch - only show highest version number plugins (2.85 KB, patch)
2009-10-25 04:56 UTC, Adrian Smith
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Erland Isaksson 2009-10-07 21:53:07 UTC
If a plugin is available in two plugin repositories and not previously installed on the local SqueezeCenter, the Extension Downloader (Plugins tab) shows the incorrect version.

For example, in the following repository there is a TrackStat 2.9pre5 version while there in the official repository is a 2.9.1 version.

http://erland.homeip.net/squeezecenterrepotesting.xml

If I have this repository configured in the "Plugins" tab, TrackStat shows up under "Erland's plugins and applets (BETA testing versions)" instead of under the "Recommended 3rd party plugins" list where the newest version is.

The problem is only for new installations, the upgrade detection seems to work correctly. If I have TrackStat 2.9.1 installed it doesn't consider the one in the testing repository as newer. If I have the 2.9pre5 version installed it correctly consider the 2.9.1 version newer and indicates that an upgrade is needed.
Comment 1 Adrian Smith 2009-10-11 05:41:53 UTC
At present the pruning out logic in the settings page is to show the later repo in the list - this is because I was assuming if you had a repo added with duplicate entries then it was because you wanted to use them to override the built in repos (recommened, 3rd party).

Do we always want to show the newest rather than the entry in the user selected repo?  It would mean you could never install an older version from a user selected repo?

I think this needs a discussion before changing.  In the interim I suggest you comment entries out of your testing repo once you make a new release (as you have done)
Comment 2 Erland Isaksson 2009-10-11 21:49:19 UTC
(In reply to comment #1)
> At present the pruning out logic in the settings page is to show the later repo
> in the list - this is because I was assuming if you had a repo added with
> duplicate entries then it was because you wanted to use them to override the
> built in repos (recommened, 3rd party).

The problem is that it doesn't override it completely, when I click the save button when selecting the older beta version (shown in the  list) is till installs the newest one from the main repository (not shown in the list).

It feels like a good idea make it possible to override, so we should probably just fix it so the override really works.
Comment 3 Adrian Smith 2009-10-12 11:27:10 UTC
OK - will look again as what you describe would definately be a bug - its installing a different version than the one shown?
Comment 4 Erland Isaksson 2009-10-12 15:01:54 UTC
(In reply to comment #3)
> OK - will look again as what you describe would definately be a bug - its
> installing a different version than the one shown?

Yes, that was definitely the case when I tested it. 

I checked "TrackStat *Beta*" from the testing repository and after the restart it had installed 2.9.1 which comes from the official non beta repository. It should have installed the 2.9pre5 version which is in the testing repository mentioned in the first comment to this bug report.

The only manual repository I'd setup was a single testing repository as mentioned in the initial comment. The 2.9.1 version is in the automatically included recommended repository.
Comment 5 Philip Meyer 2009-10-13 16:11:35 UTC
Created attachment 6114 [details]
Need two restarts to install a plugin

The other issue is that some plugins seem to require two restarts for the plugin to get installed correctly.

This happens when I try to install Erlands Database Query plugin:

1. Uninstalled DatabaseQuery plugin, restarted
2. Installed DatabaseQuery, restarted.
3. I was told that plugin updates are available, and upon visiting Plugins page, it automatically applied an update, and asked for another restart.
4. Restarted SbS again.

I caught the log, trimmed it to only show lines containing DatabaseQuery.
Comment 6 Adrian Smith 2009-10-14 13:47:51 UTC
Thats strange - it looks like the server restarted but it may not have saved the preference which indicates there is a new plugin to install.

Is it reproducable if you leave it 10 seconds after selecting a new plugin before asking the server to restart?

Michael - is it possible restarting the server on windows may no cause prefs to be saved first?
Comment 7 Philip Meyer 2009-10-14 14:42:11 UTC
Yes - repeatable.

I run SVN via ActivePerl, via srvany.exe (to run ActivePerl source code as a service), if that makes any difference.

So, I manually restart by running a batch script:

net stop slimtrunk
net start slimtrunk
Comment 8 Michael Herger 2009-10-14 23:15:46 UTC
I'm sorry for the delay - obviously our "reply to bug report" feature is broken again :-(...

In answer to Triode's question about restarting SBS and losing prefs:
It shouldn't. Restart is going through the same shutdown procedure as everything else.
 
Restart method depends on the way you're running SBS though:
 
- it only works when running from the .exe, not from code
- when run as a service, Windows service control is being used in order to stop/start SBS
- when run as an app, then main::stopServer() is called


Which means that Phil's configuration can't auto restart. In this case you should see a message telling you to restart your server.

Whether this will lose prefs or not - I don't know. How is srvany stopping SBS? I would probably be safer to shut it down through the CLI ("stopserver") in order to trigger all the cleaning up needed.
Comment 9 Philip Meyer 2009-10-15 00:49:11 UTC
>Which means that Phil's configuration can't auto restart. In this case you
>should see a message telling you to restart your server.
Yes, I don't auto-restart.  I wouldn't want it to, either!

>Whether this will lose prefs or not - I don't know. How is srvany stopping SBS?
I stop the service with "net stop slimtrunk", which just kills the app.

I have never seen any issues with this, and all other plugins (I installed at the same time as this troublesome one) were installed correctly, so I don't think it's a shutdown/loss of preferences issue.

If SqueezeboxServer is installed as a service, how is this different to running srvany to run the source code as a service?  Doesn't Squeezebox Server receive the windows EndSession event?
Comment 10 Adrian Smith 2009-10-15 12:49:36 UTC
Phil - as it is reproducable.  If you ask for the install, then stop the server whats in the prefs/plugin/state.prefs file - does this show the pluin in the needs* state???  

I don't see why it does not show up as extracting when you restart the server, my only guess is the request to install it is not getting saved in the pref file?
Comment 11 Philip Meyer 2009-10-15 14:28:41 UTC
I tried again, and it installed on the first attempt.
I tried a few times - only once it didn't install on first attempt, so not quite as repeatable as I was finding the other day.

One thing I noticed was that when it works first time, there are two dialogs when hitting Apply.  I think that for the time when it didn't work, the second dialog "Please restart Squeezebox Server for the changes to take effect." wasn't displayed.

Trying to find a repeatable way to get it to fail...
Comment 12 Adrian Smith 2009-10-17 08:17:03 UTC
Phil - I think I've found a possible reason for your two update problem.  At present the "Server Restart" message is shown when you have selected a plugin to install even if it is it not yet fully downloaded.  The restart code waits for the download to complete or fail but it does not guarentee that the download has occured before before restarting.  This could mean that if a download site is slow or constantly failing then the server will show the restart dialog and restart but the plugin will not be installed.

I am not sure of a simple fix for this - we could have another popup showing that the plugins are being downloaded and then checked against the sha1 before showing the restart message, but this needs more work from Michael on the web front end.
Comment 13 Adrian Smith 2009-10-17 08:41:20 UTC
Created attachment 6150 [details]
potential patch for installing from wrong repo

Erland - could you try this patch.  This should mean that we install from the same repo that we show in the list.  This is the repo with the highest weight which means the one lowest down the list.

Phil/MC - can we create new bugs for other extension downloader problems so that we can keep this to installing from the wrong repo.  Thanks.
Comment 14 Erland Isaksson 2009-10-19 10:36:02 UTC
(In reply to comment #13)
> Created an attachment (id=6150) [details]
> potential patch for installing from wrong repo
> 
> Erland - could you try this patch.  This should mean that we install from the
> same repo that we show in the list.  This is the repo with the highest weight
> which means the one lowest down the list.

It works correctly if I only have a single repository manually configured, but if I have configured manually two repositories that contains the same plugin it shows the older beta version in the list.

I tried by configuring these manually:
http://erland.homeip.net/squeezecenterrepotesting.xml
http://erlandplugins.googlecode.com/svn/repository/trunk/latest.xml

Independent on which order I put the the Custom Browse beta version is shown in the list.

If it gets complicated it might be good enough as it works with this patch, after all we are probably only talking about few users who are going to have several manually configured repositories which contains the same plugin.
Comment 15 Erland Isaksson 2009-10-21 22:11:56 UTC
Adrian, I'm not completely sure if this is related to one of your patches but when I just run Extension Downloader with a single manually configured repository it didn't show upgrades in the list when I had your patches applied.

I removed your patches, this one and the one for Applet Installer, and after that the upgrades are shown. I'm talking about showing available upgrades of already installed plugins.
Comment 16 Adrian Smith 2009-10-24 13:40:27 UTC
Erland - was this for a plugin which is in the manual repo and in one of the main repos too (at a different version).  If so is the newer version only in the manually configured repo?
Comment 17 Erland Isaksson 2009-10-24 23:36:32 UTC
(In reply to comment #16)
> Erland - was this for a plugin which is in the manual repo and in one of the
> main repos too (at a different version).  If so is the newer version only in
> the manually configured repo?

Yes
It was with Custom Scan.
The 2.7.1 version was in the official third party repository and I had a 2.7.2 version in the only manually configured repository.
Comment 18 Adrian Smith 2009-10-25 04:56:01 UTC
Created attachment 6197 [details]
Updated patch - only show highest version number plugins

Erland - alternative patch for the two extension downloader issues (this plus bug14321).  This only shows the highest version number independant of which repo a plugin is in.  I hope this means it fights the rest of the code less and so there aren't so many corner cases.  Can you test before I commit?
Comment 19 Erland Isaksson 2009-10-27 11:32:26 UTC
(In reply to comment #18)
> Erland - alternative patch for the two extension downloader issues (this plus
> bug14321).  This only shows the highest version number independant of which
> repo a plugin is in.  I hope this means it fights the rest of the code less and
> so there aren't so many corner cases.  Can you test before I commit?

I've done some tests and it seems to work correctly.
Comment 20 Adrian Smith 2009-10-27 14:28:54 UTC
Thanks Erland.  Patch committed in change 29029.  Please reopen if other issues seen.
Comment 21 Chris Owens 2010-04-08 17:25:28 UTC
This bug has been marked fixed in a released version of Squeezebox Server or the accompanying firmware or mysqueezebox.com release.

If you are still seeing this issue, please let us know!