Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix Up #2659; Build libzstd.pc Whenever Building the Lib on Unix #2912

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

felixhandte
Copy link
Contributor

This PR builds on #2659 to add -pthread to libzstd.pc when the library is built with multithreading. It also promotes the .pc file to be built whenever the library is built (on Unix--unlike the original PR which builds it everywhere).

ericonr and others added 2 commits June 19, 2021 17:42
Add the libzstd.pc target to the lib target in lib/Makefile, which makes
it inherit LDFLAGS_DYNLIB from the lib-mt target. This allows us to add
a Libs.private field to libzstd.pc which gets conditionally populated
with '-pthread'.

The 1.5.0 release notes mention that the static library isn't
multi-threaded by default, due to concern for people building static
binaries with libzstd:

   Now the dynamic library supports multi-threaded compression by
   default.  Note that this property is not extended to the static
   library because doing so would have impacted the build script of
   existing client applications (requiring them to add -pthread to their
   recipe), thus potentially breaking their build.

To get closer to being able to enable multi-threading for all library
builds by default, this commit makes it so that any libzstd consumer
using pkg-config gets the correct flags.

We also fix the indentation of the rule for libzstd.pc and move it
outside the if/endif block for install rules (which uses a list of OSs
where the rules were validated), so the rule is available for all users
of the 'lib*' targets.
[lib] Fix libzstd.pc for lib-mt builds
@felixhandte felixhandte changed the base branch from pkg-config-fix to dev December 7, 2021 19:26
@felixhandte
Copy link
Contributor Author

Curiously, none of the GitHub Actions have run... Not sure why that is.

@felixhandte felixhandte merged commit 33562c5 into facebook:dev Dec 8, 2021
@ericonr
Copy link
Contributor

ericonr commented Dec 9, 2021

@felixhandte thank you for your fix and for merging it!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants