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] microhttpd: only link with libraries #15621

Merged
merged 2 commits into from Mar 4, 2019

Conversation

Projects
None yet
2 participants
@Rechi
Copy link
Member

commented Feb 26, 2019

Description

pkg-config adds the static libraries to libraries if microhttpd is a static lib

Motivation and Context

pkg-config --libs --static libmicrohttpd on Debian lists -lunistring, but libunistring.so isn't installed.
It is packaged in the libunistring-dev package, and is only a symlink to the real versioned libs. Those are installed correctly, so not a packaging bug from Debian.

How Has This Been Tested?

compiled for linux and osx

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

@Rechi Rechi added this to the Leia 18.2-rc1 milestone Feb 26, 2019

@Rechi Rechi requested a review from wsnipex Feb 26, 2019

@Rechi Rechi force-pushed the Rechi:cmake/microHttpd branch from e729997 to 537b724 Feb 27, 2019

@Rechi

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

@wsnipex ping

@Rechi Rechi force-pushed the Rechi:cmake/microHttpd branch from 537b724 to 79890d1 Mar 1, 2019

@Rechi Rechi requested a review from wsnipex Mar 1, 2019

else()
list(APPEND MICROHTTPD_LIBRARIES ${PC_MICROHTTPD_STATIC_LIBRARIES})
if(${MICROHTTPD_LIBRARY} MATCHES ".+\.a$" AND PC_MICROHTTPD_STATIC_LDFLAGS)
list(APPEND MICROHTTPD_LIBRARIES ${PC_MICROHTTPD_STATIC_LDFLAGS})

This comment has been minimized.

Copy link
@wsnipex

wsnipex Mar 3, 2019

Member

This still lists unistring, so it results in essentially the same behavior as without this PR for depends builds.

This comment has been minimized.

Copy link
@Rechi

Rechi Mar 3, 2019

Author Member

This is not for fixing depends builds, it's for fixing builds with shared system libs.

This comment has been minimized.

Copy link
@wsnipex

wsnipex Mar 3, 2019

Member

Now I'm confused, since I'm not aware that they are broken. PPA builds have been fine for a long time.
I also took another look and even though unistring is referenced in the .pc file, cmake seems to filter this out:

PC_MICROHTTPD_STATIC_LDFLAGS:INTERNAL=-lmicrohttpd;-lgnutls;-lgmp;/usr/lib/x86_64-linux-gnu/libunistring.so;-lidn2;-lhogweed;-lgmp;-lnettle;-ltasn1;-lp11-kit;-lz
PC_MICROHTTPD_STATIC_LDFLAGS_OTHER:INTERNAL=/usr/lib/x86_64-linux-gnu/libunistring.so
PC_MICROHTTPD_STATIC_LIBRARIES:INTERNAL=microhttpd;gnutls;gmp;idn2;hogweed;gmp;nettle;tasn1;p11-kit;z

This comment has been minimized.

Copy link
@Rechi

Rechi Mar 3, 2019

Author Member

system libs

+ pkg-config --libs libmicrohttpd
-lmicrohttpd
+ pkg-config --libs-only-l libmicrohttpd
-lmicrohttpd
+ pkg-config --libs-only-L libmicrohttpd

+ pkg-config --libs-only-other libmicrohttpd

+ pkg-config --libs --static libmicrohttpd
-lmicrohttpd -lgnutls -lgmp -lunistring -lidn2 -latomic -lhogweed -lgmp -lnettle -ltasn1 -lp11-kit
+ pkg-config --libs-only-l --static libmicrohttpd
-lmicrohttpd -lgnutls -lgmp -lunistring -lidn2 -latomic -lhogweed -lgmp -lnettle -ltasn1 -lp11-kit
+ pkg-config --libs-only-L --static libmicrohttpd

+ pkg-config --libs-only-other --static libmicrohttpd

PC_MICROHTTPD_STATIC_LDFLAGS:INTERNAL=-lmicrohttpd;-lgnutls;-lgmp;-lunistring;-lidn2;-latomic;-lhogweed;-lgmp;-lnettle;-ltasn1;-lp11-kit
PC_MICROHTTPD_STATIC_LIBRARIES:INTERNAL=microhttpd;gnutls;gmp;unistring;idn2;atomic;hogweed;gmp;nettle;tasn1;p11-kit

depends

+ pkg-config --libs libmicrohttpd
-L<PREFIX>lib -lmicrohttpd
+ pkg-config --libs-only-l libmicrohttpd
-lmicrohttpd
+ pkg-config --libs-only-L libmicrohttpd
-L<PREFIX>lib
+ pkg-config --libs-only-other libmicrohttpd

+ pkg-config --libs --static libmicrohttpd
-L<PREFIX>lib -lmicrohttpd -lgcrypt -lgpg-error -lgnutls -lz -lgmp -lhogweed -lgmp -lnettle
+ pkg-config --libs-only-l --static libmicrohttpd
-lmicrohttpd -lgcrypt -lgpg-error -lgnutls -lz -lgmp -lhogweed -lgmp -lnettle
+ pkg-config --libs-only-L --static libmicrohttpd
-L<PREFIX>lib
+ pkg-config --libs-only-other --static libmicrohttpd

I don't know what's different on your system, but linking with PC_MICROHTTPD_STATIC_LIBRARIES if microhttpd is a shared library is always wrong.

This comment has been minimized.

Copy link
@wsnipex

wsnipex Mar 3, 2019

Member

I agree it's unexpected, but the thing is, it's not only my system, it also works fine on launchpad. And since launchpad uses completely clean ubuntu, this must be something on your end.
I'd like to find out what.

But using PC_MICROHTTPD_STATIC_LDFLAGS breaks it for me, because now unistring is not filtered anymore

This comment has been minimized.

Copy link
@Rechi

Rechi Mar 3, 2019

Author Member

If you are using the shared system library PC_MICROHTTPD_STATIC_LDFLAGS won't be added to MICROHTTPD_LIBRARIES, because of the .a check you requested to add. If you want, I can replace PC_MICROHTTPD_STATIC_LDFLAGS with PC_MICROHTTPD_STATIC_LIBRARIES.

This comment has been minimized.

Copy link
@wsnipex

wsnipex Mar 3, 2019

Member

yes, plz change to PC_MICROHTTPD_STATIC_LIBRARIES

This comment has been minimized.

Copy link
@Rechi

Rechi Mar 3, 2019

Author Member

done

Rechi added some commits Mar 3, 2019

[cmake] microhttpd: only link with libraries
pkg-config adds the static libraries to libraries if microhttpd is a static lib

@Rechi Rechi force-pushed the Rechi:cmake/microHttpd branch from 79890d1 to 0c6e8e0 Mar 3, 2019

@wsnipex

wsnipex approved these changes Mar 4, 2019

@Rechi Rechi merged commit 3f51d04 into xbmc:master Mar 4, 2019

1 check passed

default You're awesome. Have a cookie
Details

@Rechi Rechi deleted the Rechi:cmake/microHttpd branch Mar 4, 2019

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