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

[addons] repository code cleanup #10059

Merged
merged 5 commits into from Jul 8, 2016

Conversation

@tamland
Copy link
Member

commented Jul 2, 2016

This fixes the last few mutuable workarounds and drops the non-zipped/multi-directory 'features' which are no longer used (and really should never be used).

@tamland tamland referenced this pull request Jul 2, 2016
2 of 2 tasks complete
@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2016

jenkins build this please

@tamland tamland merged commit e03c834 into xbmc:master Jul 8, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
jenkins.kodi.tv You did a great job. Have a cookie.
Details
@tamland tamland deleted the tamland:repository_refactor branch Jul 8, 2016
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 16, 2016

Why was the multiple repo dropped? It should remain as previously discussed. Having multiple repo addons for different Kodi version is a pain to maintain for third party devs.

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2016

I though that was obvious. It says 'drop multi-directory support' in the commit. I don't remember this previous discussion.. The only one I remember is that it's not needed at all. And you have multiple addons in the same repo targeting different Kodi versions without having multiple repository addons.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 16, 2016

Well I guess I missed it and it is very much needed and it was also discussed last time you wanted to drop it.
No you have a single Repo Addon that tells each kodi version what Repo list to get. Dropping this was certainly not something I would be OK with

@MartijnKaijser

This comment has been minimized.

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2016

Why is it much needed though? Maybe I missed something, but the only thing I can remember from the last discussion was one piracy guy that complain because he use it as a workaround for something (and actually didn't even need it).
I did this now because I needed to make changes for #10060 and this awful repository code (yet again) got in the way.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 16, 2016

Skinners only needed to provide a single repo for users to install which contained different repo lists for each Kodi version. That way they don't need people to tell to install another new repo addon. This was exactly the same as we used it in the past before you updated the repo script. However that should not have meant it should be dropped.

edit:
now you are forcing devs who need to provide different addon version with the same ID to start giving users different repo addons again.

@axbmcuser

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2016

+1
Using a repo addon.xml file with multiple <dir> seems to be the main usage and was very end-user friendly.

Also - regarding this point - end-user upgrading from KODI 16 to 17 was seamless before the changes.

Before changes:

  • Users use KODI 16 with an existing multi-dir repo file and used a skin from that repo for example
  • User upgrades from KODI 16 to 17
  • Multi-dir repo offers repo and/or skin updates needed for new KODI 17 and updates everything automatically (default setting)
  • Everything seamsless, no user irritation, no manual communication/support hassle telling users whats wrong and advising them what to do to fix things

After changes:

  • Users use KODI 16 with an existing multi-dir repo file and used a skin from that repo for example
  • User upgrades from KODI 16 to 17
  • Repo addon instantly broken because multi-dir repo not working anymore
  • User don't know whats wrong. No updates are shown because of this
  • Users have to check the internet (every thread (as in my example) of every major skin-dev repo they have installed and to fix things 1by1 in form of installing krypton+ repo-zip files)
    (If you consider users having 3-4 skin-dev repos from the major skin-devs you can imagine how inconvenient it will be. Escpecially taking into account that it seems like KODI wants to move forward to a more modern/user-friendly direction)
@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2016

@MartijnKaijser You can do that by having multiple versions in the same repo and the same requirements on the addon instead of on a directory. That' the replacement. It's a much better and more general approach that can handle all kinds of requirements, not just one arbitrary one (that have nothing to do in the repository code btw) like the directories.

Repo addon instantly broken because multi-dir repo not working anymore
User don't know whats wrong. No updates are shown because of this

v16 and older supports single directory as well. You can update your repo to use the old way with a single directory and it will work for v17.

Escpecially taking into account that it seems like KODI wants to move forward to a more modern/user-friendly direction)

Exactly. Precisely the reason we need stop supporting legacy features like this if we are to have any chance to move forward with something new and better.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 16, 2016

fact is no one knows how that works, that is has changed and no example is available.

@mikesilvo164

This comment has been minimized.

Copy link

commented Jul 17, 2016

If this does what I think it does... Please don't drop this. It is already a PITA to offer something not (understandably) available in the default Kodi repo and push updates easily. I am not a user or provider of any add-ons that do, could, or does deal with any video, piracy, or anything at all negative to the Kodi experience and brand. To even offer this undocumented functionality was difficult to discover let alone maintain in my spare time. Please let me keep using my skin mod repository for all users of Kodi. Past, present, and future... 😃

@axbmcuser

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2016

@tamland
I don't have a problem with larger changes itself.
If there had been am example of the mentioned "functionality replacement" which could create a similar end-user experience as multi-dir repos did in the past, maybe no one would have bothered.
It's just that multi-dir support is dropped and every major skin-dev i talked to didn't even know his repo file wasn't working anymore. Most of them did or will end up fixing things quickly in form of trying to tell users to manually install different repo files from now on, which is the described downstep in terms of end user experience.

Is it possibly you provide an example of the proposed future way which provides same seamless cross-kodi-version-end-user-updates using one repo for all (skin/addon) versions?

I'm happy to adapt the changes to one uniform future repo.

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 17, 2016

It can be added back. It's not a very big deal or a whole lot of work (at least for backwards compatibility since I didn't think anyone even used this hidden undocumented feature). But since jarvis or something we've supported having multiple versions of the same addons in the same repository; so the idea is that repositories should be able to put all their addons in one repository. If you need to provide different version for different kodi version, specify that in the dependency requirement, add them all, and kodi will filter out the ones that isn't compatible.

@axbmcuser

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2016

I really like the idea of having a uniform repo with different versions of the same addon, although it seems no repo owner has tested or integrated it in their addon release workflow yet.
If it's really not that big of a deal - i think it would be great to have multi-dir support back for now, letting repo owners adapt to the (as it looks like widely unknown) possibility of a uniform repo. So they would have the chance to test and prepare for it.

Regarding multiple versions of the same addon in same repo:
Do you mean the typical xbmc.gui
<import addon="xbmc.gui" version="5.10.0"/> (Kodi 16)
<import addon="xbmc.gui" version="5.12.0"/> (Kodi 17)
dependency (which seems unique to every kodi version)? Or is there more to know?

I'm wondering about folder structure in a repo which provides different addon versions with same id.

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2016

Ok, reworked it a bit and added back with #10135. @axbmcuser that's pretty much it, yes. The folder structure is the same as now, you just keep the zips side-by-side. For example some.addon/some.addon-v1.0.0.zip, some.addon/some.addon-v2.0.0.zip etc and add both to addons.xml.

I'll try to get around to updating http://kodi.wiki/view/Add-on_repositories when I have the time. I see there's a few more things that isn't quite up-to-date;)

@axbmcuser

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2016

Many thanks - for both readding multi-dir support and outlining the possible side-by-side use of different addon versions. I'll make tests regarding a uniform repo and meanwhile reinstate the old multi-dir repo.
Looking forward to #10135 getting merged. :)

@gade01

This comment has been minimized.

Copy link

commented Jul 19, 2016

@tamland Thanks a lot for readding this :)

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

@tamland can you look at our internal release thread as this introduced an issue if you have multiple repos installed.

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