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

[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts #18054

Merged
merged 6 commits into from Jun 14, 2020

Conversation

AlwinEsch
Copy link
Member

Description

This fix the stupid clang feature about export of C++ parts within addon headers, where a on addon A compiled class or function becomes public on addon B and this call A instead his.

Further is the General part cleaned up and "C++" / "C" parts separated.

Also is a change added to allow two different addon type ID's, to allow better change and update of it.

Addons not need a recompile! only want to have them in before the rest of PVR's becomes updated.

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

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

@AlwinEsch AlwinEsch added Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality Component: Binary add-ons API change: Binary add-ons v19 Matrix labels Jun 13, 2020
@AlwinEsch AlwinEsch added this to the Matrix 19.0-alpha 1 milestone Jun 13, 2020
@AlwinEsch AlwinEsch changed the title [addon] Fix pvr rework parts [addon] fix PVR rework parts Jun 13, 2020
@ksooo
Copy link
Member

ksooo commented Jun 13, 2020

The title of this PR is a bit misleading, as there is only one commit out of 6 which is PVR related.

Copy link
Contributor

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

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

Looks good, also runtime tested and addons now only call their own contexts.

@phunkyfish phunkyfish changed the title [addon] fix PVR rework parts [addon] fix PVR rework parts and other addon header changes Jun 13, 2020
@AlwinEsch AlwinEsch changed the title [addon] fix PVR rework parts and other addon header changes [addon] prevent publicate of C++ functions / classes from addon headers and cleanup General parts Jun 13, 2020
@ksooo
Copy link
Member

ksooo commented Jun 13, 2020

@phunkyfish did you have the chance to test migration API level 6.x.x -> 7.0.0? For me, the old addons silently (!) disappeared from list of installed addons. I had to manually reinstall all my pvr client addons. If I understand correctly, this PR should contain a fix for that.

@AlwinEsch AlwinEsch changed the title [addon] prevent publicate of C++ functions / classes from addon headers and cleanup General parts [addon] prevent publicate of C++ functions / classes from addon headers, fix old to bew pvr addon update and cleanup General parts Jun 13, 2020
@AlwinEsch AlwinEsch changed the title [addon] prevent publicate of C++ functions / classes from addon headers, fix old to bew pvr addon update and cleanup General parts [addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts Jun 13, 2020
@phunkyfish
Copy link
Contributor

I did not as I had already built and installed the PR before it was merged my my addon were already the new version.

@phunkyfish
Copy link
Contributor

But to test this we would need this merged and to release the pvr addons right?

@ksooo
Copy link
Member

ksooo commented Jun 13, 2020

But to test this we would need this merged and to release the pvr addons right?

@AlwinEsch ?

@AlwinEsch
Copy link
Member Author

jenkins build this with addons please

Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

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

lgtm, runtime-tested, "incompatible addon disabled" dialog appears now.

There seem by clang builds a problem that a interface function of other
addons is called instead of correct addon.
Make only on begin and end this
This have the "C" alone on end for other languages and safe ABI.
This to allow both names xbmc.pvrclient (for updates e.g. Leia to Matrix)
and kodi.pvrclient where now used.

Further allow this also other names changes if needed.
Changes before affected all addons, there no need to increase min.
but on all places and all have the version a bit increased.
@AlwinEsch AlwinEsch merged commit 0bb0b9f into xbmc:master Jun 14, 2020
@AlwinEsch AlwinEsch deleted the fix-pvr-rework-parts branch June 14, 2020 08:20
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Jun 14, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Jun 15, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
[addon] prevent publicate of C++ functions / classes from headers, fix old to new pvr update and cleanup General parts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change: Binary add-ons Component: Binary add-ons Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants