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

Move Neptune/Platinum to an external dependency #17678

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lrusak
Copy link
Contributor

@lrusak lrusak commented Apr 15, 2020

This removes the use of lib/libUPnP which is a combination of Neptune and Platinum libraries and allows them to be built as external dependencies.

There really is no reason to include the code in our tree. We already use specific versions of libdvd and ffmpeg so we should do the same for Neptune/Platinum.

I've forked the respective repositories and created simple CMake based build systems for them that mirrors the functionality that we already had. This makes it so we don't have to track changes to these libraries in our Kodi source tree and makes upstreaming easier (though I think development for these libraries is non existent anymore). I've applied all of our internal patches to the repos with relevant author info, commit names, and dates.

The external trees are here for now but would ideally be moved under the xbmc name:
https://github.com/lrusak/Neptune/tree/1.2.3-Kodi
https://github.com/lrusak/Platinum/tree/1.2.0-Kodi

When creating the build system for these libraries I had to use some creative liberty and installed the headers into the respective library name include folders such as include/Neptune/Neptune.h and include/Platinum/Platinum.h that is why there is changes to the include statements in our local files. I also did this to avoid polluting the include folder as there is quite a few headers that are needed. The other annoying part is that the Platinum refers to Neptune headers without a folder name so we need to include include/Neptune into the Platinum build.

@lrusak lrusak added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Improvement non-breaking change which improves existing functionality CMake v19 Matrix labels Apr 15, 2020
@lrusak lrusak added this to the Matrix 19.0-alpha 1 milestone Apr 15, 2020
@lrusak lrusak changed the title Move Neptune/Platinum to and external dependency Move Neptune/Platinum to an external dependency Apr 15, 2020
@AlwinEsch
Copy link
Member

AlwinEsch commented Apr 15, 2020

Really nice to remove them and have much more clean.

Only not sure how this handled by most Linux distributions? There mostly Kodi's own depends installed inside system.
Looked at ubuntu and not found the platinum and neptune, seems not available there?

@asavah
Copy link
Contributor

asavah commented Apr 15, 2020

AFAIK currently no distro ships platinum/neptune and this PR does one thing that IMO is bad:
UPNP is no longer optional as in can't be disabled (ENABLE_UPNP is gone), if no distro neptune/platinum is found it falls back to internal build, with this PR you no longer can avoid building UPNP.

@lrusak
Copy link
Contributor Author

lrusak commented Apr 15, 2020

Only not sure how this handled by most Linux distributions? There mostly Kodi's own depends installed inside system.
Looked at ubuntu and not found the platinum and neptune, seems not available there?

Yea it will have to be handled like libdvd

AFAIK currently no distro ships platinum/neptune and this PR does one thing that IMO is bad:
UPNP is no longer optional as in can't be disabled (ENABLE_UPNP is gone), if no distro neptune/platinum is found it falls back to internal build, with this PR you no longer can avoid building UPNP.

That's my mistake, I'll fix that

@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Apr 16, 2020
@bkuhls
Copy link
Contributor

bkuhls commented Apr 16, 2020

UPNP is no longer optional as in can't be disabled (ENABLE_UPNP is gone)

Hi, to retain backward-compatibility for the cmake options what about something like this?
bkuhls@33c32c4

Btw: building kodi with and without neptune/platinum works

Updated: bkuhls@4e83e78

@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label Apr 17, 2020
if(PLATINUM_URL)
get_filename_component(PLATINUM_URL "${PLATINUM_URL}" ABSOLUTE)
else()
set(PLATINUM_URL https://github.com/lrusak/Platinum/archive/${PLATINUM_VER}.tar.gz)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we fork to xbmc org?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the plan would be to move them to be under the org

@lrusak
Copy link
Contributor Author

lrusak commented Apr 21, 2020

@Paxxi can you take a stab at making the windows libs?

@Paxxi
Copy link
Member

Paxxi commented Apr 21, 2020

Sure 😀

@Montellese
Copy link
Member

I never liked how those include paths to Platinum and Neptune contained Source so changing / fixing that is actually a good thing IMO.

@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Sep 8, 2020
@jenkins4kodi
Copy link
Contributor

@lrusak this needs a rebase

@DaveTBlake DaveTBlake removed this from the Matrix 19.0-alpha 3 milestone Oct 30, 2020
@bkuhls
Copy link
Contributor

bkuhls commented Oct 31, 2020

A rebased version of this patch is available here: master...bkuhls:master-PR17678

@bkuhls
Copy link
Contributor

bkuhls commented Feb 13, 2022

A rebased version of this patch is available here: master...bkuhls:master-PR17678

rebased

@fuzzard fuzzard removed the v20 Nexus label Oct 1, 2022
@bkuhls
Copy link
Contributor

bkuhls commented Aug 20, 2023

A rebased version of this patch is available here: master...bkuhls:master-PR17678

again rebased

@bkuhls
Copy link
Contributor

bkuhls commented Jan 3, 2024

ping, shall I try to rebase again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake Rebase needed PR that does not apply/merge cleanly to current base branch Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants