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

[cmake] fixes cpack dependency list generation #11370

Merged
merged 2 commits into from Jan 9, 2017

Conversation

@nsenica
Copy link
Contributor

commented Jan 4, 2017

Description

While trying to generate deb packages from cpack I realised that the dependencies where not properly generated for binary packages. I also realised that the current implementation goes a bit about the cpack specification (Check the Note), which states that if SHLIBDEPS is ON the found dependencies will be appended to the DEPENDS of each package. Which was not what was happening with the kodi's implemention - it was overriding as the information within the txts states, which needs to be changed as well.
This PR enables PACKAGE_SHLIBDEPS in the needed packages and allows it to be appended to the PACKAGE_DEPEND list.

Documentation in each .txt template was updated to reflect the above info.

Backport will be provided when the final version of this PR is agreed upon.

Motivation and Context

Create .deb with the correct dependencies.

How Has This Been Tested?

Building .deb packages using cpack.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed
@hudokkow

This comment has been minimized.

Copy link
Member

commented Jan 5, 2017

Looks good, thx.

Are you sure it's picking all the deps? I have a vague memory about problems with this approach but can't test now.

I couldn't find the note in the link you posted above. The correct link is https://cmake.org/cmake/help/v3.6/module/CPackDeb.html#variable:CPACK_DEBIAN_%3CCOMPONENT%3E_PACKAGE_DEPENDS

btw, we're missing a kodi-game-dev.txt.in since 60cc35d. Care to add it?

@nsenica

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2017

@hudokkow Thanks for your comments.

I've updated the original post with the correct link, thanks for pointing that out.

About the dependencies, the resulting dependencies after generating the .debs match the required ones that @wsnipex defined in debian/control file for trusty. The only things I had to leave out, as I didn't find a way to add it, were the "misc:Depends" and "python:Depends" but I believe that those do not add any extra dependency.
The testing I did, provided me with enough confidence that we are not missing out any (at least) important dependency, but with this change, if we figure out that it's missing we can always add it as it now give us with that flexibility.

I'll add kodi-game-dev.txt.in, sure. :)

Once this is done, I can build and generate deb files so you can have a look at them, or I can post here the result of dpkg -I *.deb of the generated packages, as you prefer.

@nsenica

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2017

Added kodi-game-dev.txt.in

@wsnipex

This comment has been minimized.

Copy link
Member

commented Jan 5, 2017

Are you sure that our dyloaded deps are reflected in depends? As said in the mail, shlibdeps doesn't pick up dependencies not linked against.

@nsenica

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2017

@wsnipex those are untouched and are defined here and they're exacly as it is in your debian/control file, if I'm not mistaken, except for librtmp and libmad which are not required anymore.

Once again, if we realise that they're missing, we can always add them later.

@wsnipex

This comment has been minimized.

Copy link
Member

commented Jan 5, 2017

ok, thanks :)

@wsnipex

This comment has been minimized.

Copy link
Member

commented Jan 8, 2017

@hudokkow good to go?

@hudokkow hudokkow merged commit ec6cf94 into xbmc:master Jan 9, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hudokkow hudokkow added this to the L 18.0-alpha1 milestone Jan 9, 2017
@hudokkow

This comment has been minimized.

Copy link
Member

commented Jan 9, 2017

Thx.
Please backport without the kodi-game-dev.txt.in commit.

@nsenica nsenica referenced this pull request Jan 9, 2017
4 of 9 tasks complete
@nsenica nsenica deleted the PIPplware:cpack_deb_deps_fix branch Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.