[addons] fix edge cases part 2

[tamland]

@fritsch @MartijnKaijser should repair your databases.

[MilhouseVH]

@tamland - you've seen #9481?

[tamland]

@MilhouseVH yes, but that's unrelated to these fixes.
jenkins build this please

[MilhouseVH]

With this PR (and without #9481) we're back to where we were a few days ago, with user-installed addons not being enabled when there is no pre-existing Addons*.db.

Small selection of addons that I have installed in my system:

rpi22:~/.kodi/userdata/Database# ls -la /storage/.kodi/addons | grep plugin
drwxr-xr-x    3 root     root          4096 Mar 16 21:34 plugin.video.amazon65
drwxrwxr-x    3 root     root          4096 Mar 17 20:19 plugin.video.prime_instant
drwxr-xr-x    4 root     root          4096 Mar 26 19:17 plugin.video.veetle
drwxr-xr-x    3 root     root          4096 Mar 26 03:34 plugin.video.youtube

After starting current master, with no Addons*.db so that Addons22.db is created from scratch, none of the above addons are being enabled:

rpi22:~/.kodi/userdata/Database # sqlite3 Addons22.db "select * from installed where addonID like 'plugin.%'"
41|plugin.video.amazon65|0|2016-03-30 21:18:56||
42|plugin.video.prime_instant|0|2016-03-30 21:18:56||
43|plugin.video.veetle|0|2016-03-30 21:18:56||
44|plugin.video.youtube|0|2016-03-30 21:18:56||

Dependencies are also not being enabled either - script.module.mechanize is required by plugin.video.prime_instant:

# sqlite3 Addons22.db "select * from installed where addonID like '%mechanize%'"
86|script.module.mechanize|0|2016-03-30 21:18:56||

It looks like the only addons being enabled are the "system" addons (kodi dependencies).

Here's a debug log when Addons22.db is created from scratch: http://sprunge.us/VBKY

[MilhouseVH]

In order to continue distributing test builds I've reverted this PR and reinstated PR9481, it might not be the best solution but it's the lesser of two evils IMHO, and at least ensures the user will have a usable system if they were to (re-)create Addons22.db from scratch..

[tamland]

@MilhouseVH I guess I didn't explain this well enough so let me try again: In #9110 this was changed. Instead of disabling some and enabling others on first start, it now separate between 'installing' and 'enabling' and consistently does not automatically set any addons that are merely found on the filesystem to enabled. What happens underneath is that it scans the filesystem and detects any addons that are added or removed and respectively install or uninstall the into the system. Anything not going through the normal install procedure (zip/repo) are not automatically enabled. Same as before, all addon metadata (such as enable/disable state, auto update settings, install dates etc) are stored in the database. This means, that if you want to retain any of that information, the database have to be kept.

[MilhouseVH]

So basically a system is trashed if the Addons*.db becomes corrupt, or the user is advised to delete Addons*.db because up until now that has always been a safe way for a user to recover from any sort of add-on misbehaviour. Excellent.

I'm not sure of any other way to say this, but this new behaviour is a very bad move and is going to cause a lot of problems down the line.

From my next build I'll include this PR and stop including #9481, and we'll see what happens.

[MartijnKaijser]

i'm with @tamland on this. Yes it was safe to just say to users delete addons*.db and restart. Now you have to warn them of the additional side affects which are no different than deleting your music/video and whatever database. Besides database corruption should be a thing of the past now days unless your sdcard is trashed which means you have additional problems. Also if an add-on could cause database corruption something is very wrong and should be tackled.

[MilhouseVH]

It's a lot worse than deleting music/video databases - for a start there's no "Rescan addons" button, and the user has to recover the system by re-installing add-ons one by one (assuming this is even possible, as all dependencies are disabled too which may prevent re-installation, resulting in the user having to perform a complete system reinstall).

At least if you delete your music/video database, you just click on re-scan, and wait.

[tamland]

So basically a system is trashed if the Addons_.db becomes corrupt, or the user is advised to delete Addons_.db because up until now that has always been a safe way for a user to recover from any sort of add-on misbehaviour. Excellent.

Not sure what you specifically mean by that but yes, if the database is gone every information you had about addons is trashed. This isn't any different than before. Actually, this ensures you can safely recover from addon misbehaviour as everything is set to clean state.

(assuming this is even possible, as all dependencies are disabled too which may prevent re-installation, resulting in the user having to perform a complete system reinstall).

That was fixed in #9486 and this 'bug' has been there since the beginning. Again it's no different from before and has nothing to do with this. It's only surfacing now.

[MilhouseVH]

This isn't any different than before.

Well no, that's not true at all - it's now very different than before.

Before, if you deleted your Addons*.db and restarted the system, your user-installed addons and dependencies were automatically re-enabled.

Now, pretty much everything the user installed is left disabled and will have to be manually re-enabled. Not the end of the world, but a tedious and seemingly unnecessary step.

OK I accept the way it works now has changed, time will tell how much of a problem that becomes, but please stop saying it's no different than before.

[tamland]

You are taking that sentence completely out of context. I'm taking about the specific things that are the same.

[tamland]

At least give it a fair chance. If it's truly that horrible to have it this way, we can talk again reevaluate it.

[iLLiac4]

I am with MilhouseVH and I think it is a horrible solution. I am sure people will start to ask why uninstall and reinstall of addon does not work. And it does not because dependencies are disabled so the uninstall and reinstall will nor work. And honestly i really doubt that ordinary users will figure this out that they have to enable dependencies and than do the rest. DB21 was a breeze compared to 22 and 23. At least implement an option to enable them all by a button. And also testing of things is now even more time consuming because testers have to delete the databases frequently to do some proper testing. But i am sure the time will tell that this is not a good idea.

[MartijnKaijser]

Stop deleting it!

[MilhouseVH]

@tamland: I've added http://trac.kodi.tv/ticket/16658, I'd be interested in why you think it's doing what it's doing and whether enabling all PVR clients is really a good idea.