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

[cpluff] link with cpluff instead of loading it dynamic #13673

Merged
merged 1 commit into from Mar 21, 2018

Conversation

Rechi
Copy link
Member

@Rechi Rechi commented Mar 19, 2018

Description

Don't load cpluff dynamic, but link against it (shared/static).

Motivation and Context

As Kodi calls cpluff functions on startup there is no reason to lazy load the lib.

How Has This Been Tested?

compiled and runtime tested for windows and linux

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • 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

@Rechi Rechi added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: Add-ons v18 Leia labels Mar 19, 2018
@Rechi Rechi added this to the Leia 18.0-alpha2 milestone Mar 19, 2018
@Rechi Rechi requested a review from notspiff March 19, 2018 17:01
@Rechi Rechi force-pushed the cleanup/cpluff branch 2 times, most recently from 93dc69c to 47ae6d5 Compare March 20, 2018 07:21
Copy link
Contributor

@notspiff notspiff left a comment

Choose a reason for hiding this comment

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

looks all good

@Rechi Rechi merged commit faa1406 into xbmc:master Mar 21, 2018
@Rechi Rechi deleted the cleanup/cpluff branch March 21, 2018 10:38
@MilhouseVH
Copy link
Contributor

Any ideas? http://ix.io/10zf

Rechi added a commit that referenced this pull request Mar 22, 2018
[cmake] fix Ninja generator after #13673
@FernetMenta
Copy link
Contributor

I get dupe symbols on OSX:

duplicate symbol _hash_insert in:
    /Users/DE213022/progs/src/xbmc/build/build/cpluff/lib/libcpluff.a(hash.o)
    /Users/Shared/xbmc-depends/macosx10.13_x86_64-target-debug/lib/mariadb/libmariadbclient.a(ma_hash.c.o)
duplicate symbol _hash_delete in:
    /Users/DE213022/progs/src/xbmc/build/build/cpluff/lib/libcpluff.a(hash.o)
    /Users/Shared/xbmc-depends/macosx10.13_x86_64-target-debug/lib/mariadb/libmariadbclient.a(ma_hash.c.o)
duplicate symbol _hash_free in:
    /Users/DE213022/progs/src/xbmc/build/build/cpluff/lib/libcpluff.a(hash.o)
    /Users/Shared/xbmc-depends/macosx10.13_x86_64-target-debug/lib/mariadb/libmariadbclient.a(ma_hash.c.o)
duplicate symbol _list_delete in:
    /Users/DE213022/progs/src/xbmc/build/build/cpluff/lib/libcpluff.a(list.o)
    /Users/Shared/xbmc-depends/macosx10.13_x86_64-target-debug/lib/mariadb/libmariadbclient.a(ma_list.c.o)
ld: 4 duplicate symbols for architecture x86_64

@Rechi
Copy link
Member Author

Rechi commented Mar 23, 2018

force a rebuild of libcpluff

# the following symbols are clashing with mariadb symbols
CPPFLAGS += -Dhash_delete=kazlib_hash_delete -Dhash_free=kazlib_hash_free -Dhash_insert=kazlib_hash_insert -Dlist_delete=kazlib_list_delete

@FernetMenta
Copy link
Contributor

worked, thanks
shouldn't this be triggered automatically?

@Rechi
Copy link
Member Author

Rechi commented Mar 23, 2018

No as the files at lib/cpluff are not added as dependency to the cmake libcpluff target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Add-ons Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants